Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Bug 748613 (CVE-2012-1939)

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

RESOLVED FIXED

Status

()

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

People

(Reporter: decoder, Assigned: luke)

Tracking

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

10 Branch
x86_64
Linux
assertion, sec-critical, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(1 attachment)

(Reporter)

Description

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; }) {
            a();
        }
        props[a] = true;
    }
}
ownProperties();
} 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.
(Assignee)

Comment 1

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

Comment 3

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

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

Comment 7

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

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

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

(function () {
    var a = "0";
    let (a = function() { ++a; }) {
        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.
(Assignee)

Comment 11

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

[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?
(Assignee)

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]
fix

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+
(Assignee)

Comment 14

5 years ago
https://hg.mozilla.org/releases/mozilla-esr10/rev/de7fee495798
status-firefox-esr10: affected → fixed
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
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.