313370 - getting clone-parent of JS function using watchpoint

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[shutdown]

eval() can be used to get clone-parent of JS function. see the testcase.

Comment 1

[shutdown]

Attachment: 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) (inactive)]

*sigh*, another eval exploit.

Comment 3

[shutdown]

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]

Attachment: 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) (inactive)]

Attachment: 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]]

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]

(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]]

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]]

Attachment: variation of testcase that does not use a with statement

I'll resummarize the bug next.

/be

Comment 10

[Brendan Eich [:brendan]]

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]]

Attachment: 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) (inactive)]

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]]

General solution coming up in bug 313684, may want to wait for that.

/be

Comment 14

[Brendan Eich [:brendan]]

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 emeritus)]

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] (inactive)]

needs to go in the "security" suite, not the js test library.

Comment 17

[Bob Clary [:bc] (inactive)]

shutdown's tests will be available as tests/mozilla.org/security/313370-0[12].html on the qa farm.

Comment 18

[Tim Riley [:timr]]

Added fixed1.8 keyword based on comment 14

Comment 19

[Daniel Veditz [:dveditz]]

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] (inactive)]

v ff on 1.7.13 on windows/mac 20060221 builds, moz on 1.7.13 on windows 20060221 build.