JSOP_ARGUMENTS does not set obj register

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

({verified1.8.0.10, verified1.8.1.2})

Trunk
verified1.8.0.10, verified1.8.1.2
Points:
---
Bug Flags:
blocking1.8.1.2 +
blocking1.8.0.10 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments)

(Assignee)

Description

11 years ago
The interpreter case for JSOP_ARGUMENTS does not set obj register. As such using arguments() in a function body will push the older value of obj register to the stack. 

This is exploitable AFAICS as a script can organize to GC the old value and override arguments with an arbitrary function that would see as its this value GC-ed object.
(Assignee)

Comment 1

11 years ago
Created attachment 250054 [details]
HTML test case that crashes the browser

The test case "hopes" that the branch callback calls the GC at the right moment after executing obj.x getter which unroots the old obj value stored in the register. To see the crash it may be necessary to click continue in the slow script alert.
Whiteboard: [sg:critical?]
(Assignee)

Comment 2

11 years ago
Created attachment 250055 [details] [diff] [review]
Fix v1

The fix sets obj to NULL as arguments should be treated as any other var declared in a function at the call site.
Attachment #250055 - Flags: review?(brendan)
(Assignee)

Comment 3

11 years ago
The same problem exists on branches.
Status: NEW → ASSIGNED
Flags: blocking1.8.1.2?
Flags: blocking1.8.0.10?
(Assignee)

Comment 4

11 years ago
Comment on attachment 250055 [details] [diff] [review]
Fix v1

The patch applies to the branches, 1.8.0 requires a trivial merger as the patch is not applicable there as-is due to too much context.
Attachment #250055 - Flags: approval1.8.1.2?
Attachment #250055 - Flags: approval1.8.0.10?
Comment on attachment 250055 [details] [diff] [review]
Fix v1

r=me, should go into branches ASAP as it is a totally safe fix. Thanks,

/be
Attachment #250055 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

11 years ago
Here is a test case for jsshell to demonstrate the problem:

~/m/files> cat y1.js 
function gc_obj()
{
  obj = null;
  gc();
}

var obj = { x: { valueOf: gc_obj }};

function f()
{
  arguments = function() { this.something };
  // obj.x stores obj in the register and +obj.x calls gc_obj 
  // as valueOf method. That GC the register value.
  +obj.x;
  arguments();
}

f();
~/m/files> ~/m/trunk/mozilla/js/src/Linux_All_DBG.OBJ/js  y1.js
before 27696, after 27696, break 0812a000
Segmentation fault (core dumped)
(Assignee)

Comment 7

11 years ago
I committed the patch from comment 2 to the trunk:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.316; previous revision: 3.315
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Comment on attachment 250055 [details] [diff] [review]
Fix v1

Approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #250055 - Flags: approval1.8.1.2?
Attachment #250055 - Flags: approval1.8.1.2+
Attachment #250055 - Flags: approval1.8.0.10?
Attachment #250055 - Flags: approval1.8.0.10+

Updated

11 years ago
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10?
Flags: blocking1.8.0.10+
(Assignee)

Comment 9

11 years ago
I committed the patch from comment 2 to MOZILLA_1_8_BRANCH:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.79; previous revision: 3.181.2.78
done
Keywords: fixed1.8.1.2
(Assignee)

Comment 10

11 years ago
I committed the patch from comment 2 to MOZILLA_1_8_0_BRANCH:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.17.2.25; previous revision: 3.181.2.17.2.24
done
Keywords: fixed1.8.0.10

Comment 11

11 years ago
Created attachment 251758 [details]
js1_5/Function/regress-365527.js

Updated

11 years ago
Flags: in-testsuite+

Comment 12

11 years ago
verified fixed 1.8.0, 1.8.1, 1.9.0 2007-01-23 win/mac*/linux
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.10, fixed1.8.1.2 → verified1.8.0.10, verified1.8.1.2
Group: security

Comment 13

10 years ago
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-365527.js,v  <--  regress-365527.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.