The default bug view has changed. See this FAQ.

"Assertion failure: enumerators == cx->enumerators,"

VERIFIED FIXED in mozilla14

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: luke)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla14
x86
Linux
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox12 unaffected, firefox13 affected, firefox-esr10 unaffected)

Details

(Whiteboard: [sg:moderate] js-triage-done)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
mjitChunkLimit(31)
o = {}
o.valueOf = function() {
    for (var p in undefined) {
        a = new Function;
    }
    +o;
};
+o;

asserts 32-bit js debug shell *intermittently* on m-c changeset 92fe907ddac8 with -m and -n at Assertion failure: enumerators == cx->enumerators,

May be related to bug 736012. s-s because that bug is s-s.

(gdb) bt 10
#0  0xf776a430 in __kernel_vsyscall ()
#1  0xf773bebe in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42
#2  0x083ade82 in MOZ_Crash () at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:79
#3  0x083adede in MOZ_Assert (s=0x83ed73c "enumerators == cx->enumerators", file=0x83ed5cc "/home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp", ln=453) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:88
#4  0x08138a52 in js::CheckStackBalance::~CheckStackBalance (this=0xff864190, __in_chrg=<optimized out>) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:453
#5  0x0813896c in js::RunScript (cx=0x9008d38, script=0xf6c06128, fp=0xf6e60c38) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:455
#6  0x08138d92 in js::InvokeKernel (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:528
#7  0x080930c3 in js::Invoke (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.h:172
#8  0x08138f55 in js::Invoke (cx=0x9008d38, thisv=..., fval=..., argc=0, argv=0x0, rval=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:560
#9  0x08175aab in js::MaybeCallMethod (cx=0x9008d38, obj=0xf6c0c3d0, id=..., vp=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsobj.cpp:5553
(More stack frames follow...)

(2000+ stacktrace lines follow)
(Assignee)

Comment 1

5 years ago
Ah, so the basic issue is that we rejoin the interpreter and a JSOP_ENDITER and then immediately hit the C stack recursion limit.  This jumps to error handling which assumes that, if we are parked on JSOP_ENDITER, we have already executed CloseIterator, so it doesn't unwind the iterator, leaving cx->enumerators invalid.  I'm thinking of what is the least awful fix...
(Assignee)

Comment 2

5 years ago
Created attachment 610995 [details] [diff] [review]
fix

After talking with bhackett, we decided that the real culprit here is js::Interpret executing 'error' before jumping to *regs.pc: the assumption in interpreter and mjit stack-unwinding is that, if you throw at pc, you at least started to execute *pc.  That means it is correct for unwinding to not execute CloseIterator when *pc == JSOP_ENDITER: if we started executing JSOP_ENDITER we must have already called CloseIterator.
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #610995 - Flags: review?(bhackett1024)
Attachment #610995 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a185f034768
Whiteboard: js-triage-needed → js-triage-done
Target Milestone: --- → mozilla14
Can content influence the mjitChunkLimit? Is that setting necessary to this bug? Trying to guess whether this is an exploitable bug we need to worry about, and whether we need to back-port to the ESR.
(In reply to Daniel Veditz [:dveditz] from comment #4)
> Can content influence the mjitChunkLimit? Is that setting necessary to this
> bug?

Content cannot influence the chunk limit, it is a fixed value there. However, I don't think it's necessary to modify the value, you just need to make the code larger to you hit the boundary in the right place with the default value.
(Assignee)

Comment 6

5 years ago
Hitting this bug is extremely difficult.  One, you have to hit the chunk limit in just the right way, but you also have to exhaust the C stack at exactly the right place from the right op, etc.

Comment 7

5 years ago
I wouldn't count on it being hard to exhaust the C stack at exactly the right place.
(Assignee)

Comment 8

5 years ago
I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10 only on windows...
(Assignee)

Comment 9

5 years ago
Oh, I thought this was a question about backporting to aurora, but comment 4 is asking about ESR.  I don't think bug 732744 is on ESR.
(Reporter)

Comment 10

5 years ago
(In reply to Luke Wagner [:luke] from comment #8)
> I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10
> only on windows...

To clarify, this repros only in 32-bit Linux and Windows. I don't think I've managed to reproduce in 64-bit Linux or Mac (both architectures).

Time taken to find a testcase is not a good gauge of how easy / hard it is to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer* to hit the bug.
(Assignee)

Comment 11

5 years ago
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #10)
> to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer*
> to hit the bug.

Sure, in general, but this is a fuzzer-only type of bug.  Given that the bug isn't on a long-term release, it seems fine to let the fix ride the train out.
(Reporter)

Updated

5 years ago
Blocks: 732744
I think comment 9 is implying this is a regression from bug 732744 which is why we don't need it on ESR.

Taking a SWAG at sg:moderate given comment 6 explaining you'd need to reliably hit two conditions at the same time to exploit.
status-firefox-esr10: --- → unaffected
status-firefox12: --- → unaffected
status-firefox13: --- → affected
status-firefox14: --- → affected
Keywords: regression
Whiteboard: js-triage-done → [sg:moderate] js-triage-done
(Reporter)

Comment 13

5 years ago
This landed on mozilla-central:

https://hg.mozilla.org/mozilla-central/rev/3a185f034768
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
(Reporter)

Updated

5 years ago
Duplicate of this bug: 742094
Group: core-security

Updated

5 years ago
status-firefox14: affected → ---
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.