Note: There are a few cases of duplicates in user autocompletion which are being worked on.

CVE-2006-0297 probably an integer overflow in jsxml.c

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
JavaScript Engine
P2
normal
VERIFIED FIXED
12 years ago
11 years ago

People

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

Tracking

({verified1.8.0.1, verified1.8.0.2, verified1.8.1})

1.8 Branch
mozilla1.9alpha1
x86
Linux
verified1.8.0.1, verified1.8.0.2, verified1.8.1
Points:
---
Bug Flags:
blocking1.7.13 -
blocking-aviary1.0.8 -
blocking1.9a1 +
blocking1.8.0.1 +
blocking1.8.0.2 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:investigate] critical? [rft-dl])

Attachments

(4 attachments, 1 obsolete attachment)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

in jsxml.c:2351 EscapeAttributeValue
    if (c == '"')
            newlength += 5;
        else if (c == '<')
            newlength += 3;
        else if (c == '&' || c == '\n' || c == '\r' || c == '\t')
            newlength += 4;

this seems somewhat suspicous, because of "int too big" problem (probably
after multiplication with sizeof(jschar) ).

some empirical evidence suggests SEGV in free() after hitting this code,
though the exact codepath is not determined yet.

exploitability will be determined later.

debug builds die from abort(), but 1.5 release crashes in free.

testcase to follow.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1220606272 (LWP 16742)]
0xb753e339 in free () from /lib/tls/libc.so.6

#0  0xb753e339 in free () from /lib/tls/libc.so.6
#1  0xb7fc2ea9 in js_ReportCompileErrorNumberUC ()
   from /opt/firefox15/libmozjs.so
#2  0xb7fc2ef7 in js_FinishStringBuffer () from /opt/firefox15/libmozjs.so
#3  0xb7fd342d in js_IsXMLName () from /opt/firefox15/libmozjs.so
#4  0xb7fd3954 in js_IsXMLName () from /opt/firefox15/libmozjs.so
#5  0xb7fd9b62 in js_IsXMLName () from /opt/firefox15/libmozjs.so
#6  0xb7f989b9 in js_Invoke () from /opt/firefox15/libmozjs.so
#7  0xb7fa1717 in js_Interpret () from /opt/firefox15/libmozjs.so
#8  0xb7f99011 in js_Execute () from /opt/firefox15/libmozjs.so
#9  0xb7f78a28 in JS_EvaluateUCScriptForPrincipals ()
   from /opt/firefox15/libmozjs.so

(gdb) x/i $eip
0xb753e339 <free+73>:   mov    0x4(%ecx),%eax
(gdb) p/x $eax
$1 = 0xb76037e4
(gdb) p/x $ecx
$2 = 0xfffffff9



Reproducible: Always

Steps to Reproduce:
testcase
Actual Results:  
crash in free()

Expected Results:  
crash free
(Reporter)

Updated

12 years ago
Version: unspecified → 1.5 Branch
Created attachment 205552 [details]
crash in free()

crash in free()
Assignee: nobody → mrbkap
Component: Security → JavaScript Engine
Flags: blocking1.9a1+
Flags: blocking1.8.0.1+
Product: Firefox → Core
Whiteboard: [sg:investigate] critical?
Version: 1.5 Branch → 1.8 Branch

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: firefox → general
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
no action, so adding management without offensing words.
management knows better.
good management.
(Assignee)

Comment 3

12 years ago
I'm sorry for not commenting earlier. I have a patch in hand and am currently running it through tests to ensure that it does not introduce any regressions. I didn't bother commenting because I feel that, in general, the "Status" of a bug is sufficient to indicate that I am working on a fix. So there has certainly been action on this bug.
(Assignee)

Comment 4

12 years ago
Created attachment 205755 [details] [diff] [review]
patch v1

I cannot reproduce the crash on my machine (though it's brought almost to its knees), but this should fix the overflow.

I also found a bug in the use of StringBuffer::grow (please excuse my dirty C++ syntax here!) where we were too agressively growing the string.
Attachment #205755 - Flags: review?(brendan)
Comment on attachment 205755 [details] [diff] [review]
patch v1

> static JSBool
> GrowStringBuffer(JSStringBuffer *sb, size_t newlength)
> {
>     ptrdiff_t offset;
>     jschar *bp;
> 
>     offset = PTRDIFF(sb->ptr, sb->base, jschar);
>-    newlength += offset;
>-    bp = (jschar *) realloc(sb->base, (newlength + 1) * sizeof(jschar));
>+    newlength += offset + 1;
>+    if (newlength > offset && newlength < ~(size_t)0 / sizeof(jschar))

Put this in number-line order:

>+    if (offset < newlength && newlength < ~(size_t)0 / sizeof(jschar))

Also, you'll need to cast offset to (size_t) to shut up an MSVC warning.

Pick these nits and r=me (new patch in bug only for branch landing and verification).

Thanks for fixing these(! what was I thinking adding the offset in the callers?) bugs,

/be
Attachment #205755 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

12 years ago
Created attachment 205762 [details] [diff] [review]
Patch for checkin

I added the cast and an assertion to make sure that casting |offset| wasn't going to do the wrong thing.
Attachment #205755 - Attachment is obsolete: true
Attachment #205762 - Flags: review?(brendan)
Comment on attachment 205762 [details] [diff] [review]
Patch for checkin

r=me, nominate for branches once this bakes a bit on the trunk.

/be
Attachment #205762 - Flags: review?(brendan) → review+
(Assignee)

Comment 8

12 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 9

12 years ago
Although there is a testcase outlined in this bug, it is one that doesn't complete in a time short enough to allow it to be placed in the normal JavaScript Test Library so I am marking this testcase-.
Flags: testcase-
Comment on attachment 205762 [details] [diff] [review]
Patch for checkin

Another security fix that has been thoroughly baked on the trunk.

/be
Attachment #205762 - Flags: approval1.8.1?
Attachment #205762 - Flags: approval1.8.0.1?
Comment on attachment 205762 [details] [diff] [review]
Patch for checkin

a=dveditz for drivers
Attachment #205762 - Flags: approval1.8.1?
Attachment #205762 - Flags: approval1.8.1+
Attachment #205762 - Flags: approval1.8.0.1?
Attachment #205762 - Flags: approval1.8.0.1+
(Assignee)

Comment 12

12 years ago
Fix checked into the branches.
Keywords: fixed1.8.0.1, fixed1.8.1

Comment 13

12 years ago
I've rethought the time limit and think we can put this in the automated tests after all.

no crash in 1.8.0.1, 1.8.1, 1.9a1 on winxp 20060112

no crash in 1.8.0.1 linux 20060112

crash in 1.8.1 linux 20060112 TB13900472

libc.so.6 + 0x61e79 (0x00c3de79)
FreeStringBuffer()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/js/src/jsscan.c, line 823]

crash in 1.9a1 linux 20060112 TB13900588

libc.so.6 + 0x61e79 (0x00e29e79)
FreeStringBuffer()  [/builds/tinderbox/Fx-Trunk/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/js/src/jsscan.c, line 823]
Status: RESOLVED → REOPENED
Flags: testcase- → testcase?
Keywords: fixed1.8.0.1, fixed1.8.1 → verified1.8.0.1
Resolution: FIXED → ---

Comment 14

12 years ago
Created attachment 208400 [details]
e4x/Regress/regress-319872.js

Updated

12 years ago
Flags: testcase? → testcase+
(Assignee)

Comment 15

12 years ago
Created attachment 208787 [details] [diff] [review]
Fix the bad call

In a debug build, you would have noticed an assertion in jsscan.c. The problem is fairly obvious: in the case that !STRING_BUFFER_OK(&sb), we were reporting oom and jumping to out:, where we would only test for str != null (with no assignments to str outside of null) and try to free the sentinal value. Rearranging the label won't work, so here's the patch to not free when we don't want to.
Attachment #208787 - Flags: review?(brendan)
Comment on attachment 208787 [details] [diff] [review]
Fix the bad call

r=me, thanks for fixing this.

/be
Attachment #208787 - Flags: review?(brendan)
Attachment #208787 - Flags: review+
Attachment #208787 - Flags: approval1.8.1?
Attachment #208787 - Flags: approval1.8.0.2?

Updated

12 years ago
Flags: blocking1.8.0.2?
(Assignee)

Comment 17

12 years ago
Fixed on trunk, again.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Flags: blocking1.7.13-
Flags: blocking-aviary1.0.8-
Keywords: fixed1.8.1
sorry for the bugspam, saw the checkin and thought the missing fixed1.8.1 was as oversight, but it was removed with comment 13
Keywords: fixed1.8.1

Comment 19

12 years ago
There are other issues with this that I need to sort out. I verified based on the results of the browser based tests which don't necessarily run long enough to show  crashes. I am rebuilding with the patch in attachment 208787 [details] [diff] [review] to get a better idea of what is happening.

Comment 20

12 years ago
(In reply to comment #19)
with  attachment 208787 [details] [diff] [review] I crash in windows firefox 1.5 (1.8)

JSHashNumber
js_HashString(JSString *str)
{
    JSHashNumber h;
    const jschar *s;
    size_t n;

    h = 0;
    for (s = JSSTRING_CHARS(str), n = JSSTRING_LENGTH(str); n; s++, n--)
->        h = (h >> (JS_HASH_BITS - 4)) ^ (h << 4) ^ *s;
    return h;
}


	h	0x00000000
	n	0x2fa6001a
+	s	0x00000001 ""
-	str	0x0013dc48
	length	0x2fa6001a
+	chars	0x00000001 ""


js_HashString(JSString * 0x0013dc48) line 2811 + 19 bytes
js_AtomizeString(JSContext * 0x00032dd8, JSString * 0x0013dc48, unsigned int 0x00000080) line 650 + 9 bytes
js_AtomizeChars(JSContext * 0x00032dd8, const unsigned short * 0x00000001, unsigned int 0x2fa6001a, unsigned int 0x00000000) line 752 + 19 bytes
js_GetToken(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8) line 1271 + 42 bytes
js_MatchToken(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, int 0x00000043) line 2130 + 13 bytes
XMLTagContent(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, JSTreeContext * 0x0013e308, int 0x0000003d, JSAtom * * 0x0013e244) line 3407 + 15 bytes
XMLElementOrList(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, JSTreeContext * 0x0013e308, int 0x00000000) line 3534 + 23 bytes
XMLElementContent(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, JSParseNode * 0x004a9dd0, JSTreeContext * 0x0013e308) line 3486 + 19 bytes
XMLElementOrList(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, JSTreeContext * 0x0013e308, int 0x00000000) line 3584 + 21 bytes
XMLElementOrListRoot(JSContext * 0x00032dd8, JSTokenStream * 0x004a9af8, JSTreeContext * 0x0013e308, int 0x00000000) line 3671 + 21 bytes
js_ParseXMLTokenStream(JSContext * 0x00032dd8, JSObject * 0x000387c8, JSTokenStream * 0x004a9af8, int 0x00000000) line 3717 + 24 bytes
ParseXMLSource(JSContext * 0x00032dd8, JSString * 0x008d7ff8) line 1943 + 25 bytes
ToXML(JSContext * 0x00032dd8, long 0x008d7ffc) line 2042 + 13 bytes
XML(JSContext * 0x00032dd8, JSObject * 0x008d86a8, unsigned int 0x00000001, long * 0x0041db8c, long * 0x0013e524) line 7051 + 13 bytes
js_Invoke(JSContext * 0x00032dd8, unsigned int 0x00000001, unsigned int 0x00000001) line 1177 + 23 bytes
js_Interpret(JSContext * 0x00032dd8, unsigned char * 0x00420771, long * 0x0013ee20) line 3095 + 15 bytes
js_Execute(JSContext * 0x00032dd8, JSObject * 0x000387c8, JSScript * 0x00420670, JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x0013fecc) line 1423 + 19 bytes
JS_ExecuteScript(JSContext * 0x00032dd8, JSObject * 0x000387c8, JSScript * 0x00420670, long * 0x0013fecc) line 3999 + 25 bytes
Process(JSContext * 0x00032dd8, JSObject * 0x000387c8, char * 0x00032d2a) line 223 + 22 bytes
ProcessArgs(JSContext * 0x00032dd8, JSObject * 0x000387c8, char * * 0x00032cc4, int 0x00000003) line 470 + 17 bytes
main(int 0x00000003, char * * 0x00032cc4, char * * 0x00033268) line 2565 + 21 bytes
JS! mainCRTStartup + 227 bytes
KERNEL32! 7c816d4f()

I am ASSuming this is related to the overlong string crash in bug 342422 and the int overflow is indeed resolved with this patch. I'll try the patch in bug 324422 combined with this.
(Assignee)

Comment 21

12 years ago
Bob, would you file a new bug on that crash? If it turns out to be a dupe, we'll mark it then.

Comment 22

12 years ago
(In reply to comment #21)
Filed Bug 324533 

Updated

12 years ago
Summary: probably an integer overflow in jsxml.c → CVE-2006-0297 probably an integer overflow in jsxml.c
regarding CVE in the summary:

i consider mitre/CVE corporate/government's puppies and have low opinion of
them.

mitre's  "Steven M. Christey" <coley@linus.mitre.org>
co-authored the "responsible disclosure draft rfc" which was
obviously a corporate plot and was ditched by even the IETF - check the
ietf archives for this corporate fiasco.
Attachment #208787 - Flags: approval1.8.1? → branch-1.8.1?(brendan)
Comment on attachment 208787 [details] [diff] [review]
Fix the bad call

approving for 1.8 and 1.8.0 branch checkin.

/be
Attachment #208787 - Flags: branch-1.8.1?(brendan)
Attachment #208787 - Flags: branch-1.8.1+
Attachment #208787 - Flags: approval1.8.0.2?
Attachment #208787 - Flags: approval1.8.0.2+
Group: security
Flags: blocking1.8.0.2? → blocking1.8.0.2+

Comment 25

12 years ago
I don't have a stack for this, but this now crashes in 1.8/1.8.0.1 linux only in browser tests on e4x/Regress/regress-319872.js

linux-1.8_2006020704
linux-1.8.0.1_2006020704
Keywords: verified1.8.0.1
(Assignee)

Comment 26

12 years ago
Isn't that just bug 324533?

Comment 27

12 years ago
(In reply to comment #26)
> Isn't that just bug 324533?
> 

yeah.

Updated

12 years ago
Status: RESOLVED → VERIFIED
Keywords: verified1.8.0.1, verified1.8.1
(Assignee)

Comment 28

12 years ago
Second fix checked into the 1.8.0.2 branch.
Keywords: fixed1.8.0.2
Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates) since in-testsuite+ indicates a test case exists in the js test library.
Whiteboard: [sg:investigate] critical? → [sg:investigate] critical? [rft-dl]
(In reply to comment #29)
> Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates)
> since in-testsuite+ indicates a test case exists in the js test library.

Are these status whiteboard marks documented somewhere?  Thanks,

/be

Comment 31

12 years ago
v ff 1.8.0.1/1.8/1.9 20060302 win/linux/mac
Keywords: fixed1.8.0.2 → verified1.8.0.2

Comment 32

12 years ago
Although I wasn't able to reproduce it with my local builds, I saw a crash in the trunk shell on windows from 20060328. No crashes in the browser versions of the tests or on mac or linux.
(In reply to comment #32)
> No crashes in the browser versions of
> the tests or on mac or linux.
> 

for me linux trunk also seems safe.

Comment 34

11 years ago
Checking in regress-319872.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-319872.js,v  <--  regress-319872.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.