649259 - TM: Assertion failure: newcap >= oldcap && !hasSlotsArray(), at jsobj.cpp:4203

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on TI revision 23a746dac370 (run with -m -n -a),
tested on 64 bit:

var length = 4294967295;
eval("this.__defineSetter__('x', gc); this.watch('x', [].slice); x = 1;");

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

s-s on behalf of bhackett on irc..

Comment 2

[Brian Hackett [Laid off!]]

This is an integer overflow on TM where Array.slice calls NewDenseAllocatedArray with UINT32_MAX elements, we JS_ROUNDUP that to zero and end up making an empty array.

This isn't exploitable as the followup code does not assume the array actually was allocated with that many elements (though it could, as that is part of NewDenseAllocatedArray's contract), using SetArrayElement instead of setDenseArrayElement.  There is a similar case in Array.map which could also overflow the array on creation but does not assume the number of elements in the result.

This code changed in bug 584917 but the roundup problem seems to predate that bug.  This assert shows up in JM but not TM as the JM branch applies the patch in bug 648321 which has this extra assertion.

We should watch for this overflow and set OOM if it happens.

Comment 3

[Christian Holler (:decoder)]

Are you sure this is not exploitable? When I run this in valgrind on an opt build, I see lots of invalid writes to addresses like this:

==9371== Invalid write of size 1
==9371==    at 0x4C28F0C: memcpy (mc_replace_strmem.c:497)
==9371==    by 0x4B28ED: JSObject::allocSlots(JSContext*, unsigned long) (string3.h:52)
==9371==    by 0x4263F2: T.1955 (jsobj.h:644)
==9371==    by 0x42987F: array_slice(JSContext*, unsigned int, js::Value*) (jsarray.cpp:2882)
==9371==    by 0x494B54: js::Invoke(JSContext*, js::CallArgs const&, js::ConstructOption) (jscntxtinlines.h:293)
==9371==    by 0x4953DC: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:827)
==9371==    by 0x4A8F7B: obj_watch_handler(JSContext*, JSObject*, long, unsigned long, unsigned long*, void*) (jsobj.cpp:1389)
==9371==    by 0x449252: js_watch_set_wrapper(JSContext*, unsigned int, js::Value*) (jsdbgapi.cpp:852)
==9371==    by 0x494B54: js::Invoke(JSContext*, js::CallArgs const&, js::ConstructOption) (jscntxtinlines.h:293)
==9371==    by 0x4953DC: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:827)
==9371==    by 0x49548F: js::ExternalGetOrSet(JSContext*, JSObject*, long, js::Value const&, JSAccessMode, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:867)
==9371==    by 0x4A997A: js_NativeSet(JSContext*, JSObject*, js::Shape const*, bool, bool, js::Value*) (jsscopeinlines.h:293)
==9371==  Address 0x5edfd31 is 1 bytes after a block of size 0 alloc'd
==9371==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==9371==    by 0x4B27B6: JSObject::allocSlots(JSContext*, unsigned long) (jsutil.h:239)
==9371==    by 0x4263F2: T.1955 (jsobj.h:644)
==9371==    by 0x42987F: array_slice(JSContext*, unsigned int, js::Value*) (jsarray.cpp:2882)
==9371==    by 0x494B54: js::Invoke(JSContext*, js::CallArgs const&, js::ConstructOption) (jscntxtinlines.h:293)
==9371==    by 0x4953DC: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:827)
==9371==    by 0x4A8F7B: obj_watch_handler(JSContext*, JSObject*, long, unsigned long, unsigned long*, void*) (jsobj.cpp:1389)
==9371==    by 0x449252: js_watch_set_wrapper(JSContext*, unsigned int, js::Value*) (jsdbgapi.cpp:852)
==9371==    by 0x494B54: js::Invoke(JSContext*, js::CallArgs const&, js::ConstructOption) (jscntxtinlines.h:293)
==9371==    by 0x4953DC: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:827)
==9371==    by 0x49548F: js::ExternalGetOrSet(JSContext*, JSObject*, long, js::Value const&, JSAccessMode, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:867)
==9371==    by 0x4A997A: js_NativeSet(JSContext*, JSObject*, js::Shape const*, bool, bool, js::Value*) (jsscopeinlines.h:293)

Comment 4

[Johnny Stenback (:jst)]

Brian, can you please suggest a sg: severity rating for this but per https://wiki.mozilla.org/Security_Severity_Ratings.

Comment 5

[Brian Hackett [Laid off!]]

Per comment 2 this should not affect TM, but that is only lucky and small changes to the surrounding code could trigger buffer overflows.

Comment 6

[Christian Holler (:decoder)]

I tried to reproduce this issue on current mozilla-central (which includes TI now) and I was not able to produce a crash or assert.

Brian, is there any more work required on this issue or can this be closed?

Comment 7

[Daniel Veditz [:dveditz]]

Ditto comment 6: can this bug be resolved? Can it be un-hidden?

Comment 8

[Brian Hackett [Laid off!]]

This is fixed on trunk through beta, due to ObjShrink, but it may affect ESR branches like Firefox 10 and 3.6.* (?)

Comment 9

[Christian Holler (:decoder)]

According to comment 8, this should be fixed in all supported builds by now, including ESR. Marking as FIXED.

Comment 10

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 11

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

(In reply to Brian Hackett (:bhackett) from comment #8)
> This is fixed on trunk through beta, due to ObjShrink, but it may affect ESR
> branches like Firefox 10 and 3.6.* (?)

Wouldn't ESR 17 be affected by this issue?

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Al Billings [:abillings] from comment #11)
> (In reply to Brian Hackett (:bhackett) from comment #8)
> > This is fixed on trunk through beta, due to ObjShrink, but it may affect ESR
> > branches like Firefox 10 and 3.6.* (?)
> 
> Wouldn't ESR 17 be affected by this issue?

Nope, that is unlikely, Brian made this comment in Feb 2012, so the only ESR then was 10. Trunk then was Firefox 13.