Closed
Bug 1064346
Opened 10 years ago
Closed 10 years ago
Crash [@ IsInsideNursery] with use-after-free (likely OOM)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: decoder, Assigned: terrence)
Details
(4 keywords, Whiteboard: [adv-main33+][adv-esr31.2+])
Crash Data
Attachments
(2 files)
3.70 KB,
application/zip
|
Details | |
1.82 KB,
patch
|
billm
:
review+
abillings
:
approval-mozilla-aurora+
abillings
:
approval-mozilla-beta+
abillings
:
approval-mozilla-esr31+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The attached testcase crashes on mozilla-central revision 2255d7d187b2 (run with --no-threads --fuzzing-safe).
Reporter | ||
Comment 1•10 years ago
|
||
This looks like a memory-sensitive issue (likely OOM) and it only reproduced for me in an optimized build (--disable-debug --enable-optimize --enable-valgrind --enable-gczeal).
Backtrace:
Program received signal SIGSEGV, Segmentation fault.
IsInsideNursery (cell=0x4b4b4b4b4b4b4b4b) at ../../dist/include/js/HeapAPI.h:264
264 return location & ChunkLocationAnyNursery;
#0 IsInsideNursery (cell=0x4b4b4b4b4b4b4b4b) at ../../dist/include/js/HeapAPI.h:264
#1 MarkInternal<JSAtom> (trc=0x1650518, thingp=0x7ffff535f0b8) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:285
#2 0x000000000078016d in trace (trc=0x1650518, this=0x7ffff535f080) at /srv/repos/mozilla-central/js/src/jsfun.cpp:720
#3 fun_trace (trc=0x1650518, obj=0x7ffff535f080) at /srv/repos/mozilla-central/js/src/jsfun.cpp:757
#4 0x00000000004fe8e5 in js::GCMarker::processMarkStackTop (this=0x1650518, budget=...) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:1748
#5 0x00000000004d6cad in js::GCMarker::drainMarkStack (this=0x1650518, budget=...) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:1810
#6 0x000000000079c272 in drainMarkStack (phase=js::gcstats::PHASE_MARK, sliceBudget=..., this=0x164fc18) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4817
#7 js::gc::GCRuntime::incrementalCollectSlice (this=0x164fc18, budget=<optimized out>, reason=<optimized out>) at /srv/repos/mozilla-central/js/src/jsgc.cpp:5351
rax 0x4b4fffe8 5425512962856058856
rbx 0x1650518 23397656
rcx 0x20000 131072
rdx 0xa95db6 11099574
rsi 0xf535f0b8 140737307340984
rdi 0x4b4b4b4b 5425512962855750475
rbp 0x1650518 23397656
rsp 0xffffca00 140737488341504
r8 0xc011 49169
r9 0x21080 135296
r10 0x100 256
r11 0x0 8796093022208
r12 0xf5342158 140737307222360
r13 0xf5360088 140737307345032
r14 0x2 2
r15 0xffffcb40 140737488341824
rip 0x4c1d3c <MarkInternal<JSAtom>(JSTracer*, JSAtom**)+76>
=> 0x4c1d3c <MarkInternal<JSAtom>(JSTracer*, JSAtom**)+76>: mov (%rax),%eax
Looks dangerous to me, marking s-s. Ccing Jonco and sfink because it looks related to GGC.
status-firefox35:
--- → affected
Updated•10 years ago
|
Keywords: csectype-uaf
Comment 2•10 years ago
|
||
Can you look at this, Terrence? It looks like Jon is on PTO.
Flags: needinfo?(terrence)
Assignee | ||
Comment 4•10 years ago
|
||
This is yet another path where JSFunction's quirky C-struct overlay on object slots is biting us.
In this case, we're failing a JSFunction allocation at setSingletonType after Object::create and before NewFunction. This leaves a JSObject with JSFunction::class's trace hook in the heap without having initialized it's memory in a way that the trace hook can understand it. In this case, shape->slotSpan() is hiding the slots and leaving them uninitialized; it really doesn't matter though. Whether the values we leave are uninitialized memory, poisoning, or even correct-for-an-object UndefinedValues, when JSFunction::trace looks at atom_, it's going to try to mark gibberish and crash.
We've already hacked up a bunch of paths at various places in the object and gc guts to manually check for JSFunction, so I think the right solution here is just to extend that and make JSObject::create aware that it needs to give JSFunctions C-style POD memory initialization.
Attachment #8487552 -
Flags: review?(wmccloskey)
Attachment #8487552 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8487552 [details] [diff] [review]
bug_1064346-v0.diff
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Pretty difficult.
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Yes.
Which older supported branches are affected by this flaw?
All of them.
If not all supported branches, which bug introduced the flaw?
Nak.
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should be easy to backport.
How likely is this patch to cause regressions; how much testing does it need?
Attachment #8487552 -
Flags: sec-approval?
Comment 6•10 years ago
|
||
How likely is this to cause regressions?
status-firefox32:
--- → wontfix
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox-esr31:
--- → affected
tracking-firefox33:
--- → +
tracking-firefox34:
--- → +
tracking-firefox35:
--- → +
tracking-firefox-esr31:
--- → 33+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Al Billings [:abillings] from comment #6)
> How likely is this to cause regressions?
Very, very unlikely.
Updated•10 years ago
|
Whiteboard: [checkin on 9/23]
Comment 8•10 years ago
|
||
Comment on attachment 8487552 [details] [diff] [review]
bug_1064346-v0.diff
sec-approval+ for checkin on 9/23 or later. We should have Aurora, Beta, and ESR31 patches for then too.
Updated•10 years ago
|
Attachment #8487552 -
Flags: sec-approval? → sec-approval+
Comment 9•10 years ago
|
||
Terrence, can we have the uplift request? Thanks
Flags: needinfo?(terrence)
Keywords: checkin-needed
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8487552 [details] [diff] [review]
bug_1064346-v0.diff
Approval Request Comment
[Feature/regressing bug #]: None - it's an ancient problem.
[User impact if declined]: A potentially exploitable crash.
[Describe test coverage new/current, TBPL]: The fuzzer testcase is too large (and potentially sensitive?) to be useful as a test.
[Risks and why]: Low. This simply adds initialization for JSFunction.
[String/UUID change made/needed]: None.
Attachment #8487552 -
Flags: approval-mozilla-release?
Attachment #8487552 -
Flags: approval-mozilla-esr31?
Attachment #8487552 -
Flags: approval-mozilla-beta?
Attachment #8487552 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(terrence)
Comment 12•10 years ago
|
||
Comment on attachment 8487552 [details] [diff] [review]
bug_1064346-v0.diff
Giving approvals (except for release branch, which I cleared).
Attachment #8487552 -
Flags: approval-mozilla-release?
Attachment #8487552 -
Flags: approval-mozilla-esr31?
Attachment #8487552 -
Flags: approval-mozilla-esr31+
Attachment #8487552 -
Flags: approval-mozilla-beta?
Attachment #8487552 -
Flags: approval-mozilla-beta+
Attachment #8487552 -
Flags: approval-mozilla-aurora?
Attachment #8487552 -
Flags: approval-mozilla-aurora+
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 14•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/6232dcb73aa5
https://hg.mozilla.org/releases/mozilla-beta/rev/fd4720dd6a46
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/41046048a8f7
https://hg.mozilla.org/releases/mozilla-esr31/rev/611e355a3543
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ae553c9bedd1
Comment 15•10 years ago
|
||
Comment 16•10 years ago
|
||
Reproduced the original issue using the information from comment #0 and comment #1 using revision 2255d7d187b2
fx 35 [revision: b85c260821ab] - Passed
fx 34 [revision: ee3cd82e3acb] - Passed
fx 33 [revision: d79568d581e6] - Passed
esr31 [revision: 8c431dcec0ff] - Passed
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•10 years ago
|
Whiteboard: [adv-main33+][adv-esr31.2+]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•