Closed Bug 944321 Opened 6 years ago Closed 6 years ago

--ion-check-range-analysis failure with Float32Array (SIGTRAP)

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g koi+
Tracking Status
firefox26 --- wontfix
firefox27 + verified
firefox28 + verified
firefox29 + verified
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: gkw, Assigned: sunfish)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main27+])

Attachments

(3 files, 1 obsolete file)

Attached file stack
toString = (function() {
    function f() {
        Float32Array()[6] = 33554433
    }
    return f
})()
this + Object({
    e: this % 1
})

causes a SIGTRAP in js opt shell on m-c changeset 77b5c6edfe96 with --ion-check-range-analysis --ion-eager --ion-gvn=off

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/c78a87a9aa66
user:        Nathan Froyd
date:        Tue Aug 27 16:32:44 2013 -0400
summary:     Bug 909922 - don't pragma pack ipc message headers; r=bent

but I think autoBisect is wrong here.

Adding :sunfish as well, as this seems to involve --ion-check-range-analysis. s-s because SIGTRAPs can be bad.
Flags: needinfo?(sunfish)
There is a 33554432 where range analysis expects 33554433. 33554433 rounded to float32 is 33554432.

The SIGTRAP is just --ion-check-range-analysis' way of reporting an error. I should probably convert that to use something like what OsiPointRegisterCheckFailed() does so that it's not as mysterious.

Range analysis errors can theoretically be manipulated into bypassed bounds checks, so security-sensitive is prudent.
Summary: Crash at SIGTRAP involving Float32Array → --ion-check-range-analysis failure with Float32Array
Simpler:
./js --ion-check-range-analysis --ion-eager --ion-gvn=off

function f() {
    (new Float32Array(1))[0] = 33554433;
}
f();
f();
Summary: --ion-check-range-analysis failure with Float32Array → --ion-check-range-analysis failure with Float32Array (SIGTRAP)
The attached patch fixes the bug by applying the somewhat heavy hammer of disabling range analysis for MToFloat32, in case that's desired.

Also, this bug is a dup of 940638, which should probably also be security-sensitive (I don't seem to have the ability to change that myself).
Flags: needinfo?(sunfish)
> Also, this bug is a dup of 940638, which should probably also be
> security-sensitive (I don't seem to have the ability to change that myself).

Changed. (feel free to dupe as necessary)
Duplicate of this bug: 940638
Blocks: 888109
Keywords: sec-high
sunfish, can we land that patch or a more targeted fix?
Flags: needinfo?(sunfish)
A more targeted fix needs something like bug 939843, so I'll pursue the simple fix for now. Attached is a refreshed patch.
Attachment #8341115 - Attachment is obsolete: true
Attachment #8344610 - Flags: review?(benj)
Flags: needinfo?(sunfish)
Attachment #8344610 - Flags: review?(benj) → review+
Comment on attachment 8344610 [details] [diff] [review]
disable-tofloat32-computerange.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Someone would have to be very familiar with range analysis optimizations to pull this off, but it's presumably possible.

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

The patch makes it clear where the bug is, but not how to exploit it.

Which older supported branches are affected by this flaw?

It's in mozilla-release, and probably others. As of this writing any branch which contains MToFloat32::computeRange (in RangeAnalysis.cpp) contains this flaw.

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

Bug 888109.

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

Backports would be trivial to create, and very low-risk.

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

Very unlikely. The patch is a single deleted line, disabling an optimization.
Attachment #8344610 - Flags: sec-approval?
Attachment #8344610 - Flags: sec-approval? → sec-approval+
landed on central https://hg.mozilla.org/mozilla-central/rev/a29676c924d7
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Dan, can you please nominate this for Aurora/Beta/b2g26 uplift?
Assignee: general → sunfish
Flags: needinfo?(sunfish)
Comment on attachment 8344610 [details] [diff] [review]
disable-tofloat32-computerange.patch

[Triage Comment]
Attachment #8344610 - Flags: approval-mozilla-beta+
Attachment #8344610 - Flags: approval-mozilla-aurora+
The original patch seems to have acquired aurora approval already. Attached is a version of the patch rebased to apply cleanly on mozilla-beta and b2g26, and corresponding nominations. Here's the approval form:

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 888109
User impact if declined: security-relevant issue goes unfixed
Testing completed: On mozilla-central for weeks
Risk to taking this patch (and alternatives if risky): very low
String or UUID changes made by this patch: none
Attachment #8351710 - Flags: approval-mozilla-beta?
Attachment #8351710 - Flags: approval-mozilla-b2g26?
Flags: needinfo?(sunfish)
Attachment #8351710 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
blocking-b2g: --- → koi?
koi+ for sec-high bug
blocking-b2g: koi? → koi+
Attachment #8351710 - Flags: approval-mozilla-b2g26?
Confirmed SIGTRAP on m-c, 2013-11-28.
Verified fixed on beta, 2014-01-20.
Verified fixed on Aurora, 2014-01-21.
Verified fixed on m-c, 2014-01-21.
Whiteboard: [adv-main27+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.