Last Comment Bug 313684 - getting clone-parent of JS function using Array generic methods
: getting clone-parent of JS function using Array generic methods
Status: RESOLVED FIXED
[sg:critical] not ff1.0/moz1.7
: fixed1.8, js1.6
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8rc1
Assigned To: Brendan Eich [:brendan]
:
Mentors:
Depends on:
Blocks: sbb? 313370
  Show dependency treegraph
 
Reported: 2005-10-24 18:22 PDT by shutdown
Modified: 2006-11-10 12:13 PST (History)
7 users (show)
brendan: blocking1.8rc1+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.56 KB, text/html)
2005-10-24 18:25 PDT, shutdown
no flags Details
proposed general fix (11.43 KB, patch)
2005-10-24 22:46 PDT, Brendan Eich [:brendan]
mrbkap: review+
Details | Diff | Review
patch that I checked into the trunk (11.44 KB, patch)
2005-10-25 01:13 PDT, Brendan Eich [:brendan]
brendan: review+
shaver: superreview+
mtschrep: approval1.8rc1+
Details | Diff | Review

Description shutdown 2005-10-24 18:22:47 PDT
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 2005-10-24 18:25:26 PDT
Created attachment 200691 [details]
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] 2005-10-24 18:34:15 PDT
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] 2005-10-24 22:46:31 PDT
Created attachment 200706 [details] [diff] [review]
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] 2005-10-24 22:47:52 PDT
Stops this bug's testcase as well as bug 313370's testcase.

/be
Comment 5 Daniel Veditz [:dveditz] 2005-10-24 23:36:07 PDT
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] 2005-10-25 00:17:47 PDT
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) (please use needinfo!) 2005-10-25 00:56:13 PDT
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] 2005-10-25 01:03:40 PDT
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] 2005-10-25 01:13:42 PDT
Created attachment 200713 [details] [diff] [review]
patch that I checked into the trunk
Comment 10 shutdown 2005-10-25 01:35:53 PDT
(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 2005-10-25 01:35:58 PDT
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] 2005-10-25 01:47:35 PDT
(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) 2005-10-27 01:34:44 PDT
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:] 2005-12-23 17:34:53 PST
needs to go in the "security" suite, not the js test library.
Comment 15 Bob Clary [:bc:] 2005-12-27 11:01:57 PST
shutdown's test will be available as
tests/mozilla.org/security/313684-01.html on the qa farm.
Comment 16 timeless 2006-07-03 10:34:20 PDT
do members of the security group who don't work for mozilla corporation have access to that?
Comment 17 Bob Clary [:bc:] 2006-07-03 14:07:40 PDT
(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.

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