Crash [@ js::jit::JitFrameIterator::script] with OOM and invalid pointer

RESOLVED FIXED in Firefox 50



3 years ago
3 years ago


(Reporter: decoder, Assigned: jandem)


(Blocks 2 bugs, 4 keywords)

Dependency tree / graph
Bug Flags:
in-testsuite ?
qe-verify -

Firefox Tracking Flags

(firefox48 unaffected, firefox49 unaffected, firefox-esr45 unaffected, firefox50+ fixed, firefox51+ fixed, firefox52 fixed)


(Whiteboard: [jsbugmon:ignore][post-critsmash-triage], crash signature)


(2 attachments)

The following testcase crashes on mozilla-central revision e78975b53563 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --thread-count=2):

See attachment.


 received signal SIGSEGV, Segmentation fault.
js::jit::JitFrameIterator::script (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:213
#0  js::jit::JitFrameIterator::script (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:213
#1  0x00000000006b4a92 in js::jit::JitFrameIterator::checkInvalidation (this=this@entry=0x7fffffffad10, ionScriptOut=ionScriptOut@entry=0x7fffffffa5c0) at js/src/jit/JitFrames.cpp:149
#2  0x00000000006b626b in js::jit::JitFrameIterator::ionScript (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2208
#3  0x00000000006b68c1 in js::jit::JitFrameIterator::safepoint (this=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2226
#4  0x00000000006b6928 in js::jit::JitFrameIterator::machineState (this=this@entry=0x7fffffffad10) at js/src/jit/JitFrames.cpp:297
#5  0x00000000006bc1e4 in js::jit::InlineFrameIterator::resetOn (this=0x7fffffffae40, iter=0x7fffffffad10) at js/src/jit/JitFrames.cpp:2290
#6  0x00000000006c96a3 in js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:784
#7  0x00007ffff7fe6646 in ?? ()
#12 0x0000000000000000 in ?? ()
rax	0x807ffeffb240	141287227372096
rbx	0x7fffffffad10	140737488334096
rcx	0x30	48
rdx	0x7fffffffb1d0	140737488335312
rsi	0x7fffffffa5c0	140737488332224
rdi	0x7fffffffad10	140737488334096
rbp	0x7fffffffa590	140737488332176
rsp	0x7fffffffa590	140737488332176
r8	0x30	48
r9	0x7ffff24fffa0	140737258717088
r10	0x2	2
r11	0x7ffff7e9bdf8	140737352678904
r12	0x7fffffffa5c0	140737488332224
r13	0x7fffffffa8c0	140737488332992
r14	0x7fffffffad10	140737488334096
r15	0x7fffffffad10	140737488334096
rip	0x6b49f9 <js::jit::JitFrameIterator::script() const+25>
=> 0x6b49f9 <js::jit::JitFrameIterator::script() const+25>:	mov    0x10(%rax),%rdi
   0x6b49fd <js::jit::JitFrameIterator::script() const+29>:	callq  0x6b2030 <js::jit::ScriptFromCalleeToken(js::jit::CalleeToken)>

The testcase here is intermittent. Sometimes it crashes after only 1-2 seconds, sometimes it runs for 30 seconds before crashing. The memory access looks sec-critical, marking as such.
Posted file Testcase
Naveed can you help find someone to work on this sec-critical bug?
Flags: needinfo?(nihsanullah)
David can you help out here ? Maybe you can help find an owner to investigate this since it is sec-critical and may affect beta and other versions.
Flags: needinfo?(dbolter)
Jandem, any thoughts?
Flags: needinfo?(dbolter) → needinfo?(jdemooij)
I'll take a look tomorrow.
Flags: needinfo?(nihsanullah)
Posted patch PatchSplinter Review
Oops, regression from bug 1272598. We shouldn't jump to masm.exceptionLabel() when ArgumentsObject::finishForIon fails, because we didn't enter an exit frame.

This patch changes the code to just fall back to the slow path.
Assignee: nobody → jdemooij
Flags: needinfo?(jdemooij)
Attachment #8783926 - Flags: review?(nicolas.b.pierron)
Attachment #8783926 - Flags: review?(nicolas.b.pierron) → review+
Jan, can we land this? Is this blocked on something to request sec-approval and uplift?
Flags: needinfo?(jdemooij)
It only affects 50 and 51. If someone asked for sec-approval+ to land on trunk and Aurora approval, I'd give it.
Duplicate of this bug: 1301608
Comment on attachment 8783926 [details] [diff] [review]

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Not trivial but not that difficult...

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

> Which older supported branches are affected by this flaw?
Aurora, beta (50+).

> If not all supported branches, which bug introduced the flaw?
Bug 1272598.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should apply or be easy.

> How likely is this patch to cause regressions; how much testing does it need?
Flags: needinfo?(jdemooij)
Attachment #8783926 - Flags: sec-approval?
(In reply to Jan de Mooij [:jandem] from comment #10)
> > Which older supported branches are affected by this flaw?
> Aurora, beta (50+).

Only aurora, actually, since the merge was moved to next week.
Comment on attachment 8783926 [details] [diff] [review]

sec-approval+ for trunk.
Please request Aurora approval now so we can get this onto 50 before it becomes Beta as well.
Attachment #8783926 - Flags: sec-approval? → sec-approval+
Comment on attachment 8783926 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Bug 1272598.
[User impact if declined]: Crashes on OOM, potential security bugs.
[Describe test coverage new/current, TreeHerder]: Fixes the fuzz test.
[Risks and why]: Low risk. Only affects an OOM case and hitting that here should be unlikely.
[String/UUID change made/needed]: None.
Attachment #8783926 - Flags: approval-mozilla-beta?
Attachment #8783926 - Flags: approval-mozilla-aurora?
Closed: 3 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8783926 [details] [diff] [review]

Sec-crit issue, Aurora51+, Beta50+
Attachment #8783926 - Flags: approval-mozilla-beta?
Attachment #8783926 - Flags: approval-mozilla-beta+
Attachment #8783926 - Flags: approval-mozilla-aurora?
Attachment #8783926 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 1297142
Group: javascript-core-security → core-security-release
Duplicate of this bug: 1299103
Flags: qe-verify-
Whiteboard: [jsbugmon:ignore] → [jsbugmon:ignore][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.