From fd6c388adedb75d3dde66f076b533f895dd1093c Mon Sep 17 00:00:00 2001 From: "mstoltz%netscape.com" Date: Tue, 14 Aug 2001 00:18:58 +0000 Subject: [PATCH] 86984 - make history.length sameOrigin-accessible. Security prefs change. 91714 - CheckLoadURI should trest 'safe' and 'unsafe' about: URLs as different protocols 56260 - 'Remember This Decision' in signed script grant dialog should default to unchecked 83131 - More descriptive security error messages 93951 - Added null check in GetBaseURIScheme to prevent crash. All bugs r=jtaylor, sr=jst git-svn-id: svn://10.0.0.236/trunk@100964 18797224-902f-48f8-a5cc-f745e15eee43 --- .../caps/include/nsScriptSecurityManager.h | 4 +- mozilla/caps/src/nsScriptSecurityManager.cpp | 177 +++++++++++++----- mozilla/dom/src/base/nsDOMClassInfo.cpp | 37 ---- mozilla/dom/src/base/nsGlobalWindow.cpp | 2 - .../xmlextras/base/src/nsXMLHttpRequest.cpp | 10 +- 5 files changed, 136 insertions(+), 94 deletions(-) diff --git a/mozilla/caps/include/nsScriptSecurityManager.h b/mozilla/caps/include/nsScriptSecurityManager.h index 55085f15764..b076657f8b7 100644 --- a/mozilla/caps/include/nsScriptSecurityManager.h +++ b/mozilla/caps/include/nsScriptSecurityManager.h @@ -165,8 +165,8 @@ private: SavePrincipal(nsIPrincipal* aToSave); nsresult - CheckXPCPermissions(JSContext* cx, nsISupports* aObj, - const char* aObjectSecurityLevel, const char* aErrorMsg); + CheckXPCPermissions(nsISupports* aObj, + const char* aObjectSecurityLevel); nsresult InitPrefs(); diff --git a/mozilla/caps/src/nsScriptSecurityManager.cpp b/mozilla/caps/src/nsScriptSecurityManager.cpp index 615108125ea..b5343f60865 100644 --- a/mozilla/caps/src/nsScriptSecurityManager.cpp +++ b/mozilla/caps/src/nsScriptSecurityManager.cpp @@ -163,7 +163,7 @@ nsScriptSecurityManager::CheckConnect(JSContext* aJSContext, nsresult rv = CheckLoadURIFromScript(aJSContext, aTargetURI); if (NS_FAILED(rv)) return rv; - return CheckPropertyAccessImpl(nsIXPCSecurityManager::ACCESS_GET_PROPERTY, nsnull, + return CheckPropertyAccessImpl(nsIXPCSecurityManager::ACCESS_CALL_METHOD, nsnull, aJSContext, nsnull, nsnull, aTargetURI, nsnull, nsnull, aClassName, aPropertyName, nsnull); } @@ -187,6 +187,7 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, // We have native code or the system principal: just allow access return NS_OK; + static const char unknownClassName[] = "UnknownClass"; #ifdef DEBUG_mstoltz if (aProperty) printf("### CheckPropertyAccess(%s.%s, %i) ", aClassName, aProperty, aAction); @@ -198,7 +199,7 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, aClassInfo->GetClassDescription(getter_Copies(classNameStr)); className = classNameStr.get(); if(!className) - className = "UnknownClass"; + className = unknownClassName; nsCAutoString propertyStr(className); propertyStr += '.'; propertyStr.AppendWithConversion((PRUnichar*)JSValIDToString(aJSContext, aName)); @@ -236,11 +237,11 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, className = classNameStr.get(); if (!className) - className = "UnknownClass"; + className = unknownClassName; propertyName.AssignWithConversion((PRUnichar*)JSValIDToString(aJSContext, aName)); } - // if (aPropertyStr), we were called from CheckPropertyAccess or checkConnect, + // if (aProperty), we were called from CheckPropertyAccess or checkConnect, // so we can assume this is a DOM class. Otherwise, we ask the ClassInfo. secLevel = GetSecurityLevel(subjectPrincipal, (aProperty || IsDOMClass(aClassInfo)), @@ -350,14 +351,64 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, } } } - rv = CheckXPCPermissions(aJSContext, aObj, objectSecurityLevel, - "Permission denied to access property"); + rv = CheckXPCPermissions(aObj, objectSecurityLevel); #ifdef DEBUG_mstoltz if(NS_SUCCEEDED(rv)) printf("CheckXPCPerms GRANTED.\n"); else printf("CheckXPCPerms DENIED.\n"); #endif + + if (NS_FAILED(rv)) //-- Security tests failed, access is denied, report error + { + nsCAutoString errorMsg("Permission denied to "); + switch(aAction) + { + case nsIXPCSecurityManager::ACCESS_GET_PROPERTY: + errorMsg += "get property "; + break; + case nsIXPCSecurityManager::ACCESS_SET_PROPERTY: + errorMsg += "set property "; + break; + case nsIXPCSecurityManager::ACCESS_CALL_METHOD: + errorMsg += "call method "; + } + + if (aProperty) + { + errorMsg += aClassName; + errorMsg += '.'; + errorMsg += aProperty; + } + else + { + nsXPIDLCString className; + if (aClassInfo) + aClassInfo->GetClassDescription(getter_Copies(className)); + if(className) + errorMsg += className; + else + errorMsg += unknownClassName; + errorMsg += '.'; + errorMsg.AppendWithConversion(JSValIDToString(aJSContext, aName)); + } + + JS_SetPendingException(aJSContext, + STRING_TO_JSVAL(JS_NewStringCopyZ(aJSContext, errorMsg))); + //-- if (aProperty) we were called from somewhere other than xpconnect, so we need to + // tell xpconnect that an exception was thrown. + if (aProperty) + { + nsCOMPtr xpc = do_GetService(nsIXPConnect::GetCID()); + if (xpc) + { + nsCOMPtr xpcCallContext; + xpc->GetCurrentNativeCallContext(getter_AddRefs(xpcCallContext)); + if (xpcCallContext) + xpcCallContext->SetExceptionWasThrown(PR_TRUE); + } + } + } return rv; } @@ -661,6 +712,9 @@ nsScriptSecurityManager::CheckLoadURIFromScript(JSContext *cx, nsIURI *aURI) nsresult nsScriptSecurityManager::GetBaseURIScheme(nsIURI* aURI, char** aScheme) { + if (!aURI) + return NS_ERROR_FAILURE; + nsresult rv; nsCOMPtr uri(aURI); @@ -696,9 +750,27 @@ nsScriptSecurityManager::GetBaseURIScheme(nsIURI* aURI, char** aScheme) if (NS_FAILED(rv)) return rv; } + //-- if uri is an about uri, distinguish 'safe' and 'unsafe' about URIs + static const char aboutScheme[] = "about"; + if(nsCRT::strcasecmp(scheme, aboutScheme) == 0) + *aScheme = PL_strdup(scheme); + { + nsXPIDLCString spec; + if(NS_FAILED(uri->GetSpec(getter_Copies(spec)))) + return NS_ERROR_FAILURE; + const char* page = spec.get() + sizeof(aboutScheme); + if ((PL_strcmp(page, "blank") == 0) || + (PL_strcmp(page, "") == 0) || + (PL_strcmp(page, "mozilla") == 0) || + (PL_strcmp(page, "credits") == 0)) + { + *aScheme = PL_strdup("about safe"); + return *aScheme ? NS_OK : NS_ERROR_OUT_OF_MEMORY; + } + } + *aScheme = PL_strdup(scheme); - NS_ASSERTION(*aScheme, "GetBaseURIScheme returning null scheme"); - return NS_OK; + return *aScheme ? NS_OK : NS_ERROR_OUT_OF_MEMORY; } NS_IMETHODIMP @@ -732,7 +804,7 @@ nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI, } //-- If the schemes don't match, the policy is specified in this table. - enum Action { AllowProtocol, DenyProtocol, PrefControlled, ChromeProtocol, AboutProtocol }; + enum Action { AllowProtocol, DenyProtocol, PrefControlled, ChromeProtocol }; static const struct { const char *name; @@ -742,9 +814,9 @@ nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI, //-- Keep the most commonly used protocols at the top of the list // to increase performance { "http", AllowProtocol }, + { "chrome", ChromeProtocol }, { "file", PrefControlled }, { "https", AllowProtocol }, - { "chrome", ChromeProtocol }, { "mailbox", DenyProtocol }, { "pop", AllowProtocol }, { "imap", DenyProtocol }, @@ -752,7 +824,8 @@ nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI, { "news", AllowProtocol }, { "javascript", AllowProtocol }, { "ftp", AllowProtocol }, - { "about", AboutProtocol }, + { "about safe", AllowProtocol }, + { "about", DenyProtocol }, { "mailto", AllowProtocol }, { "aim", AllowProtocol }, { "data", AllowProtocol }, @@ -764,8 +837,6 @@ nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI, { "res", DenyProtocol } }; - nsXPIDLCString targetSpec; - const char* targetPage; for (unsigned i=0; i < sizeof(protocolList)/sizeof(protocolList[0]); i++) { if (nsCRT::strcasecmp(targetScheme, protocolList[i].name) == 0) @@ -794,16 +865,6 @@ nsScriptSecurityManager::CheckLoadURI(nsIURI *aSourceURI, nsIURI *aTargetURI, (PL_strcmp(sourceScheme, "resource") == 0)) return NS_OK; return ReportErrorToConsole(aTargetURI); - case AboutProtocol: - // Allow loading 'safe' about pages, otherwise deny - if(NS_FAILED(aTargetURI->GetSpec(getter_Copies(targetSpec)))) - return NS_ERROR_FAILURE; - targetPage = targetSpec.get() + sizeof("about:") - 1; - return (PL_strcmp(targetPage, "blank") == 0) || - (PL_strcmp(targetPage, "") == 0) || - (PL_strcmp(targetPage, "mozilla") == 0) || - (PL_strcmp(targetPage, "credits") == 0) ? - NS_OK : ReportErrorToConsole(aTargetURI); case DenyProtocol: // Deny access return ReportErrorToConsole(aTargetURI); @@ -1571,7 +1632,8 @@ nsScriptSecurityManager::RequestCapability(nsIPrincipal* aPrincipal, if (*canEnable == nsIPrincipal::ENABLE_WITH_USER_PERMISSION) { // Prompt user for permission to enable capability. - static PRBool remember = PR_TRUE; + // "Remember This Decision" is unchecked by default. + static PRBool remember = PR_FALSE; nsAutoString query, check; if (NS_FAILED(Localize("EnableCapabilityQuery", query))) return NS_ERROR_FAILURE; @@ -1775,23 +1837,44 @@ nsScriptSecurityManager::CanCreateWrapper(JSContext *aJSContext, if (checkedComponent) checkedComponent->CanCreateWrapper((nsIID *)&aIID, getter_Copies(objectSecurityLevel)); - return CheckXPCPermissions(aJSContext, aObj, objectSecurityLevel, - "Permission denied to create wrapper for object"); + nsresult rv = CheckXPCPermissions(aObj, objectSecurityLevel); + if (NS_FAILED(rv)) + { + //-- Access denied, report an error + nsCAutoString errorMsg("Permission denied to create wrapper for object "); + // XXX get class name + nsXPIDLCString className; + if (aClassInfo) + { + aClassInfo->GetClassDescription(getter_Copies(className)); + if (className) + { + errorMsg += "of class "; + errorMsg += className; + } + } + JS_SetPendingException(aJSContext, + STRING_TO_JSVAL(JS_NewStringCopyZ(aJSContext, errorMsg))); + } + return rv; } NS_IMETHODIMP nsScriptSecurityManager::CanCreateInstance(JSContext *aJSContext, const nsCID &aCID) { - //XXX Special cases needed: exceptions? -#if 0 - char* cidStr = aCID.ToString(); - printf("### CanCreateInstance(%s) ", cidStr); - PR_FREEIF(cidStr); -#endif - - return CheckXPCPermissions(aJSContext, nsnull, nsnull, - "Permission denied to create instance of class"); + nsresult rv = CheckXPCPermissions(nsnull, nsnull); + if (NS_FAILED(rv)) + { + //-- Access denied, report an error + nsCAutoString errorMsg("Permission denied to create instance of class. CID="); + nsXPIDLCString cidStr; + cidStr += aCID.ToString(); + errorMsg.Append(cidStr); + JS_SetPendingException(aJSContext, + STRING_TO_JSVAL(JS_NewStringCopyZ(aJSContext, errorMsg))); + } + return rv; } NS_IMETHODIMP @@ -1804,8 +1887,18 @@ nsScriptSecurityManager::CanGetService(JSContext *aJSContext, PR_FREEIF(cidStr); #endif - return CheckXPCPermissions(aJSContext, nsnull, nsnull, - "Permission denied to get service"); + nsresult rv = CheckXPCPermissions(nsnull, nsnull); + if (NS_FAILED(rv)) + { + //-- Access denied, report an error + nsCAutoString errorMsg("Permission denied to get service. CID="); + nsXPIDLCString cidStr; + cidStr += aCID.ToString(); + errorMsg.Append(cidStr); + JS_SetPendingException(aJSContext, + STRING_TO_JSVAL(JS_NewStringCopyZ(aJSContext, errorMsg))); + } + return rv; } /* void CanAccess (in PRUint32 aAction, in nsIXPCNativeCallContext aCallContext, in JSContextPtr aJSContext, in JSObjectPtr aJSObject, in nsISupports aObj, in nsIClassInfo aClassInfo, in JSVal aName, inout voidPtr aPolicy); */ @@ -1825,10 +1918,8 @@ nsScriptSecurityManager::CanAccess(PRUint32 aAction, } nsresult -nsScriptSecurityManager::CheckXPCPermissions(JSContext *aJSContext, - nsISupports* aObj, - const char* aObjectSecurityLevel, - const char* aErrorMsg) +nsScriptSecurityManager::CheckXPCPermissions(nsISupports* aObj, + const char* aObjectSecurityLevel) { //-- Check for the all-powerful UniversalXPConnect privilege PRBool ok = PR_FALSE; @@ -1873,9 +1964,7 @@ nsScriptSecurityManager::CheckXPCPermissions(JSContext *aJSContext, } } - //-- Access tests failed, so report error - JS_SetPendingException(aJSContext, - STRING_TO_JSVAL(JS_NewStringCopyZ(aJSContext, aErrorMsg))); + //-- Access tests failed return NS_ERROR_DOM_XPCONNECT_ACCESS_DENIED; } diff --git a/mozilla/dom/src/base/nsDOMClassInfo.cpp b/mozilla/dom/src/base/nsDOMClassInfo.cpp index 75a5a5c9629..e45e46a7346 100644 --- a/mozilla/dom/src/base/nsDOMClassInfo.cpp +++ b/mozilla/dom/src/base/nsDOMClassInfo.cpp @@ -1958,19 +1958,6 @@ nsDOMClassInfo::CheckAccess(nsIXPConnectWrappedNative *wrapper, JSContext *cx, nsIXPCSecurityManager::ACCESS_GET_PROPERTY); if (NS_FAILED(rv)) { - // The security manager vetoed access to prop_name on real_obj, - // tell XPConnect about this so that XPConnect doesn't hide this - // error from the caller - nsCOMPtr cnccx; - - sXPConnect->GetCurrentNativeCallContext(getter_AddRefs(cnccx)); - - if (cnccx) { - // Tell XPConnect that an exception was already thrown - - cnccx->SetExceptionWasThrown(PR_TRUE); - } - // Let XPConnect know that the access was not granted. *_retval = PR_FALSE; } @@ -2173,18 +2160,6 @@ nsWindowSH::doCheckWriteAccess(JSContext *cx, JSObject *obj, jsval id, isLocation ? "location" : "scriptglobals", nsIXPCSecurityManager::ACCESS_SET_PROPERTY); - if (NS_SUCCEEDED(rv)) { - return rv; - } - - // Security check failed. The above call set a JS exception. The - // following lines ensure that the exception is propagated. - - nsCOMPtr cnccx; - sXPConnect->GetCurrentNativeCallContext(getter_AddRefs(cnccx)); - if (cnccx) - cnccx->SetExceptionWasThrown(PR_TRUE); - return rv; // rv is from CheckPropertyAccess() } @@ -2223,18 +2198,6 @@ nsWindowSH::doCheckReadAccess(JSContext *cx, JSObject *obj, jsval id, isLocation ? "location" : "scriptglobals", nsIXPCSecurityManager::ACCESS_GET_PROPERTY); - if (NS_SUCCEEDED(rv)) { - return rv; - } - - // Security check failed. The above call set a JS exception. The - // following lines ensure that the exception is propagated. - - nsCOMPtr cnccx; - sXPConnect->GetCurrentNativeCallContext(getter_AddRefs(cnccx)); - if (cnccx) - cnccx->SetExceptionWasThrown(PR_TRUE); - return rv; // rv is from CheckPropertyAccess() } diff --git a/mozilla/dom/src/base/nsGlobalWindow.cpp b/mozilla/dom/src/base/nsGlobalWindow.cpp index 0cc833a3afb..54c4d60eafb 100644 --- a/mozilla/dom/src/base/nsGlobalWindow.cpp +++ b/mozilla/dom/src/base/nsGlobalWindow.cpp @@ -4244,8 +4244,6 @@ NavigatorImpl::Preference() "Navigator", "preferenceinternal", action); if (NS_FAILED(rv)) { - //-- XXX doing the right thing here? Does the exception propagate? - ncc->SetExceptionWasThrown(PR_TRUE); return NS_OK; } diff --git a/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp b/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp index 6082d2aa62a..88d9a44c4cc 100644 --- a/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp +++ b/mozilla/extensions/xmlextras/base/src/nsXMLHttpRequest.cpp @@ -710,15 +710,7 @@ nsXMLHttpRequest::Open(const char *method, const char *url) rv = secMan->CheckConnect(cx, targetURI, "XMLHttpRequest","open"); if (NS_FAILED(rv)) { - // Security check failed. The above call set a JS exception. The - // following lines ensure that the exception is propagated. - - nsCOMPtr xpc(do_GetService(nsIXPConnect::GetCID(), &rv)); - nsCOMPtr cc; - if(NS_SUCCEEDED(rv)) - xpc->GetCurrentNativeCallContext(getter_AddRefs(cc)); - if (cc) - cc->SetExceptionWasThrown(PR_TRUE); + // Security check failed. return NS_OK; }