Fix for bug 470720 can be circumvented since XPC_NW_RewrapIfDeepWrapper unwraps XOW. http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/XPCNativeWrapper.cpp#347

This fixes both bugs by making XPCNativeWrapper do security checks, like the other two wrappers.

The previous patch left out the meat of the code in EnsureLegalActivity.

Comment on attachment 356587 [details] [diff] [review] Betterer fix - In EnsureLegalActivity(): + PRBool isPrivileged = PR_FALSE; + if (NS_SUCCEEDED(ssm->IsSystemPrincipal(subjectPrincipal, &isPrivileged)) && + isPrivileged) { + // Chrome code can do anything. + return JS_TRUE; + } + + // Alternatively, this might be content code with UniversalXPConnect. + void *annotation = JS_GetFrameAnnotation(cx, fp); + nsresult rv = subjectPrincipal->IsCapabilityEnabled("UniversalXPConnect", + annotation, + &isPrivileged); IsCapabilityEnabled() always returns true if you're chrome (as you said yourself :), so you should be able to remove the above IsSystemPrincipal() check. - In a/js/src/xpconnect/src/xpcwrappednative.cpp + XPCWrappedNative* wrapper; + if(!XPCNativeWrapper::GetWrappedNative(cx, cur, &wrapper)) + JS_ClearPendingException(cx); Nothing in there throws, right? So no need to clear exceptions here. r+sr=jst

With jst's comments addressed: diff --git a/js/src/xpconnect/src/XPCNativeWrapper.cpp b/js/src/xpconnect/src/XPCNativeWrapper.cpp --- a/js/src/xpconnect/src/XPCNativeWrapper.cpp +++ b/js/src/xpconnect/src/XPCNativeWrapper.cpp @@ -204,25 +204,19 @@ EnsureLegalActivity(JSContext *cx, JSObj JSStackFrame *fp; nsIPrincipal *subjectPrincipal = ssm->GetCxSubjectPrincipalAndFrame(cx, &fp); if (!subjectPrincipal || !fp) { // We must allow the access if there is no code running. return JS_TRUE; } + // This might be chrome code or content code with UniversalXPConnect. + void *annotation = JS_GetFrameAnnotation(cx, fp); PRBool isPrivileged = PR_FALSE; - if (NS_SUCCEEDED(ssm->IsSystemPrincipal(subjectPrincipal, &isPrivileged)) && - isPrivileged) { - // Chrome code can do anything. - return JS_TRUE; - } - - // Alternatively, this might be content code with UniversalXPConnect. - void *annotation = JS_GetFrameAnnotation(cx, fp); nsresult rv = subjectPrincipal->IsCapabilityEnabled("UniversalXPConnect", annotation, &isPrivileged); if (NS_SUCCEEDED(rv) && isPrivileged) { return JS_TRUE; } // We're in unprivileged code, ensure that we're allowed to access the diff --git a/js/src/xpconnect/src/xpcwrappednative.cpp b/js/src/xpconnect/src/xpcwrappednative.cpp --- a/js/src/xpconnect/src/xpcwrappednative.cpp +++ b/js/src/xpconnect/src/xpcwrappednative.cpp @@ -1440,19 +1440,17 @@ return_tearoff: if(clazz == &sXPC_XOW_JSClass.base && (unsafeObj = XPCWrapper::Unwrap(cx, cur))) return GetWrappedNativeOfJSObject(cx, unsafeObj, funobj, pobj2, pTearOff); if(XPCNativeWrapper::IsNativeWrapperClass(clazz)) { XPCWrappedNative* wrapper; - if(!XPCNativeWrapper::GetWrappedNative(cx, cur, &wrapper)) - JS_ClearPendingException(cx); - else if(wrapper) + if(XPCNativeWrapper::GetWrappedNative(cx, cur, &wrapper) && wrapper) return GetWrappedNativeOfJSObject(cx, wrapper->GetFlatJSObject(), funobj, pobj2, pTearOff); } if(IsXPCSafeJSObjectWrapperClass(clazz) && (unsafeObj = STOBJ_GET_PARENT(cur))) return GetWrappedNativeOfJSObject(cx, unsafeObj, funobj, pobj2, pTearOff);

I checked this in but had to back it out because XPCNativeWrapper::GetWrappedNative doesn't deal with UniversalXPConnect scripts properly.

(http://hg.mozilla.org/mozilla-central/rev/5d3af3ff9639 was the checkin and http://hg.mozilla.org/mozilla-central/rev/cd3ec07611fc was the backout).

This has r+sr=jst in person. I had to push null around one of the calls in the window.open() chain since the current JS can't actually touch the brand-new window until it's loaded.

http://hg.mozilla.org/mozilla-central/rev/122cf69f6692 and http://hg.mozilla.org/mozilla-central/rev/427d147720e3 landed to fix this.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/61beb1b0e247 (http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d9ac8c9a8346 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/130653abfd3f) fixed this on 1.9.1.