508752 - COW can be circumvented

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[moz_bug_r_a4]

feedWriter.onPageChanged({}, 0, obj);

When obj is converted to a string, obj.toString method can access the original
native onPageChanged method.

Comment 1

[moz_bug_r_a4]

Attachment: testcase - arbitrary code execution

This uses bug 344495's trick.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

This closes the hole in wrapperization by not exposing the caller of a function that was called through a SJOW.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/577258550954

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

I had to back this out. It caused mochitest-chrome failure.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Passing mochichrome

This works, but is there a better way?

diff --git a/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp b/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
--- a/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
+++ b/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp
@@ -527,17 +527,20 @@ static inline JSBool
 CallWithoutStatics(JSContext *cx, JSObject *obj, jsval fval, uintN argc,
                    jsval *argv, jsval *rval)
 {
   JSRegExpStatics statics;
   JSTempValueRooter tvr;
   js_SaveRegExpStatics(cx, &statics, &tvr);
   JS_ClearRegExpStatics(cx);
   JSStackFrame *fp = JS_SaveFrameChain(cx);
+  uint32 options =
+    JS_SetOptions(cx, JS_GetOptions(cx) | JSOPTION_DONT_REPORT_UNCAUGHT);
   JSBool ok = ::JS_CallFunctionValue(cx, obj, fval, argc, argv, rval);
+  JS_SetOptions(cx, options);
   JS_RestoreFrameChain(cx, fp);
   js_RestoreRegExpStatics(cx, &statics, &tvr);
   return ok;
 }
 
 // Call wrapper to help with wrapping calls to functions or callable
 // objects in a scripted function (see XPC_SJOW_Call()). The first
 // argument passed to this method is the unsafe function to call, the

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 393709 [details] [diff] [review]
Passing mochichrome

Brendan, I'd appreciate your comments on this, as well.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 393709 [details] [diff] [review]
Passing mochichrome

That seems necessary and minimal (new API could make it smaller yet -- a followup bug if necessary, but we could wait until there's a second use-case in the wild).

/be

Comment 8

[moz_bug_r_a4]

The proposed fix makes SJOW exploitable.  With the fix, when an untrusted
function is called through XPC_SJOW_CallWrapper, there is no running script on
the stack, and thus a subject principal is computed from the global object of
the context that the function is running on.  (When an untrusted function is
called through XPC_SJOW_GetOrSetProperty or XPC_SJOW_toString, there is a
scripted caller created by SJOW on the stack properly.)

Comment 9

[moz_bug_r_a4]

Attachment: testcase 2 - arbitrary code execution

This works with "Passing mochichrome" patch.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix for that too

I stared at this for a while and could not figure out what XPC_SJOW_CallWrapper gains us. All it does is check for less than 1 arg and strip off the first parameter (which is the actual function we want to call). We don't have to worry about the regexp static stuff because this is already on the right side of the firewall: we've already made the transition to unprivileged code.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/3b7735ec7bbd

Comment 12

[Daniel Veditz [:dveditz]]

arg, this should have been a 1.9.2 blocker. Didn't notice it just missed the branch.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Applying to the 1.9.2 branch

This is ready to be pushed to the 1.9.2 branch.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

This was fixed by the patch I checked in for bug 504021.