getting clone-parent of JS function using Array generic methods

RESOLVED FIXED in mozilla1.8rc1

Status

()

Core
JavaScript Engine
P1
normal
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: shutdown, Assigned: brendan)

Tracking

({fixed1.8, js1.6})

Trunk
mozilla1.8rc1
fixed1.8, js1.6
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8rc1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical] not ff1.0/moz1.7)

Attachments

(3 attachments)

(Reporter)

Description

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

Comment 1

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

Comment 2

12 years ago
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
Assignee: general → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8rc1+
Keywords: js1.6
Priority: -- → P1
Target Milestone: --- → mozilla1.8rc1
(Assignee)

Updated

12 years ago
Blocks: 313370
(Assignee)

Comment 3

12 years ago
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
Attachment #200706 - Flags: superreview?(shaver)
Attachment #200706 - Flags: review?(mrbkap)
(Assignee)

Comment 4

12 years ago
Stops this bug's testcase as well as bug 313370's testcase.

/be
Status: NEW → ASSIGNED
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).
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
Whiteboard: [sg:critical]
Blocks: 256195
(Assignee)

Comment 6

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

/be
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
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
Attachment #200706 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 8

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

Comment 9

12 years ago
Created attachment 200713 [details] [diff] [review]
patch that I checked into the trunk
Attachment #200713 - Flags: superreview?(shaver)
Attachment #200713 - Flags: review+
Attachment #200713 - Flags: approval1.8rc1?
(Assignee)

Updated

12 years ago
Attachment #200706 - Flags: superreview?(shaver)
(Reporter)

Comment 10

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

12 years ago
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?
Attachment #200713 - Flags: approval1.8rc1? → approval1.8rc1+
(Assignee)

Comment 12

12 years ago
(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
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Comment on attachment 200713 [details] [diff] [review]
patch that I checked into the trunk

sr=shaver, I like the commonization.
Attachment #200713 - Flags: superreview?(shaver) → superreview+
needs to go in the "security" suite, not the js test library.
Flags: testcase?
shutdown's test will be available as
tests/mozilla.org/security/313684-01.html on the qa farm.
Flags: testcase? → testcase+
Whiteboard: [sg:critical] → [sg:critical] not ff1.0/moz1.7
Group: security

Comment 16

11 years ago
do members of the security group who don't work for mozilla corporation have access to that?
(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.
You need to log in before you can comment on or make changes to this bug.