Closed Bug 751320 Opened 8 years ago Closed 8 years ago

JavaScript variable intermittently not assigned value after operation (Firefox; stand-alone test case)

Categories

(Core :: JavaScript Engine, defect, critical)

12 Branch
x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla15
Tracking Status
firefox12 --- affected
firefox13 + fixed
firefox14 + fixed
firefox15 --- verified
firefox-esr10 --- unaffected

People

(Reporter: tbhorton, Assigned: bhackett)

References

Details

(Keywords: regression, testcase)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

Exercised functionality in my company's webapp after customers complained about data corruption.  Their complaints were reproducible only in Firefox 12 (which they had recently upgraded to) *without* any Firebug panels activated.


Actual results:

Essentially, the values returned by a function called repeatedly in a for loop began to yield different results for the same output.  Please see the attached stand-alone test case.


Expected results:

The stand-alone test case will execute correctly in Firefox 11 (or lower), Safari, and Chrome.  Two alerts will appear indicating "START test" and "END test" with no failure notices.  However, opening the test page in FF12 (without Firebug panel activated) will indicate a "test FAILED..." condition in an alert.

While a malicious attack may be difficult to construct from this test case, I am still marking this as a security bug since the outcome of such a basic failure is so unpredictable.
This test page fails on FF12 on Windows and MacOS X (and probably others).
Severity: normal → critical
OS: Mac OS X → All
Attachment #620434 - Attachment mime type: text/plain → text/html
I confirm that the behavior is different between 11 and 12.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hopefully this is the same underlying bug as bug 748044.
Tracking Firefox 13 as a web-compat regression
Assignee: general → luke
Keywords: regression, testcase
Version: unspecified → 12 Branch
I don't think this introduces any general vulnerability in Firefox. It may be a security issue for some rare web application, but we'll make better progress on a fix if more people can see the bug.
Group: core-security
Attached patch patchSplinter Review
Thanks for the great testcase!

This is a miscompilation where if a variable changes from int to double within a switch statement, then after jumping to case labels after the variable turns to a double the compiled code may treat the variable as a double even though it is still represented as an integer.  The fix adds checks to ensure the variable has the correct representation when jumping to such case labels.
Assignee: luke → bhackett1024
Attachment #620722 - Flags: review?(dvander)
Awesome, can we dup bug 748044 to this?
(In reply to Luke Wagner [:luke] from comment #8)
> Awesome, can we dup bug 748044 to this?

I think so, but I need to see more of the malfunctioning switch in bug 748044 to confirm.
Attachment #620722 - Flags: review?(dvander) → review+
https://hg.mozilla.org/mozilla-central/rev/b9c801fd1b52
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Verified fixed in nightly trunk build from changset: http://hg.mozilla.org/mozilla-central/rev/448f554f6acb.
Status: RESOLVED → VERIFIED
Is this bug sufficiently low risk or does it have significant enough of a web compatibility impact that we should land now on FF13? If so, please nominate prior to 5/21.
Comment on attachment 620722 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: Some websites may behave incorrectly (confusing integers and doubles, no s-s).
Risk to taking this patch (and alternatives if risky): Very low.
Attachment #620722 - Flags: approval-mozilla-beta?
Attachment #620722 - Flags: approval-mozilla-aurora?
(In reply to Brian Hackett (:bhackett) from comment #14)
> Comment on attachment 620722 [details] [diff] [review]
> patch
> 
> [Approval Request Comment]
> User impact if declined: Some websites may behave incorrectly (confusing
> integers and doubles, no s-s).
> Risk to taking this patch (and alternatives if risky): Very low.

A little extra background for the approval requests:
 - The bug did ship in Fx12, so if 13 goes out with the bug, at least we are not regressing further.
 - Bug 748044, breakage of a Mandreel game, is believed to a be a dup of this bug, so that's an extra user impact if declined. Rumor has it that Mandreel worked around but I have no way to directly verify that.
Comment on attachment 620722 [details] [diff] [review]
patch

[Triage Comment]
Given the low-risk evaluation and confirmed user impact, approving for Aurora 14 and Beta 13.
Attachment #620722 - Flags: approval-mozilla-beta?
Attachment #620722 - Flags: approval-mozilla-beta+
Attachment #620722 - Flags: approval-mozilla-aurora?
Attachment #620722 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/9fd1222af3b1

(Beta has been closed since yesterday and I can't find anyone on IRC to open it up.)
No longer blocks: 704387
No longer depends on: 748044
Blocks: 704387
Depends on: 748044
Beta is open again, please go ahead with landing.
Target Milestone: mozilla15 → mozilla13
Target milestone tracks when it landed on m-c. The tracking flags track uplifts.
Target Milestone: mozilla13 → mozilla15
Duplicate of this bug: 748044
You need to log in before you can comment on or make changes to this bug.