371292 - Crash [@ js_AtomToPrintableString]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

This crash occurred about 5 times last night (out of about a hundred tests related to bug 326633).  I can't reproduce it on demand so I can't make a reduced testcase.

Regression from bug 365869?

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000009

Thread 0 Crashed:
0   libmozjs.dylib           	0x01023ff0 js_AtomToPrintableString + 15 (jsatom.c:63)
1   libmozjs.dylib           	0x0105e5fb js_CheckRedeclaration + 740 (jsinterp.c:1814)
2   libmozjs.dylib           	0x01074fdf js_Interpret + 91003 (jsinterp.c:5290)
3   libmozjs.dylib           	0x0105d41a js_Invoke + 3423 (jsinterp.c:1367)
4   libmozjs.dylib           	0x0105d7b0 js_InternalInvoke + 309 (jsinterp.c:1442)
5   libmozjs.dylib           	0x0101d53b JS_CallFunctionValue + 60 (jsapi.c:4368)
6   libgklayout.dylib        	0x18781e2a nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) + 1228 (nsJSEnvironment.cpp:1774)
7   libgklayout.dylib        	0x187a4574 nsGlobalWindow::RunTimeout(nsTimeout*) + 1778 (nsGlobalWindow.cpp:6772)
8   libgklayout.dylib        	0x187a4a62 nsGlobalWindow::TimerCallback(nsITimer*, void*) + 54 (nsGlobalWindow.cpp:7103)
9   libxpcom_core.dylib      	0x0134c7f4 nsTimerImpl::Fire() + 892 (nsTimerImpl.cpp:384)
10  libxpcom_core.dylib      	0x0134c9a1 nsTimerEvent::Run() + 191 (nsTimerImpl.cpp:458)
11  libxpcom_core.dylib      	0x01348a58 nsThread::ProcessNextEvent(int, int*) + 556 (nsThread.cpp:483)
...

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Oops, this is all me.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Oh, and the reduced testcase is:
js> ({1:1,1:2})
typein:1: strict warning: redeclaration of property 1

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 256066 [details] [diff] [review]
Fix

Yoiks. Should have seen this too. Did I ever file that bug on eliminating jsid by making it be jsval?

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.

Comment 6

[Bob Clary [:bc] (inactive)]

Back in the day, I couldn't reproduce the crash in this bug but I've added the reduced testcase in comment 3 to 

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-365869.js,v  <--  regress-365869.js
new revision: 1.2; previous revision: 1.1

Comment 7

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 20070320 win/mac*/linux

Comment 9

[Igor Bukanov]

The bug exists on the 1.8 branch and leads to branch crashes with the following example:

"012345".__defineSetter__(5, function(){});

Comment 10

[Igor Bukanov]

I nominate this bug to 1.8.1.15 as the bug 431409 (marked as a dup of this bug) is blocking .15 release.

Comment 11

[Igor Bukanov]

Comment on attachment 256066 [details] [diff] [review]
Fix

The patch applies as-is to the 1.8 branch.

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 256066 [details] [diff] [review]
Fix

Approved for 1.8.1.15, a=dveditz for release-drivers

Comment 13

[Daniel Veditz [:dveditz]]

Is this really a regression? It appears to affect the 1.8 branch as well, but the proposed regressing bug 365869 didn't land there.

Comment 14

[Daniel Veditz [:dveditz]]

And how bad is it as a security bug? so far we've only seen accesses (not execution) of small offsets from null.

Comment 15

[Igor Bukanov]

(In reply to comment #14)
> And how bad is it as a security bug? so far we've only seen accesses (not
> execution) of small offsets from null.

Just replace the number 5 in the example from the bug 431409 (also duplicated in the comment 9 here) by any other number and the access can be made at the arbitrary odd address from 0..2**32 range. Since the code then accesses a word at that address that is feed to js_ValueToPrintableString, one can prepare a pointer there that resembles JSObject* with the toString converter pointing to a script-prepared native code. 

Comment 16

[Daniel Veditz [:dveditz]]

I'd like to say that sounds impossible to exploit in practice, but given recent amazingly hacks -- like http://www.matasano.com/log/1032/this-new-vulnerability-dowds-inhuman-flash-exploit/ -- I wouldn't want to stake my life on it.

Comment 17

[Daniel Veditz [:dveditz]]

mrbkap's out for the month and I can't check in to mozilla/js -- Igor, Gavin or Crowder, could any of you check this in on the 1.8 branch please? Thanks

Comment 18

[Daniel Veditz [:dveditz]]

I can verify that the patch in the bug does nicely stop the crash.

Comment 19

[Jeff Walden [:Waldo]]

random-three-o-eight:~/moz/branch18/mozilla/js/src jwalden$ cvs st jsinterp.c 
===================================================================
File: jsinterp.c        Status: Locally Modified

   Working revision:    3.181.2.99
   Repository revision: 3.181.2.99      /cvsroot/mozilla/js/src/jsinterp.c,v
   Sticky Tag:          MOZILLA_1_8_BRANCH (branch: 3.181.2)
   Sticky Date:         (none)
   Sticky Options:      (none)

random-three-o-eight:~/moz/branch18/mozilla/js/src jwalden$ cvs ci -m "Bug 371292 - p=igor, r=brendan, a=dveditz" jsinterp.c
Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.100; previous revision: 3.181.2.99
done

Comment 20

[Bob Clary [:bc] (inactive)]

1.8.1.15/1.9.0 linux/mac/win tested with comment 9 added to js1_5/extensions/regress-365869.js with no crashes.