Last Comment Bug 335535 - potential int overflow in jsstr.c tagify().
: potential int overflow in jsstr.c tagify().
Status: RESOLVED FIXED
[sg:critical?]
: fixed1.8.0.4, fixed1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Brendan Eich [:brendan]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-26 08:59 PDT by georgi - hopefully not receiving bugspam
Modified: 2006-07-13 15:34 PDT (History)
4 users (show)
dveditz: blocking1.7.14?
dveditz: blocking‑aviary1.0.9?
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.4+
bob: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
int overflow for x86_64 with 32 bit firefox (707 bytes, text/html)
2006-04-26 09:01 PDT, georgi - hopefully not receiving bugspam
no flags Details
fix (963 bytes, patch)
2006-04-26 13:13 PDT, Brendan Eich [:brendan]
mrbkap: review+
dveditz: approval1.8.0.4+
Details | Diff | Splinter Review

Description georgi - hopefully not receiving bugspam 2006-04-26 08:59:30 PDT
potential int overflow in jsstr.c tagify().

there is potential int overflow in jsstr.c tagify().

could only made it crash on a really perverted config: x86_64 linux with a
lot of virtual memory AND *32 bit compiled firefox*.

if it is possible to make a js string with length of 2^30-C with C small, this
may be exploitable in other configurations (personally can't make C less than
kilobytes on 32 bit linux yet).

testcase for 32 bit firefox for x64_64 linux to follow.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsstr.c&rev=3.125&mark=2162,2183,2187,2189#2162

in tagify:
2183 parlen = JSSTRING_LENGTH(param);
2187 taglen += JSSTRING_LENGTH(str) + 2 + endlen + 1;    /* 'str</end>' */
2189 tagbuf = (jschar *) JS_malloc(cx, (taglen + 1) * sizeof(jschar));

so if str == param and JSTRING_LENGTH == 2^30-1

(2^30-1+2^30-1+C)*sizeof(jschar) will overflow 2^32-1
Comment 1 georgi - hopefully not receiving bugspam 2006-04-26 09:01:00 PDT
Created attachment 219879 [details]
int overflow for x86_64 with 32 bit firefox

int overflow for x86_64 with 32 bit firefox
Comment 2 Brendan Eich [:brendan] 2006-04-26 13:13:50 PDT
Created attachment 219918 [details] [diff] [review]
fix

Don't see how earlier sums could exceed 2^32-1, so all we need is a length test.

BTW, I extracted this by hand from a jsstr.c with other changes, so line numbers are off -- patch(1) will cope.

/be
Comment 3 georgi - hopefully not receiving bugspam 2006-04-26 13:49:02 PDT
(In reply to comment #2)
> Created an attachment (id=219918) [edit]
> fix
> 
> Don't see how earlier sums could exceed 2^32-1, so all we need is a length
> test.
> 

brendan, let me explain that i crashed in the explained scenario.
according to you how much is:
2*2^30*sizeof(jschar)+C
C is small.


Comment 4 georgi - hopefully not receiving bugspam 2006-04-26 13:57:04 PDT
ooops, i flamed lamely.

my Comment #3 is buggy, so don't take it seriously.

i apologize.
Comment 5 Blake Kaplan (:mrbkap) 2006-04-26 14:48:11 PDT
Fix checked into trunk.
Comment 6 Blake Kaplan (:mrbkap) 2006-04-26 14:49:30 PDT
Comment on attachment 219918 [details] [diff] [review]
fix

I think we want this on the branches.
Comment 7 georgi - hopefully not receiving bugspam 2006-04-28 05:34:40 PDT
i suspect that strings/objects with byte size near 2GB may cause other int troubles on  32 bit systems.

what about patching JS_malloc/JS_realloc this way:

JS_PUBLIC_API(void *)
JS_malloc(JSContext *cx, size_t nbytes)
{
    void *p;

    JS_ASSERT(nbytes != 0);
    if (nbytes == 0)
        nbytes = 1;
+ if (nbytes > 50*1024*1024) return NULL;

the limit may be configurable and having in mind there is already check for 0 the performance hit probably will be small.

Comment 8 Daniel Veditz [:dveditz] 2006-04-28 11:36:06 PDT
Comment on attachment 219918 [details] [diff] [review]
fix

approved for 1.8.0 branch, a=dveditz for drivers
Comment 9 Daniel Veditz [:dveditz] 2006-04-28 11:36:38 PDT
Please also remember to land on the 1.8 branch
Comment 10 Blake Kaplan (:mrbkap) 2006-05-03 17:01:42 PDT
Fix checked into the 1.8 branches.
Comment 11 Bob Clary [:bc:] 2006-05-10 16:22:18 PDT
I don't know how to write this test for 32bit machines so that it doesn't run out of memory before finishing constructing the r string.
Comment 12 georgi - hopefully not receiving bugspam 2006-05-10 23:47:00 PDT
(In reply to comment #11)
> I don't know how to write this test for 32bit machines so that it doesn't run
> out of memory before finishing constructing the r string.
> 

i tried a lot of testcases with large strings, but with no luck on 32 bit machines. not sure about the reason for failure. the testcase should be very close to this one imho.

similar "issues" are Bug 336409 and Bug 336410

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