From a913a959d2f3a98202d26f60b4f06e188136a18a Mon Sep 17 00:00:00 2001 From: "bent.mozilla%gmail.com" Date: Wed, 29 Aug 2007 00:16:21 +0000 Subject: [PATCH] Bug 304048 - "xpconnect getters/setters don't have principals until after they pass or fail their security check." Patch by jst, sr=bzbarsky, a=jst. --- caps/src/nsScriptSecurityManager.cpp | 44 ++++++++++++------ js/src/xpconnect/idl/nsIXPConnect.idl | 11 +++++ js/src/xpconnect/src/XPCDispConvert.cpp | 1 + js/src/xpconnect/src/nsXPConnect.cpp | 28 ++++++++++- js/src/xpconnect/src/xpcconvert.cpp | 14 ++++-- js/src/xpconnect/src/xpcprivate.h | 7 +++ js/src/xpconnect/src/xpcwrappedjs.cpp | 62 +++++++++++++++++++++++++ 7 files changed, 149 insertions(+), 18 deletions(-) diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 750a3144970..b9401b03dc1 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -493,6 +493,12 @@ nsScriptSecurityManager::CheckObjectAccess(JSContext *cx, JSObject *obj, if (!ssm) return JS_FALSE; + nsCOMPtr native; + nsresult rv = + sXPConnect->GetNativeOfJSObject(cx, obj, + NS_GET_IID(nsISupports), + getter_AddRefs(native)); + // Get the object being accessed. We protect these cases: // 1. The Function.prototype.caller property's value, which might lead // an attacker up a call-stack to a function or another object from @@ -505,11 +511,13 @@ nsScriptSecurityManager::CheckObjectAccess(JSContext *cx, JSObject *obj, // Do the same-origin check -- this sets a JS exception if the check fails. // Pass the parent object's class name, as we have no class-info for it. - nsresult rv = - ssm->CheckPropertyAccess(cx, target, JS_GetClass(cx, obj)->name, id, - (mode & JSACC_WRITE) ? - nsIXPCSecurityManager::ACCESS_SET_PROPERTY : - nsIXPCSecurityManager::ACCESS_GET_PROPERTY); + rv = ssm->CheckPropertyAccessImpl(( (mode & JSACC_WRITE) ? + nsIXPCSecurityManager::ACCESS_SET_PROPERTY : + nsIXPCSecurityManager::ACCESS_GET_PROPERTY ), + nsnull, cx, target, native, nsnull, + nsnull, JS_GET_CLASS(cx, obj)->name, id, + nsnull); + if (NS_FAILED(rv)) return JS_FALSE; // Security check failed (XXX was an error reported?) @@ -771,14 +779,24 @@ nsScriptSecurityManager::CheckPropertyAccessImpl(PRUint32 aAction, nsXPIDLCString objectSecurityLevel; if (checkedComponent) { - nsCOMPtr wrapper; - nsCOMPtr interfaceInfo; - const nsIID* objIID; - rv = aCallContext->GetCalleeWrapper(getter_AddRefs(wrapper)); - if (NS_SUCCEEDED(rv)) - rv = wrapper->FindInterfaceWithMember(aProperty, getter_AddRefs(interfaceInfo)); - if (NS_SUCCEEDED(rv)) - rv = interfaceInfo->GetIIDShared(&objIID); + const nsIID* objIID = nsnull; + if (aCallContext) { + // If we have a call context, find the wrapper and the IID + // with the member in question to pass to + // nsISecurityCheckedComponent, if not, pass a null IID + // and it's up to the implementation to decide whether or + // not it wants to permit access. + nsCOMPtr wrapper; + nsCOMPtr interfaceInfo; + rv = aCallContext->GetCalleeWrapper(getter_AddRefs(wrapper)); + if (NS_SUCCEEDED(rv)) + rv = wrapper->FindInterfaceWithMember(aProperty, getter_AddRefs(interfaceInfo)); + if (NS_SUCCEEDED(rv)) + rv = interfaceInfo->GetIIDShared(&objIID); + } else { + rv = NS_OK; + } + if (NS_SUCCEEDED(rv)) { switch (aAction) diff --git a/js/src/xpconnect/idl/nsIXPConnect.idl b/js/src/xpconnect/idl/nsIXPConnect.idl index a523c7c7c10..69223081936 100644 --- a/js/src/xpconnect/idl/nsIXPConnect.idl +++ b/js/src/xpconnect/idl/nsIXPConnect.idl @@ -674,6 +674,17 @@ interface nsIXPConnect : nsISupports */ void flagSystemFilenamePrefix(in string aFilenamePrefix); + /** + * Get a native pointer of type aIID from aJSObject if the object + * is a wrapped native or a wrapped JS object, but never create a + * new wrapper, only use existing ones. + */ + void + getNativeOfJSObject(in JSContextPtr aJSContext, + in JSObjectPtr aJSObj, + in nsIIDRef aIID, + [iid_is(aIID),retval] out nsQIResult result); + /** * Restore an old prototype for wrapped natives of type * aClassInfo. This should be used only when restoring an old diff --git a/js/src/xpconnect/src/XPCDispConvert.cpp b/js/src/xpconnect/src/XPCDispConvert.cpp index ed94fdf9574..83d0526f9ce 100644 --- a/js/src/xpconnect/src/XPCDispConvert.cpp +++ b/js/src/xpconnect/src/XPCDispConvert.cpp @@ -331,6 +331,7 @@ JSBool XPCDispConvert::JSToCOM(XPCCallContext& ccx, obj, &NSID_IDISPATCH, nsnull, + PR_TRUE, &err)) { // Avoid cleaning up garbage diff --git a/js/src/xpconnect/src/nsXPConnect.cpp b/js/src/xpconnect/src/nsXPConnect.cpp index 91e9f94551d..bd9048dee88 100644 --- a/js/src/xpconnect/src/nsXPConnect.cpp +++ b/js/src/xpconnect/src/nsXPConnect.cpp @@ -1112,7 +1112,7 @@ nsXPConnect::WrapJS(JSContext * aJSContext, nsresult rv; if(!XPCConvert::JSObject2NativeInterface(ccx, result, aJSObj, - &aIID, nsnull, &rv)) + &aIID, nsnull, PR_TRUE, &rv)) return rv; return NS_OK; } @@ -1138,7 +1138,7 @@ nsXPConnect::WrapJSAggregatedToNative(nsISupports *aOuter, nsresult rv; if(!XPCConvert::JSObject2NativeInterface(ccx, result, aJSObj, - &aIID, aOuter, &rv)) + &aIID, aOuter, PR_TRUE, &rv)) return rv; return NS_OK; } @@ -2097,6 +2097,30 @@ nsXPConnect::OnDispatchedEvent(nsIThreadInternal* aThread) return NS_ERROR_UNEXPECTED; } +/* void getNativeOfJSObject(in JSContextPtr aJSContext, in JSObjectPtr aJSObj, in nsIIDRef aIID, [iid_is(aIID),retval] out nsQIResult result); */ +NS_IMETHODIMP +nsXPConnect::GetNativeOfJSObject(JSContext * aJSContext, + JSObject * aJSObj, + const nsIID & aIID, + void * *result) +{ + NS_ASSERTION(aJSContext, "bad param"); + NS_ASSERTION(aJSObj, "bad param"); + NS_ASSERTION(result, "bad param"); + + *result = nsnull; + + XPCCallContext ccx(NATIVE_CALLER, aJSContext); + if(!ccx.IsValid()) + return UnexpectedFailure(NS_ERROR_FAILURE); + + nsresult rv; + if(!XPCConvert::JSObject2NativeInterface(ccx, result, aJSObj, + &aIID, nsnull, PR_FALSE, &rv)) + return rv; + return NS_OK; +} + #ifdef DEBUG /* These are here to be callable from a debugger */ JS_BEGIN_EXTERN_C diff --git a/js/src/xpconnect/src/xpcconvert.cpp b/js/src/xpconnect/src/xpcconvert.cpp index 395983bb626..11c8de8ce65 100644 --- a/js/src/xpconnect/src/xpcconvert.cpp +++ b/js/src/xpconnect/src/xpcconvert.cpp @@ -1030,7 +1030,7 @@ XPCConvert::JSData2Native(XPCCallContext& ccx, void* d, jsval s, } return JSObject2NativeInterface(ccx, (void**)d, obj, iid, - nsnull, pErr); + nsnull, PR_TRUE, pErr); } default: NS_ASSERTION(0, "bad type"); @@ -1230,6 +1230,7 @@ XPCConvert::JSObject2NativeInterface(XPCCallContext& ccx, void** dest, JSObject* src, const nsID* iid, nsISupports* aOuter, + PRBool allowCreateNew, nsresult* pErr) { NS_ASSERTION(dest, "bad param"); @@ -1239,7 +1240,7 @@ XPCConvert::JSObject2NativeInterface(XPCCallContext& ccx, JSContext* cx = ccx.GetJSContext(); *dest = nsnull; - if(pErr) + if(pErr) *pErr = NS_ERROR_XPC_BAD_CONVERT_JS; nsISupports* iface; @@ -1288,8 +1289,15 @@ XPCConvert::JSObject2NativeInterface(XPCCallContext& ccx, // else... + nsresult rv; + nsXPCWrappedJS* wrapper; - nsresult rv = nsXPCWrappedJS::GetNewOrUsed(ccx, src, *iid, aOuter, &wrapper); + if (allowCreateNew) { + rv = nsXPCWrappedJS::GetNewOrUsed(ccx, src, *iid, aOuter, &wrapper); + } else { + rv = nsXPCWrappedJS::GetUsedOnly(ccx, src, *iid, aOuter, &wrapper); + } + if(pErr) *pErr = rv; if(NS_SUCCEEDED(rv) && wrapper) diff --git a/js/src/xpconnect/src/xpcprivate.h b/js/src/xpconnect/src/xpcprivate.h index 767db756651..1e6593c8697 100644 --- a/js/src/xpconnect/src/xpcprivate.h +++ b/js/src/xpconnect/src/xpcprivate.h @@ -2356,6 +2356,12 @@ public: REFNSIID aIID, nsISupports* aOuter, nsXPCWrappedJS** wrapper); + static nsresult + GetUsedOnly(XPCCallContext& ccx, + JSObject* aJSObj, + REFNSIID aIID, + nsISupports* aOuter, + nsXPCWrappedJS** wrapperResult); nsISomeInterface* GetXPTCStub() { return mXPTCStub; } JSObject* GetJSObject() const {return mJSObj;} @@ -2531,6 +2537,7 @@ public: void** dest, JSObject* src, const nsID* iid, nsISupports* aOuter, + PRBool allowCreateNew, nsresult* pErr); /** diff --git a/js/src/xpconnect/src/xpcwrappedjs.cpp b/js/src/xpconnect/src/xpcwrappedjs.cpp index 32cd5edcf71..f6505649a30 100644 --- a/js/src/xpconnect/src/xpcwrappedjs.cpp +++ b/js/src/xpconnect/src/xpcwrappedjs.cpp @@ -417,6 +417,68 @@ return_wrapper: return NS_OK; } +// static +nsresult +nsXPCWrappedJS::GetUsedOnly(XPCCallContext& ccx, + JSObject* aJSObj, + REFNSIID aIID, + nsISupports* aOuter, + nsXPCWrappedJS** wrapperResult) +{ + JSObject2WrappedJSMap* map; + JSBool hasProp; + JSObject* rootJSObj; + nsXPCWrappedJS* root; + nsXPCWrappedJS* wrapper = nsnull; + nsXPCWrappedJSClass *clazz = nsnull; + XPCJSRuntime* rt = ccx.GetRuntime(); + + map = rt->GetWrappedJSMap(); + if(!map) + { + NS_ASSERTION(map,"bad map"); + return NS_ERROR_FAILURE; + } + + nsXPCWrappedJSClass::GetNewOrUsed(ccx, aIID, &clazz); + if(!clazz) + return NS_ERROR_FAILURE; + + // GetRootJSObject will attempt to call a QueryInterface function on + // aJSObj. If QueryInterface doesn't exist on the object then a strict + // warning will be emitted, so check to make sure that the QueryInterface + // function exists before proceeding. + if(JS_HasProperty(ccx.GetJSContext(), aJSObj, + rt->GetStringName(XPCJSRuntime::IDX_QUERY_INTERFACE), + &hasProp) && hasProp) + rootJSObj = clazz->GetRootJSObject(ccx, aJSObj); + else + rootJSObj = aJSObj; + + NS_RELEASE(clazz); + + if(!rootJSObj) + return NS_ERROR_FAILURE; + + // look for the root wrapper + { // scoped lock + XPCAutoLock lock(rt->GetMapLock()); + root = map->Find(rootJSObj); + } + + if(root) + { + if((nsnull != (wrapper = root->Find(aIID))) || + (nsnull != (wrapper = root->FindInherited(aIID)))) + { + NS_ADDREF(wrapper); + } + } + + *wrapperResult = wrapper; + return NS_OK; +} + nsXPCWrappedJS::nsXPCWrappedJS(XPCCallContext& ccx, JSObject* aJSObj, nsXPCWrappedJSClass* aClass,