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
:
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 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 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 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 Igor Bukanov 2006-12-31 10:25:05 PST
The same problem exists on branches.
Comment 4 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 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 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 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 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 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 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 Bob Clary [:bc:] 2007-01-17 03:36:56 PST
Created attachment 251758 [details]
js1_5/Function/regress-365527.js
Comment 12 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 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.