getting clone-parent of JS function using watchpoint

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: shutdown, Assigned: brendan)

Tracking

({fixed1.8, verified1.7.13})

Trunk
fixed1.8, verified1.7.13
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.13 +
blocking-aviary1.0.8 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

12 years ago
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
*sigh*, another eval exploit.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

12 years ago
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;
}
Summary: getting clone-parent of JS function using eval() → getting clone-parent of JS function using WithObject
(Reporter)

Updated

12 years ago
Attachment #200432 - Attachment is obsolete: true
(Reporter)

Comment 4

12 years ago
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
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.
Attachment #200534 - Flags: review?(brendan)
(Assignee)

Comment 6

12 years ago
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
(Reporter)

Comment 7

12 years ago
(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__
Blocks: 256195
Flags: blocking1.8rc1+
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Whiteboard: [sg:critical]
(Assignee)

Comment 8

12 years ago
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
(Assignee)

Comment 9

12 years ago
Created attachment 200679 [details]
variation of testcase that does not use a with statement

I'll resummarize the bug next.

/be
(Assignee)

Updated

12 years ago
Summary: getting clone-parent of JS function using WithObject → getting clone-parent of JS function using watchpoint
(Assignee)

Updated

12 years ago
Assignee: general → brendan
(Assignee)

Comment 10

12 years ago
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
Attachment #200534 - Flags: review?(brendan)
(Assignee)

Comment 11

12 years ago
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
Attachment #200688 - Flags: superreview?(shaver)
Attachment #200688 - Flags: review?(mrbkap)
Comment on attachment 200688 [details] [diff] [review]
check (and root) only if argv[1] was converted (and use the right name)

r=mrbkap
Attachment #200688 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 13

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

/be
(Assignee)

Updated

12 years ago
Depends on: 313684
(Assignee)

Comment 14

12 years ago
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
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Flags: blocking1.8rc1+
Resolution: --- → FIXED
Comment on attachment 200688 [details] [diff] [review]
check (and root) only if argv[1] was converted (and use the right name)

sr=shaver
Attachment #200688 - Flags: superreview?(shaver) → superreview+

Comment 16

12 years ago
needs to go in the "security" suite, not the js test library.
Flags: testcase?

Comment 17

12 years ago
shutdown's tests will be available as tests/mozilla.org/security/313370-0[12].html on the qa farm.
Flags: testcase? → testcase+

Comment 18

11 years ago
Added fixed1.8 keyword based on comment 14
Keywords: fixed1.8
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).
Keywords: fixed-aviary1.0.8, fixed1.7.13

Comment 20

11 years ago
v ff on 1.7.13 on windows/mac 20060221 builds, moz on 1.7.13 on windows 20060221 build.
Keywords: fixed-aviary1.0.8, fixed1.7.13 → verified-aviary1.0.8, verified1.7.13
Group: security
You need to log in before you can comment on or make changes to this bug.