Assertion failure: entry_ == makeIndex(clasp, key, kind), at vm/Runtime.h

VERIFIED FIXED in Firefox 34

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: gkw, Assigned: jonco)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla35
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox32 wontfix, firefox33 wontfix, firefox34+ verified, firefox35 verified, firefox-esr31 unaffected, b2g-v1.4 unaffected, b2g-v2.0 disabled, b2g-v2.0M disabled, b2g-v2.1 disabled, b2g-v2.2 fixed)

Details

(Whiteboard: [jsbugmon:][adv-main34+])

Attachments

(1 attachment)

Reporter

Description

5 years ago
for (var i = 0; i < 200; ++i) {
    Object.getOwnPropertyNames(undefined + "");
}
function p(s) {
    for (var i = 0; i < s.length; i++) {
        s.charCodeAt(i);
    }
}
function m(f) {
    var a = [];
    for (var j = 0; j < 700; ++j) {
        try {
            f()
        } catch (e) {
            a.push(e.toString());
        }
    }
    p(uneval(a));
}
f = Function("\
    function f() {\
        functionf\n{}\
    }\
    m(f);\
");
f();
f();
for (var x = 0; x < 99; x++) {
    newGlobal()
}

asserts the following js debug shell on m-c changeset 6c824fbb73e8 with --no-asmjs --no-baseline --no-ion --no-threads at Assertion failure: entry_ == makeIndex(clasp, key, kind), at vm/Runtime.h.

https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-01-mozilla-central-debug/jsshell-mac64.zip

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140930050920" and the hash "1de7e7a7ad8d".
The "bad" changeset has the timestamp "20140930052624" and the hash "f33f7dc50aa9".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1de7e7a7ad8d&tochange=f33f7dc50aa9

Jon, is bug 1073577 a possible regressor? (Setting this as s-s because bug 1073577 is s-s.)
Flags: needinfo?(jcoppeard)
Reporter

Comment 1

5 years ago
There are no nice stacks available for js shells from tinderbox-builds.
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Reporter

Comment 3

5 years ago
This does not reproduce for locally compiled builds but seems to reproduce readily with tinderbox-builds ones.
Assignee

Comment 4

5 years ago
(In reply to Gary Kwong [:gkw] [:nth10sd] GMT-4 Sep 29 - Oct 3 from comment #0)
Bug 1073577 added the assertion, but this may be a pre-existing problem.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Assignee

Comment 5

5 years ago
Posted patch bug1075546Splinter Review
Repros fine for me with the attached testcase.

The problem was the fix for bug 1073577 was incomplete.  When recording the current GC number, it's important to record it *before* a GC can happen :)

With this patch, the gc number is recorded only if the cache lookup fails.  If it succeeds, there are two cases - either we create a new object successfully and return, or we fail and GC.  In the latter case we just leave gcNumber initialized to zero, which means the later test for GC will always fail.
Attachment #8499493 - Flags: review?(terrence)
Attachment #8499493 - Flags: review?(terrence) → review+
Assignee

Comment 6

5 years ago
Comment on attachment 8499493 [details] [diff] [review]
bug1075546

[Security approval request comment]

This is a followup to fix an issue with the fix in bug 1073577, so the details here are mostly the same.

How easily could an exploit be constructed based on the patch?

Difficult.

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

The contents of the patch indicate an issue with GCs occurring in specific places, but I don't think it's easy to take advantage of that.

Which older supported branches are affected by this flaw?

As per bug 1073577, everything back to FF 32.

If not all supported branches, which bug introduced the flaw?

Generational GC enabled on desktop in bug 619558.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Backporting should be combined with bug 1073577, the patch is easy to create and should apply without problems.

How likely is this patch to cause regressions; how much testing does it need?

Should be low risk, again it would be a good idea to let it sit on central for a week first.
Attachment #8499493 - Flags: sec-approval?
Comment on attachment 8499493 [details] [diff] [review]
bug1075546

This is too late to make it into 33. We're building soon for the release build. We should get this on Aurora though. Can you make and nominate an Aurora patch?
What is the security severity on this? bug 1073577 is a sec-audit, though I don't know if that's actually correct.
Assignee

Comment 9

5 years ago
(In reply to Al Billings [:abillings] from comment #8)
I've nominated the bug 1073577 for aurora/beta uplift.  The patch here will apply cleanly after that one.  I'm waiting on sec-approval to land this though (my guess at a security rating is for the original bug is sec-high).
Attachment #8499493 - Flags: sec-approval? → sec-approval+
Keywords: sec-high
Jon, please go ahead and do the Aurora approval request when you push this to inbound so we don't wait another day to cycle around.
https://hg.mozilla.org/mozilla-central/rev/4007aeccec79
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee

Comment 13

5 years ago
Comment on attachment 8499493 [details] [diff] [review]
bug1075546

Approval Request Comment
[Feature/regressing bug #]: Bug 619558 (fix in bug 1073577 is incomplete)
[User impact if declined]: Potential security vulnerability
[Describe test coverage new/current, TBPL]: Test case in bug
[Risks and why]: Low
[String/UUID change made/needed]: None
Attachment #8499493 - Flags: approval-mozilla-aurora?
Attachment #8499493 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1075598
Reproduced the original issue using the steps from comment #0:

* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/10/2014-10-01-mozilla-central-debug/
** Assertion: Assertion failure: entry_ == makeIndex(clasp, key, kind), at /builds/slave/m-cen-osx64-d-0000000000000000/build/js/src/vm/Runtime.h:334

Verified using the following builds:

* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-central-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion

* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-aurora-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion

* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-beta-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion
Status: RESOLVED → VERIFIED
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main34+]

Updated

4 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.