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

VERIFIED FIXED in Firefox 34, Firefox OS v2.2

Status

()

Core
JavaScript: GC
--
critical
VERIFIED FIXED
3 years ago
a year ago

People

(Reporter: gkw, Assigned: jonco)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla35
x86_64
Mac OS X
assertion, regression, sec-high, testcase
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

3 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

3 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

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

Comment 4

3 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

3 years ago
Created attachment 8499493 [details] [diff] [review]
bug1075546

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

3 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?
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → disabled
status-b2g-v2.0M: --- → disabled
status-b2g-v2.1: --- → disabled
status-b2g-v2.2: --- → fixed
status-firefox32: --- → wontfix
status-firefox33: --- → affected
status-firefox34: --- → affected
status-firefox-esr31: --- → unaffected
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?
status-firefox33: affected → wontfix
tracking-firefox34: --- → +
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

3 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.
(Assignee)

Comment 11

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4007aeccec79
https://hg.mozilla.org/mozilla-central/rev/4007aeccec79
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox35: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
(Assignee)

Comment 13

3 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+
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/75ebb70f8b38
status-firefox34: affected → fixed
Flags: in-testsuite+
Keywords: checkin-needed
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
status-firefox34: fixed → verified
status-firefox35: fixed → verified
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main34+]

Updated

2 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.