The default bug view has changed. See this FAQ.

integer overflow in jsfun.c:Function

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Assigned: brendan)

Tracking

({verified1.8.0.5, verified1.8.1})

Trunk
x86
Linux
verified1.8.0.5, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.0.5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(5 attachments, 1 obsolete attachment)

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)
Created attachment 222037 [details]
int too big

Updated

11 years ago
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
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.
(Assignee)

Comment 3

11 years ago
(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
(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.

(Assignee)

Comment 5

11 years ago
Created attachment 222153 [details] [diff] [review]
proposed fix

Georgi, you are welcome to review too.

/be
Attachment #222153 - Flags: review?(mrbkap)

Updated

11 years ago
Attachment #222153 - Flags: review?(mrbkap) → review+
believe that Brendan's patch stops the int overflow.
(Assignee)

Updated

11 years ago
Attachment #222153 - Flags: approval1.8.0.5?
Attachment #222153 - Flags: approval-branch-1.8.1+
is Bug 338121 fixed by this patch (i doubt so)?
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

(Assignee)

Comment 9

11 years ago
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
Assignee: general → brendan
(Assignee)

Comment 10

11 years ago
Fixed on 1.8 branch as well as on trunk.

/be
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: blocking1.8.0.5?
Keywords: fixed1.8.1
Resolution: --- → FIXED
the testcase is fixed on trunk
Comment on attachment 222153 [details] [diff] [review]
proposed fix

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #222153 - Flags: approval1.8.0.5? → approval1.8.0.5+
Flags: blocking1.8.0.5? → blocking1.8.0.5+

Comment 13

11 years ago
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?

Updated

11 years ago
Flags: in-testsuite+
(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.
Fix checked into the 1.8.0 branch.
Keywords: fixed1.8.0.5

Comment 16

11 years ago
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
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.5, fixed1.8.1 → verified1.8.0.5, verified1.8.1
Whiteboard: [sg:critical?]

Comment 17

11 years ago
Created attachment 232730 [details] [diff] [review]
1.0.x landable

Comment 18

11 years ago
Created attachment 245285 [details]
js1_5/Function/regress-338001.js

modify expected exit code for the shell per bug 358975.
Attachment #225688 - Attachment is obsolete: true
Group: security

Comment 19

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

Comment 20

9 years ago
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?
Attachment #312311 - Flags: review?(igor)

Comment 21

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

Yes, this the right thing to do for the bug.
Attachment #312311 - Flags: review?(igor) → review+

Comment 22

9 years ago
/cvsroot/mozilla/js/tests/js1_5/Function/regress-338001.js,v  <--  regress-338001.js
new revision: 1.4; previous revision: 1.3
You need to log in before you can comment on or make changes to this bug.