As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 365527 - JSOP_ARGUMENTS does not set obj register
: JSOP_ARGUMENTS does not set obj register
Status: VERIFIED FIXED
[sg:critical?]
: verified1.8.0.10, verified1.8.1.2
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Igor Bukanov
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-12-31 10:11 PST by Igor Bukanov
Modified: 2007-05-06 01:31 PDT (History)
4 users (show)
jaymoz: blocking1.8.1.2+
jaymoz: blocking1.8.0.10+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
HTML test case that crashes the browser (578 bytes, text/html)
2006-12-31 10:19 PST, Igor Bukanov
no flags Details
Fix v1 (779 bytes, patch)
2006-12-31 10:23 PST, Igor Bukanov
brendan: review+
dveditz: approval1.8.1.2+
dveditz: approval1.8.0.10+
Details | Diff | Splinter Review
js1_5/Function/regress-365527.js (2.74 KB, text/plain)
2007-01-17 03:36 PST, Bob Clary [:bc:]
no flags Details

Description User image Igor Bukanov 2006-12-31 10:11:38 PST
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.
Comment 1 User image Igor Bukanov 2006-12-31 10:19:08 PST
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.
Comment 2 User image Igor Bukanov 2006-12-31 10:23:43 PST
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.
Comment 3 User image Igor Bukanov 2006-12-31 10:25:05 PST
The same problem exists on branches.
Comment 4 User image Igor Bukanov 2006-12-31 10:27:53 PST
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.
Comment 5 User image Brendan Eich [:brendan] 2006-12-31 11:35:42 PST
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
Comment 6 User image Igor Bukanov 2006-12-31 18:47:44 PST
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)
Comment 7 User image Igor Bukanov 2006-12-31 19:07:37 PST
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
Comment 8 User image Daniel Veditz [:dveditz] 2007-01-03 14:43:22 PST
Comment on attachment 250055 [details] [diff] [review]
Fix v1

Approved for 1.8/1.8.0 branches, a=dveditz for drivers
Comment 9 User image Igor Bukanov 2007-01-05 01:46:42 PST
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
Comment 10 User image Igor Bukanov 2007-01-05 01:55:37 PST
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
Comment 11 User image Bob Clary [:bc:] 2007-01-17 03:36:56 PST
Created attachment 251758 [details]
js1_5/Function/regress-365527.js
Comment 12 User image Bob Clary [:bc:] 2007-01-29 09:31:51 PST
verified fixed 1.8.0, 1.8.1, 1.9.0 2007-01-23 win/mac*/linux
Comment 13 User image Bob Clary [:bc:] 2007-05-06 01:31:19 PDT
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-365527.js,v  <--  regress-365527.js
initial revision: 1.1

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