Last Comment Bug 313370 - getting clone-parent of JS function using watchpoint
: getting clone-parent of JS function using watchpoint
Status: RESOLVED FIXED
[sg:critical]
: fixed1.8, verified1.7.13
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Brendan Eich [:brendan]
:
:
Mentors:
Depends on: 313684
Blocks: sbb?
  Show dependency treegraph
 
Reported: 2005-10-22 05:06 PDT by shutdown
Modified: 2006-11-10 12:13 PST (History)
7 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.41 KB, text/html)
2005-10-22 05:07 PDT, shutdown
no flags Details
new testcase (1.43 KB, text/html)
2005-10-23 04:27 PDT, shutdown
no flags Details
shot in the dark (1.51 KB, patch)
2005-10-23 11:44 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
variation of testcase that does not use a with statement (1.49 KB, text/html)
2005-10-24 16:52 PDT, Brendan Eich [:brendan]
no flags Details
check (and root) only if argv[1] was converted (and use the right name) (2.00 KB, patch)
2005-10-24 17:43 PDT, Brendan Eich [:brendan]
mrbkap: review+
shaver: superreview+
Details | Diff | Splinter Review

Description shutdown 2005-10-22 05:06:36 PDT
eval() can be used to get clone-parent of JS function. see the testcase.
Comment 1 shutdown 2005-10-22 05:07:40 PDT
Created attachment 200432 [details]
testcase

Works on:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051021 Firefox/1.6a1
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b5) Gecko/20051021 Firefox/1.5
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.12) Gecko/20051021 Firefox/1.0.7
Comment 2 Blake Kaplan (:mrbkap) 2005-10-23 02:23:12 PDT
*sigh*, another eval exploit.
Comment 3 shutdown 2005-10-23 04:13:42 PDT
Oops. Actual problem is WithObject but not eval().

function getCloneParent(clonedFunction) {
  var withObject;
  with(clonedFunction) {
    withObject = function(){}.__parent__;
  }
  var cloneParent;
  ({}).watch({ toString: function() {
    cloneParent = arguments.callee.caller.arguments[1];
  } }, withObject);
  return cloneParent;
}
Comment 4 shutdown 2005-10-23 04:27:49 PDT
Created attachment 200495 [details]
new testcase

Works on:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051022 Firefox/1.6a1
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b5) Gecko/20051022 Firefox/1.5
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.12) Gecko/20051021 Firefox/1.0.7
Comment 5 Blake Kaplan (:mrbkap) 2005-10-23 11:44:29 PDT
Created attachment 200534 [details] [diff] [review]
shot in the dark

I haven't fully wrapped my head around this exploit yet, but here's a patch that stops both attachments in their tracks.
Comment 6 Brendan Eich [:brendan] 2005-10-23 19:01:00 PDT
Not sure |with| is required, even.  The cloned function object has a __proto__ that is parented by the XBL compilation scope.  Would this work just as well?

  ({}).watch({ toString: function() {
    cloneParent = arguments.callee.caller.arguments[1];
  } }, clonedFunction.__proto__.__parent__);

?

/be
Comment 7 shutdown 2005-10-23 19:50:20 PDT
(In reply to comment #6)
> Not sure |with| is required, even.  The cloned function object has a __proto__
> that is parented by the XBL compilation scope.  Would this work just as well?

Error: uncaught exception: Permission denied to get property Function.__proto__
Comment 8 Brendan Eich [:brendan] 2005-10-24 01:02:04 PDT
shutdown: brilliant testcase, it exploits:

- The "triangle diagram of doom" between cloned "child" function object, clone "parent" linked from clone "child" via __proto__, the shared JSFunction private data struct and its fun->object, which points to the clone parent.

This arose out of cloned function objects being added years after the primordial fun->object <=> fun relation.

- Resulting clone parent becomes accessible via the local explicit GC root (argv[1]) in obj_watch, available via arguments.callee.caller.arguments[1] -- brilliant!

- Thus bypassing explicit access checks for __proto__.

I would like to fix this by avoiding going the wrong way up the triangle (the fun->object alias for __proto__):

  clone-parent---private-------+
       ^     ^                 |
       |     |                 |
   __proto__  \_fun->object_   |
       |                    \  |
       |                     \ v
  clone-child---private----> fun

How best to do that will require more thinking than I can do at this late hour.

/be
Comment 9 Brendan Eich [:brendan] 2005-10-24 16:52:46 PDT
Created attachment 200679 [details]
variation of testcase that does not use a with statement

I'll resummarize the bug next.

/be
Comment 10 Brendan Eich [:brendan] 2005-10-24 17:36:06 PDT
Comment on attachment 200534 [details] [diff] [review]
shot in the dark

Close, you hit it in the leg.  I'll finish it off next. ;-)

/be
Comment 11 Brendan Eich [:brendan] 2005-10-24 17:43:26 PDT
Created attachment 200688 [details] [diff] [review]
check (and root) only if argv[1] was converted (and use the right name)

The bad indirect call is to the function that's hidden in the proto chain (or whereever js_ValueToFunction/OBJ_DEFAULT_VALUE might hide it), so use that function's name.

While testing OBJ_DEFAULT_VALUE here and elsewhere I noticed it causes a silent failure, easily fixed.

/be
Comment 12 Blake Kaplan (:mrbkap) 2005-10-24 18:10:05 PDT
Comment on attachment 200688 [details] [diff] [review]
check (and root) only if argv[1] was converted (and use the right name)

r=mrbkap
Comment 13 Brendan Eich [:brendan] 2005-10-24 18:35:09 PDT
General solution coming up in bug 313684, may want to wait for that.

/be
Comment 14 Brendan Eich [:brendan] 2005-10-25 01:58:11 PDT
Fixed for 1.8 and trunk by fix for bug 313684.  Removing nominations, leaving this for the 1.0.x and 1.7.x branches.

/be
Comment 15 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-10-27 01:31:15 PDT
Comment on attachment 200688 [details] [diff] [review]
check (and root) only if argv[1] was converted (and use the right name)

sr=shaver
Comment 16 Bob Clary [:bc:] 2005-12-23 17:34:43 PST
needs to go in the "security" suite, not the js test library.
Comment 17 Bob Clary [:bc:] 2005-12-27 10:55:31 PST
shutdown's tests will be available as tests/mozilla.org/security/313370-0[12].html on the qa farm.
Comment 18 Tim Riley [:timr] 2006-02-13 16:53:38 PST
Added fixed1.8 keyword based on comment 14
Comment 19 Daniel Veditz [:dveditz] 2006-02-14 00:23:59 PST
patch checked in to aviary101/moz17 branches (only first chunk; second chunk didn't apply and looks like it was fixing a regression from a later change).
Comment 20 Bob Clary [:bc:] 2006-02-22 02:53:05 PST
v ff on 1.7.13 on windows/mac 20060221 builds, moz on 1.7.13 on windows 20060221 build.

Note You need to log in before you can comment on or make changes to this bug.