338001 - integer overflow in jsfun.c:Function

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[georgi - hopefully not receiving bugspam]

integer overflow in jsfun.c:Function

in js_fun.c:

   for (i = 0; i < n; i++) {
            /* Collect the lengths for all the function-argument arguments. */
            arg = js_ValueToString(cx, argv[i]);
            if (!arg)
                return JS_FALSE;
            argv[i] = STRING_TO_JSVAL(arg);
            args_length += JSSTRING_LENGTH(arg);
        }


 mark = JS_ARENA_MARK(&cx->tempPool);
        JS_ARENA_ALLOCATE_CAST(cp, jschar *, &cx->tempPool,
                               (args_length+1) * sizeof(jschar));

64 argv[i] each occupying 64MB makes 4096MB.


(gdb) bt
#0  0xffffe410 in ?? ()
#1  0xbfe24f5c in ?? ()
#2  0xb740eff4 in ?? () from /lib/tls/libc.so.6
#3  0xbfe24f48 in ?? ()
#4  0xb73737b6 in nanosleep () from /lib/tls/libc.so.6
#5  0xb73735df in sleep () from /lib/tls/libc.so.6
#6  0xb7eb28bb in ah_crap_handler (signum=11) at nsSigHandlers.cpp:133
#7  0xb7eca878 in nsProfileLock::FatalSignalHandler (signo=11)
    at nsProfileLock.cpp:210
#8  <signal handler called>
#9  0xb7d973c3 in Function (cx=0x869a260, obj=0x88299e0, argc=65, 
    argv=0x88cfd60, rval=0xbfe254e8)
    at /opt/joro/firefox/mozilla/js/src/jsfun.c:1854
#10 0xb7d9eec6 in js_Invoke (cx=0x869a260, argc=65, flags=1)
    at /opt/joro/firefox/mozilla/js/src/jsinterp.c:1311
#11 0xb7da0ae8 in js_InvokeConstructor (cx=0x869a260, vp=0x88cfd58, argc=65)
    at /opt/joro/firefox/mozilla/js/src/jsinterp.c:1850
#12 0xb7dab563 in js_Interpret (cx=0x869a260, pc=0x87230b6 "#", 
    result=0xbfe26154) at /opt/joro/firefox/mozilla/js/src/jsinterp.c:3405
#13 0xb7d9f983 in js_Execute (cx=0x869a260, chain=0x86f5e78, script=0x8722f88, 
    down=0x0, flags=0, result=0xbfe26248)
    at /opt/joro/firefox/mozilla/js/src/jsinterp.c:1555

(gdb) frame 9
#9  0xb7d973c3 in Function (cx=0x869a260, obj=0x88299e0, argc=65, 
    argv=0x88cfd60, rval=0xbfe254e8)
    at /opt/joro/firefox/mozilla/js/src/jsfun.c:1854
1854                (void) js_strncpy(cp, JSSTRING_CHARS(arg), arg_length);
Current language:  auto; currently c
(gdb) x/i $eip
0xb7d973c3 <Function+1654>:     repz movsb %ds:(%esi),%es:(%edi)
(gdb) x/4x $edi
0x88f2000:      Cannot access memory at address 0x88f2000
(gdb) x/4x $esi
0xa9c63740:     0x00660066      0x00660066      0x00660066      0x00660066
(gdb)

Comment 1

[georgi - hopefully not receiving bugspam]

Attachment: int too big

Comment 2

[georgi - hopefully not receiving bugspam]

not surprisingly, brendan has something to do with this.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsfun.c&rev=3.155&mark=1824-1860#1824

btw, suspect that this is exploitable at least on linux with enough VM.

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #2)
> not surprisingly, brendan has something to do with this.

Remember to use cvs annotate -r to find originating checkin. This code essentially goes back to rev 3.5, which mccabe checked in.  I was doing mozilla.org, not JS hacking, then -- but if you want to blame me, since after all I did touch this code later (but not recently, since you started pointing out size-scaling overflow hazards), feel free to blame me.

More helpful would be to patch the silly bug.

/be

Comment 4

[georgi - hopefully not receiving bugspam]

(In reply to comment #3)
> 
> More helpful would be to patch the silly bug.
> 

check Bug 334513 and my backported patch for official problem in sqlite - nobody cares to checkin for quite some time. the above bug may also be a sec problem.

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed fix

Georgi, you are welcome to review too.

/be

Comment 6

[georgi - hopefully not receiving bugspam]

believe that Brendan's patch stops the int overflow.

Comment 7

[georgi - hopefully not receiving bugspam]

is Bug 338121 fixed by this patch (i doubt so)?

Comment 8

[georgi - hopefully not receiving bugspam]

hm, gdb may continue after the first SEGV:

[Switching to Thread -1220614464 (LWP 15932)]
0xb7f9488f in js_IsIdentifier () from /opt/firefox1503/libmozjs.so
(gdb) x/i $eip
0xb7f9488f <js_IsIdentifier+887>:       repz movsb %ds:(%esi),%es:(%edi)
(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1231406160 (LWP 15937)]
0xb7f32597 in TimerThread::UpdateFilter ()
   from /opt/firefox1503/libxpcom_core.so
(gdb) x/i $eip
0xb7f32597 <_ZN11TimerThread12UpdateFilterEjjj+555>:    call   *0x4(%eax)
(gdb) p/x $eax
$1 = 0x660066

Comment 9

[Brendan Eich [:brendan]]

Let's talk about that other bug over in that other bug.

I checked into the trunk.  Waiting on the 1.8 branch to open.

/be

Comment 10

[Brendan Eich [:brendan]]

Fixed on 1.8 branch as well as on trunk.

/be

Comment 11

[georgi - hopefully not receiving bugspam]

the testcase is fixed on trunk

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 222153 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz for drivers

Comment 13

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Function/regress-338001.js

Georgi, you had a copyright and license notice in the test case. I used the standard tri-license boilerplate for this test. Is that ok with you?

Comment 14

[georgi - hopefully not receiving bugspam]

(In reply to comment #13)
> Created an attachment (id=225688) [edit]
> js1_5/Function/regress-338001.js
> 
> Georgi, you had a copyright and license notice in the test case. I used the
> standard tri-license boilerplate for this test. Is that ok with you?
> 

sure, it is ok with me.

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.8.0 branch.

Comment 16

[Bob Clary [:bc] (inactive)]

js1_5/Function/regress-338001.js throws out of memory on all branches/platforms but no crash. verified fixed 1.8.0.5, 1.8.1, 1.9a1

Comment 17

[Alexander Sack]

Attachment: 1.0.x landable

Comment 18

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Function/regress-338001.js

modify expected exit code for the shell per bug 358975.

Comment 19

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/Function/regress-338001.js,v  <--  regress-338001.js

Comment 20

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Function/regress-338001.js.patch

Igor, does this match what you were saying and is ok depending on the memory/architecture of the test machine?

Comment 21

[Igor Bukanov]

Comment on attachment 312311 [details] [diff] [review]
js1_5/Function/regress-338001.js.patch

Yes, this the right thing to do for the bug.

Comment 22

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/Function/regress-338001.js,v  <--  regress-338001.js
new revision: 1.4; previous revision: 1.3