559256 - bad oom-handling in js_GrowSlots, js_AllocSlots

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: fix

js_GrowSlots does not check for null being returned by the cx->realloc at the end.  crash-stats shows 26 crashes in js_GrowSlots in the last week, many pointing to the exact line after realloc, so its definitely hitting.  Dereferencing null, so no security vulnerability.

js_AllocSlots does check for cx->malloc returning null, but returns true.  The callers seem to assume true means success, so this seems wrong.  This bug seems scarier, since callers may go on to assume obj->dslots is the size requested and write past the end of dslots.  Maybe there is a good argument as to why this cannot be exploited but I can't see it, so I'm defaulting to security confidential.

Comment 1

[Brendan Eich [:brendan]]

OOM is gal's kryptonite.

/be

Comment 2

[Luke Wagner [:luke]]

Attachment: patch

Although crashstats indicates that it doesn't happen in practice (why would a sane CRT fail for a shrinking realloc?), cx->realloc is also unchecked in js_ShrinkSlots.

Comment 3

[Brendan Eich [:brendan]]

We have seen FreeBSD (ulp, jemalloc?) realloc fail when asked to shrink. Typical size classifying problem.

/be

Comment 4

[Luke Wagner [:luke]]

Ah, then my apologies to them for questioning their CRT's sanity.

Comment 5

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/20fa375ca096

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/20fa375ca096

Comment 7

[Christian Holler (:decoder)]

Luke, did this affect 1.9.2 and if so, was it fixed there? If it was fixed or did not affect 1.9.2, is there any reason to keep this locked?

Comment 8

[Luke Wagner [:luke]]

js_ShrinkSlots on 1.9.2 appears not to check realloc's return value.  However, on oom, it always derefs 0 so produces a safe (and extremely rare) crash.  js_ShrinkSlots has a void return type on 1.9.2 so it isn't a tiny fix.  Given this poor risk/reward I would elect not to fix.