Here is unrooted object access found in Array.prototype implementation in jsarray.c 1. array_slice does not root the array for slice results with trivial demonstration of segmentation fault in jsshell: function index_getter() { gc(); return 100; } var a = [0, 1]; a.__defineGetter__(0, index_getter); uneval(a.slice(0, 1)); To make this to crash the browser just replace "gc();" by for (var i = 0; i != 1 << 15; ++i) new Object(); 2. array_reverse does not root v and v2 indexes that it use to hold reversing values. Here is an example for jsshell that demonstrates how to expose 64 bits that holds JSString value into a script variable: var subverted = 0; function index_getter() { delete a[0]; gc(); for (var i = 0; i != 1 << 14; ++i) { var tmp = new String("test"); tmp = null; } return 1; } function index_setter(value) { subverted = value; } var a = [ Math.sqrt(2), 0 ]; a.__defineGetter__(1, index_getter); a.__defineSetter__(1, index_setter); a.reverse(); print(subverted) On my Linux box it prints 1.3303688272434068e-266 which is assembled from JSString bits for 'new String("test");' interpreted as double value. In a similar way one can make a GC thing that JS considers to be JSString from arbitrary double value which could potentially allow to read arbitrary memory context into scripts variables via this pseudo-string. Due to this potential exposure I mark this bug as security-restricted.

I am not 100% sure that I can use argv[argc] as a space for the second local root in array_reverse. Is it OK?

The previois fix should move, not copy the line with *rval = OBJECT_TO_JSVAL(nobj) to the beginning of the function. So here is removal of the extra assignment.

Thanks, Igor. Another old bug that can be safely fixed for 1.8. mrbkap, can you drive this one in? /be

Comment on attachment 199013 [details] [diff] [review] Fix 2 To use argv[argc] as a local root, you need to reserve one local root by changing {"reverse", array_reverse, 0,JSFUN_GENERIC_NATIVE,0}, to have 1 instead of 0 in the last member initializer. /be

I used 2 extra local roots to swap temporaries for less hackish code and added that 2 into array_methods declaration. More hackish version would use argv[-2] for the second local root, right?

Comment on attachment 199024 [details] [diff] [review] Fix 3 Great, thanks. I will rely on mrbkap to do a second review and help get this checked in. /be

Comment on attachment 199024 [details] [diff] [review] Fix 3 r=mrbkap

> [sg:low] memory read at least, maybe more Unfortunately it is the same type of problem as in bug 311497 and can be used for arbitrary code execution, see comment 10 there for details.

Blake, would you shepherd this to fixed resolution? Thanks, /be

Fix checked into trunk.

(In reply to comment #5) > Created an attachment (id=199024) [edit] > Fix 3 > > I used 2 extra local roots to swap temporaries for less hackish code and added > that 2 into array_methods declaration. > > More hackish version would use argv[-2] for the second local root, right? That would be the callee function object, but it might be bad to overwrite it, if you use an in-runtime debugger that might be confused. /be

Fix checked into MOZILLA_1_8_BRANCH. Reopening to give to Igor.

verified firefox 1.5 rc2 linux/win32 2005-11-07

testcase+ to get this off my radar. when this is made public, i will check in the test.

Fix checked into the 1.7 branches.

js1_5/Array/regress-311792-01.js, js1_5/Array/regress-311792-02.js verified no crash in automatic tests of firefox nightlies 1.7.13, 1.8.0.1, 1.8, 1.9a1 on windows/linux/mac. Note that 1.7.5 and 1.7.12 did crash. For mozilla 1.7.13 on winxp running the tests in the browser_however_ I was able to get a nightly Mozilla 1.7.13 from 20060213 to crash by clicking and attempting to edit the url in the urlbar back and forth from -01 to -02 however it did not crash immediately. I could not reproduce with a debug firefox trunk build from 20060214. I was able to hit an assert in Firefox 1.0.8 debug build from 20060214 though. <http://test.mozilla.com/tests/mozilla.org/js/js-test-driver-standards.html?test=js1_5/Array/regress-311792-01.js;language=language;javascript> Assertion failure: flags != GCF_FINAL, at c:/work/mozilla/builds/ff/1.0.x/mozilla/js/src/jsgc.c:835 + cx 0x03bb08c0 thing 0x03baf368 arg 0x00000000 + vp 0x03c734e8 + flagp 0x03bae225 "" + obj 0x03baeba8 nslots 0x00000005 v 0x80000001 + end 0x03c734e8 + rt 0x00ec9e78 + str 0x03bae9b0 flags 0x10 '' js_MarkGCThing(JSContext * 0x03bb08c0, void * 0x03baf368, void * 0x00000000) line 835 + 35 bytes js_MarkGCThing(JSContext * 0x03bb08c0, void * 0x03bae520, void * 0x00000000) line 919 + 18 bytes js_MarkGCThing(JSContext * 0x03bb08c0, void * 0x03b2a360, void * 0x00000000) line 919 + 18 bytes js_MarkGCThing(JSContext * 0x03bb08c0, void * 0x03b2ad50, void * 0x00000000) line 919 + 18 bytes gc_root_marker(JSDHashTable * 0x00ec9e98, JSDHashEntryHdr * 0x03b8786c, unsigned long 0x000001e6, void * 0x03bb08c0) line 975 + 18 bytes JS_DHashTableEnumerate(JSDHashTable * 0x00ec9e98, int (JSDHashTable *, JSDHashEntryHdr *, unsigned long, void *)* 0x10042cb0 gc_root_marker(JSDHashTable *, JSDHashEntryHdr *, unsigned long, void *), void * 0x03bb08c0) line 618 + 34 bytes js_GC(JSContext * 0x03bb08c0, unsigned int 0x00000000) line 1189 + 21 bytes js_ForceGC(JSContext * 0x03bb08c0, unsigned int 0x00000000) line 1000 + 13 bytes JS_GC(JSContext * 0x03bb08c0) line 1698 + 11 bytes nsJSContext::Notify(nsJSContext * const 0x03bb0720, nsITimer * 0x0344b058) line 1859 + 13 bytes nsTimerImpl::Fire() line 386 nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x0316f7f0) line 616 nsAppShell::Run(nsAppShell * const 0x02dcd6f8) line 142 nsAppShellService::Run(nsAppShellService * const 0x02dcd638) line 495 xre_main(int 0x00000004, char * * 0x003e6e08, const nsXREAppData * 0x0041e01c kAppData) line 1907 + 35 bytes main(int 0x00000004, char * * 0x003e6e08) line 58 + 18 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 7c816d4f() I think this is fixed, but leaving unverified on the 1.7 branch for now until someone looks at the stack.

v ff 1.0.8, 1.5.0.1, 1.5, 1.6a1 20060217 win/linux/mac, mozilla 1.7.13 winxp.

RCS file: /cvsroot/mozilla/js/tests/js1_5/GC/regress-311792-01.js,v done Checking in regress-311792-01.js; /cvsroot/mozilla/js/tests/js1_5/GC/regress-311792-01.js,v <-- regress-311792-01.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_5/GC/regress-311792-02.js,v done Checking in regress-311792-02.js; /cvsroot/mozilla/js/tests/js1_5/GC/regress-311792-02.js,v <-- regress-311792-02.js initial revision: 1.1 done