Last Comment Bug 338001 - integer overflow in jsfun.c:Function
: integer overflow in jsfun.c:Function
Status: VERIFIED FIXED
[sg:critical?]
: verified1.8.0.5, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Brendan Eich [:brendan]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-15 08:03 PDT by georgi - hopefully not receiving bugspam
Modified: 2008-03-28 12:16 PDT (History)
7 users (show)
dveditz: blocking1.8.0.5+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
int too big (500 bytes, text/html)
2006-05-15 08:04 PDT, georgi - hopefully not receiving bugspam
no flags Details
proposed fix (2.44 KB, patch)
2006-05-16 01:26 PDT, Brendan Eich [:brendan]
mrbkap: review+
brendan: approval‑branch‑1.8.1+
dveditz: approval1.8.0.5+
Details | Diff | Splinter Review
js1_5/Function/regress-338001.js (2.41 KB, text/plain)
2006-06-15 04:05 PDT, Bob Clary [:bc:]
no flags Details
1.0.x landable (1.86 KB, patch)
2006-08-08 08:20 PDT, Alexander Sack
no flags Details | Diff | Splinter Review
js1_5/Function/regress-338001.js (2.44 KB, text/plain)
2006-11-10 16:36 PST, Bob Clary [:bc:]
no flags Details
js1_5/Function/regress-338001.js.patch (942 bytes, patch)
2008-03-28 10:58 PDT, Bob Clary [:bc:]
igor: review+
Details | Diff | Splinter Review

Description georgi - hopefully not receiving bugspam 2006-05-15 08:03:27 PDT
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 2006-05-15 08:04:23 PDT
Created attachment 222037 [details]
int too big
Comment 2 georgi - hopefully not receiving bugspam 2006-05-15 11:16:23 PDT
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] 2006-05-15 16:20:24 PDT
(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 2006-05-15 23:59:32 PDT
(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] 2006-05-16 01:26:03 PDT
Created attachment 222153 [details] [diff] [review]
proposed fix

Georgi, you are welcome to review too.

/be
Comment 6 georgi - hopefully not receiving bugspam 2006-05-16 02:18:34 PDT
believe that Brendan's patch stops the int overflow.
Comment 7 georgi - hopefully not receiving bugspam 2006-05-16 03:58:34 PDT
is Bug 338121 fixed by this patch (i doubt so)?
Comment 8 georgi - hopefully not receiving bugspam 2006-05-16 04:36:44 PDT
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] 2006-05-16 05:13:01 PDT
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] 2006-05-16 05:17:57 PDT
Fixed on 1.8 branch as well as on trunk.

/be
Comment 11 georgi - hopefully not receiving bugspam 2006-05-16 07:30:30 PDT
the testcase is fixed on trunk
Comment 12 Daniel Veditz [:dveditz] 2006-06-05 12:05:33 PDT
Comment on attachment 222153 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz for drivers
Comment 13 Bob Clary [:bc:] 2006-06-15 04:05:43 PDT
Created attachment 225688 [details]
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 2006-06-15 07:41:21 PDT
(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) 2006-06-15 19:18:50 PDT
Fix checked into the 1.8.0 branch.
Comment 16 Bob Clary [:bc:] 2006-06-26 17:33:03 PDT
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 2006-08-08 08:20:35 PDT
Created attachment 232730 [details] [diff] [review]
1.0.x landable
Comment 18 Bob Clary [:bc:] 2006-11-10 16:36:38 PST
Created attachment 245285 [details]
js1_5/Function/regress-338001.js

modify expected exit code for the shell per bug 358975.
Comment 19 Bob Clary [:bc:] 2007-02-08 14:57:13 PST
/cvsroot/mozilla/js/tests/js1_5/Function/regress-338001.js,v  <--  regress-338001.js
Comment 20 Bob Clary [:bc:] 2008-03-28 10:58:06 PDT
Created attachment 312311 [details] [diff] [review]
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 2008-03-28 11:18:03 PDT
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:] 2008-03-28 12:16:28 PDT
/cvsroot/mozilla/js/tests/js1_5/Function/regress-338001.js,v  <--  regress-338001.js
new revision: 1.4; previous revision: 1.3

Note You need to log in before you can comment on or make changes to this bug.