Bug 748613 (CVE-2012-1939)

[ESR] Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348




JavaScript Engine
5 years ago
4 years ago


(Reporter: decoder, Assigned: luke)


(Blocks: 1 bug, {assertion, sec-critical, testcase})

10 Branch
assertion, sec-critical, testcase
Dependency tree / graph

Firefox Tracking Flags

(firefox12 fixed, firefox13 unaffected, firefox14 unaffected, firefox15 unaffected, firefox-esr1013+ verified)


(Whiteboard: [sg:critical][qa+:ashughes][advisory-tracking+] fixed on m-c by 692274)


(1 attachment)



5 years ago
The following test asserts on mozilla-esr10 revision 65efc6ce102f (options -m -n -a):

try {
Object.prototype.nameSETS = 0;
function ownProperties() {
    var props = {};
    var r = function () {};
    for (var a in r) {
        let (a = function() { for (var r=0;r<6;++r) ++a; }) {
        props[a] = true;
} catch(exc1) {}

According to bisection, the issue might have been fixed on mozilla-central by this revision:

The first good revision is:
changeset:   83258:9272bb82eeba
user:        Luke Wagner
date:        Fri Oct 07 12:02:50 2011 -0700
summary:     Bug 692274, part 3 - Remove JSOP_BLOCKCHAIN and JSOP_NULLBLOCKCHAIN, which produces incorrect let scoping until the next patch (r=jorendorff)

Marking s-s because infer failures can be security related.

Comment 1

5 years ago
Bug 692274 is not a bug you want to backport.
See bug 703857 and bug 709633.

Comment 3

5 years ago
If this is bug 703857, then it is sg:critical and we must fix it somehow on ESR.

Comment 4

5 years ago
I don't know what the actual error is.  Bug 703857 comment 1 seems to.  Brian: is there some coarse-grained de-optimization we can apply here like "assume all let vars are closed" or something?
This is the same problem (and testcase) as bug 703857, the upvar analysis around let variables is broken pre-692274.  The problem is with the 'var a' in the outer function being mislabelled by the frontend as not closed.  TI already treats let variables as totally unknown, so there isn't an easy deoptimization I can see and an actual fix (or 692274 backport) is needed.
I agree w/Luke's comment 1. -- any fixes short of backporting the whole 300K patch?

At first I was hopeful given the Oct date of the bisect in comment 0 since that would be in Firefox 10/ESR. But that's the changeset date, the m-c push date was actually late December (Firefox 12).
status-firefox12: --- → fixed
status-firefox13: --- → unaffected
status-firefox14: --- → unaffected
status-firefox15: --- → unaffected
Depends on: 692274
Summary: Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348 → [ESR] Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348
Whiteboard: js-triage-needed → [sg:critical] fixed on m-c by 692274

Comment 7

5 years ago
Created attachment 618388 [details] [diff] [review]

Alright, I found the underlying bug that got swept away by bug 692274.  Pretty straight-forward.
Assignee: general → luke
Attachment #618388 - Flags: review?(jorendorff)
Comment on attachment 618388 [details] [diff] [review]

Nice work. Want to add the test? Slightly more minimal:

(function () {
    var a = "0";
    let (a = function() { ++a; }) {
    "" + a;
Attachment #618388 - Flags: review?(jorendorff) → review+
Keywords: sec-critical
Tracking for ESR shipping with Firefox 13, we won't chemspill the current ESR release already out there, which is affected, so this is the best we can do.
tracking-firefox-esr10: ? → 13+
Please nominate for approval-mozilla-esr and we can approve/land closer to the release date.

Comment 11

5 years ago
Comment on attachment 618388 [details] [diff] [review]

[Approval Request Comment]
Fix Landed on Version: not on trunk
Risk to taking this patch (and alternatives if risky): less than backporting the giant patch, but not zero
Attachment #618388 - Flags: approval-mozilla-esr10?

Comment 12

5 years ago
Since this is an ESR-only landing, should I be including a test-case that points directly to the bug (as suggested in comment 8)?
Comment on attachment 618388 [details] [diff] [review]

approved for landing to esr10, if it's not too much to also land the test case, I'm always a fan of tests :)
Attachment #618388 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+

Comment 14

5 years ago
status-firefox-esr10: affected → fixed


5 years ago
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] fixed on m-c by 692274 → [sg:critical][qa+] fixed on m-c by 692274
Whiteboard: [sg:critical][qa+] fixed on m-c by 692274 → [sg:critical][qa+][advisory-tracking+] fixed on m-c by 692274
Verified fixed in Firefox 10.0.5esrpre 2012-05-31.
status-firefox-esr10: fixed → verified
Whiteboard: [sg:critical][qa+][advisory-tracking+] fixed on m-c by 692274 → [sg:critical][qa+:ashughes][advisory-tracking+] fixed on m-c by 692274
Alias: CVE-2012-1939
Group: core-security
You need to log in before you can comment on or make changes to this bug.