359024 - script bug causes deref of 0x80000001 (JSVAL_VOID)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[georgi - hopefully not receiving bugspam]

Script.compile();Script.exec(); causes scary crash on branches

Script.compile();Script.exec(); causes a crash with scary registers on 
branches (2.0, 1.5 - linux, macosxppc).
debug builds die from JS_Assert with this stack:
(gdb) bt
#0  0x9004796c in kill ()
#1  0x9012dc14 in abort ()
#2  0x23107524 in JS_Assert (s=0x2313a41c "slot < fp->nvars",
file=0x23139a8c "/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c",
ln=4859) at /Users/joro/inst/firefox20/mozilla/js/src/jsutil.c:63
#3  0x230890c0 in js_Interpret (cx=0x1c4174b0, pc=0x1c4021d3 "\233",
result=0xbfffbba0) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:4859
#4  0x2306f484 in js_Execute (cx=0x1c4174b0, chain=0x1c162f0,
script=0x1c402180, down=0x1c1a234, flags=32, result=0xbfffbd58) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:1622
#5  0x230f59d4 in script_exec (cx=0x1c4174b0, obj=0x1c162d8, argc=0,
argv=0x1c1a2b8, rval=0xbfffbd58) at
/Users/joro/inst/firefox20/mozilla/js/src/jsscript.c:340
#6  0x2306e60c in js_Invoke (cx=0x1c4174b0, argc=0, flags=0) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:1377
#7  0x230841fc in js_Interpret (cx=0x1c4174b0, pc=0x1e875210 ":",
result=0xbfffcd04) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:4119
#8  0x2306e6d4 in js_Invoke (cx=0x1c4174b0, argc=1, flags=2) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:1396
9  0x2306eb3c in js_InternalInvoke (cx=0x1c4174b0, obj=0x1c15990,
fval=29449960, flags=0, argc=1, argv=0xbfffd048, rval=0xbfffd058) at
/Users/joro/inst/firefox20/mozilla/js/src/jsinterp.c:1471
#10 0x2301c938 in JS_CallFunctionValue (cx=0x1c4174b0, obj=0x1c15990,
fval=29449960, argc=1, argv=0xbfffd048, rval=0xbfffd058) at
/Users/joro/inst/firefox20/mozilla/js/src/jsapi.c:4419
#11 0x07816004 in nsJSContext::CallEventHandler (this=0x1b36ad40,
aTarget=0x1c15990, aHandler=0x1c15ee8, argc=1, argv=0xbfffd048,
rval=0xbfffd058) at
/Users/joro/inst/firefox20/mozilla/dom/src/base/nsJSEnvironment.cpp:1493
#12 0x0788b188 in nsJSEventListener::HandleEvent (this=0x1e8695c0,
aEvent=0x1c47a660) at
/Users/joro/inst/firefox20/mozilla/dom/src/events/nsJSEventListener.cpp:186
#13 0x076a03f8 in nsEventListenerManager::HandleEventSubType
(this=0x1e8689f0, aListenerStruct=0x1e8696d0, aListener=0x1e8695c0,
aDOMEvent=0x1c47a660, aCurrentTarget=0x1c4173c4, aSubType=1, aPhaseFlags=7)
at
/Users/joro/inst/firefox20/mozilla/content/events/src/nsEventListenerManager.cpp:1655
#14 0x076a36b8 in nsEventListenerManager::HandleEvent (this=0x1e8689f0,
aPresContext=0x1e8631a0, aEvent=0xbfffd4e0, aDOMEvent=0xbfffd3dc,
aCurrentTarget=0x1c4173c4, aFlags=7, aEventStatus=0xbfffd4dc) at
/Users/joro/inst/firefox20/mozilla/content/events/src/nsEventListenerManager.cpp:1759

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: testcase

Comment 2

[Brian Crowder]

Here's a minimal reproducible testcase that works on the trunk (if you enable HAS_SCRIPT in jsconfig.h) js shell:

function f() { var scri = new Script(""); scri.compile(); scri.exec();}; f()

Looks like new code of mine is treating a JSVAL_VOID as a pointer, fix shortly (I don't think this is security-sensitive, however).

Comment 3

[Brian Crowder]

Attachment: fix idiocy on my part

Last fix was pretty broken.  This makes it better.

Comment 4

[Brian Crowder]

Attachment: minus js.msg I don't need anymore

Comment 5

[Brian Crowder]

I cannot get this testcase to crash in either a 1_8_0 or a 1_8 build of the browser.

Comment 6

[georgi - hopefully not receiving bugspam]

(In reply to comment #2)
> (I don't think this is security-sensitive, however).
> 

remove the flag if you can

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 256885 [details] [diff] [review]
minus js.msg I don't need anymore


>-    oldscript = (JSScript *) JSVAL_TO_PRIVATE(v);
>+    oldscript = 0;
>+    if (JSVAL_IS_INT(v)) {
>+        oldscript = (JSScript *) JSVAL_TO_PRIVATE(v);
>+    }

Too pessimistic to double-assign in common case, so use oldscript = cond ? cast : null;

Don't use 0 in SpiderMonkey for NULL.

Do use !JSVAL_IS_VOID(v) instead of JSVAL_IS_INT(v) for cond.

I do not believe this bug should not be security-sensitive, since it is a guaranteed dereference of 0x80000000. Even on a 64-bit system, I don't think a bad actor can control that address.

>     /* Swap script for obj's old script, if any. */
>-    v = LOCKED_OBJ_GET_SLOT(cx, obj, JSSLOT_PRIVATE);
>-    oldscript = (JSScript *) JSVAL_TO_PRIVATE(v);
>+    v = LOCKED_OBJ_GET_SLOT(obj, JSSLOT_PRIVATE);
>+    oldscript = 0;
>+    if (JSVAL_IS_INT(v)) {
>+        oldscript = (JSScript *) JSVAL_TO_PRIVATE(v);
>+    }

Same three comments as first three above.

/be

Comment 8

[Brendan Eich [:brendan]]

Georgi, can you comment on the contention that 0x80000000+small-n is not controllable?

/be

Comment 9

[Brian Crowder]

Attachment: brendan's issues addressed

I'd be interested to know if anyone else can actually make this happen on branches, too, as I cannot.

Comment 10

[georgi - hopefully not receiving bugspam]

(In reply to comment #8)
> Georgi, can you comment on the contention that 0x80000000+small-n is not
> controllable?
> 

definitely easily controllable on x86 linux and almost sure on x86_64 linux.
[joro@sivokote progs]$ cat x80.c 
void main() {
        char *a;
        a=malloc(2*1024*1024*1024);
        while(42) {};
        /* cat /proc/pid/maps */
}
[joro@sivokote progs]$ ./a.out
cat /proc/`pidof a.out`/maps
...
08049000-0804a000 rwxp 00000000 03:05 5837615    /home/joro/progs/a.out
****37e18000-b7e1a000 rwxp 37e18000 00:00 0 
b7e1a000-b7f41000 r-xp 00000000 03:01 4997126    /lib/i686/libc-2.4.so
...

Comment 11

[georgi - hopefully not receiving bugspam]

[joro@sivokote progs]$ ./a.out 
deref ptr=80000000 *ptr=43434343
#include <stdio.h>

void main() {
        char *a;
        unsigned int len=2*1024*1024*1024;
        int *ptr;
        ptr= (int *) 0x80000000;
        a=malloc(len);
        if(!a)  return;
        memset(a,0x43,len);
        printf("deref ptr=%x *ptr=%x\n", ptr,*ptr);
        while(42) {};
        /* cat /proc/pid/maps */
}

in addition (int) 0x80000000 == - (int) 0x80000000
on 32 bit systems.

Comment 12

[georgi - hopefully not receiving bugspam]

on 32 bit system ability to control contiguous block of 2GB + small memory guarantees to control the middle of address space 0x8000000

Comment 13

[georgi - hopefully not receiving bugspam]

suspect that from theoretic point of view 0x80000000 is the best address that can  *provably* be controlled with minimal (2GB) contiguous resources on 32 bit systems without relying on any conjectures ;)

Comment 14

[Brendan Eich [:brendan]]

Georgi: thanks -- s-s this bug remains. Good to know.

/be

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 256910 [details] [diff] [review]
brendan's issues addressed

You ignored one of the three points I raised:

Do use !JSVAL_IS_VOID(v) instead of JSVAL_IS_INT(v) for cond.

/be

Comment 16

[Brian Crowder]

Attachment: ugh

Sorry for missing that.

Comment 17

[Brendan Eich [:brendan]]

Comment on attachment 256942 [details] [diff] [review]
ugh

I should have made a JSVAL_TO_PRIVATE(v) macro long ago. Oh well.

/be

Comment 18

[Brian Crowder]

jsscript.c: 3.137

I still cannot repro this on branches.  G30rgi, can you please provide more info about your build/version?

Comment 19

[Brian Crowder]

Marking fixed since I believe there was a trunk bug which is now fixed.  If this bug still exists on branches, please reopen with more info.

Comment 20

[georgi - hopefully not receiving bugspam]

(In reply to comment #19)
> Marking fixed since I believe there was a trunk bug which is now fixed.  If
> this bug still exists on branches, please reopen with more info.
> 

2.0.0.1 and 1.5.10 seem safe on linux. the scripts executes as expected without crash.

Comment 21

[Brian Crowder]

Was it just a mistake that the original submission of this said "scary crash on branches"?

Comment 22

[georgi - hopefully not receiving bugspam]

(In reply to comment #21)
> Was it just a mistake that the original submission of this said "scary crash on
> branches"?
> 

hmmm, not sure. will test more.

Comment 23

[georgi - hopefully not receiving bugspam]

on linux:

1.5.0.8:
crash:
(gdb) x/i $eip
0xb7f7cf30 <js_Interpret+40440>:        mov    %eax,(%edx,%ecx,4)
(gdb) p/x $edx
$1 = 0x8c91ec4
(gdb) p/$ecx
$2 = 0x4910dae

1.5.0.9, 1.5.0.10: script executes.

Comment 24

[georgi - hopefully not receiving bugspam]

2.0 also crashes, 2.0.0.1 and 2.0.0.2 do not

Comment 25

[georgi - hopefully not receiving bugspam]

valgrind doesn't show problems with 2.0.0.2

Comment 26

[georgi - hopefully not receiving bugspam]

this is funny:

2006-11-01-04-trunk and 2006-10-31-04-trunk don't crash while branches from this time crash.

Comment 27

[Samuel Sidler (old account; do not CC)]

Based on comments 23 and 24, it looks like this is fine on the branch. If I'm not understanding that correctly, please remove the entry in the status whiteboard that I'm now adding.

When 1.9 alpha 3 ships, we can probably remove the s-s on this.

Comment 28

[Daniel Veditz [:dveditz]]

I guess the question is, whether the branch is really fixed (there were changes to jsscript.c in 2.0.0.1) or whether the problem is just masked. If this is the right fix and the branch code looks like the trunk maybe we should take this anyway.

Comment 29

[Brian Crowder]

Sorry, I should have explained better.  The patch I have committed for the trunk here is a patch to code I recently broke on the trunk before.  The broken-ness _I'm_ thinking of (and which this patch fixes) didn't make it to the branches.  If there is another crash happening on the branches it is a different bug.  What confused me was that the original summary of this bug was something like "scary crash on branches".

Comment 30

[georgi - hopefully not receiving bugspam]

(In reply to comment #29)
> What confused me was that the original summary of this bug was
> something like "scary crash on branches".
> 

let me explain about the summary.
the bug was filed on 2006-11-01 04:05:26 PST.
at that time only branches crashed and trunk didn't crash.
see comments 24 and 26.

Comment 31

[georgi - hopefully not receiving bugspam]

(In reply to comment #28)
> whether the problem is just masked

according to comment #25 valgrind doesn't shows problems with 2.0.0.2, so if it is masked it should be masked very hard.

Comment 32

[chris hofmann]

so I guess this qualifies for the "post 1.8-branch" whiteboard comment and/or no fix is needed for the branch.  make corrections if this is not the case.

Comment 33

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-359024.js,v  <--  regress-359024.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/f0e9fd501e63