Closed Bug 1152280 (CVE-2015-2712) Opened 4 years ago Closed 4 years ago

Incorrect asm.js bounds check elimination vulnerability (Firefox 37.0.1)

Categories

(Core :: JavaScript Engine, defect)

37 Branch
x86
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- wontfix
firefox38 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- fixed
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: djohnson, Assigned: luke)

Details

(Keywords: sec-critical, Whiteboard: [adv-main38+])

Attachments

(2 files)

Attached file poc.html
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.118 Safari/537.36

Steps to reproduce:

An out-of-bounds read/write vulnerability exists in Firefox 37.0.1 (32-bit), which is similar to (but distinct from) a recent Pwn2Own vulnerability. This was tested on Windows.

To trigger the vulnerability, an asm.js change-heap function must be defined with a minLengthExclusive of 0xFFFFFFFF. This function is parsed by CheckHeapLengthCondition in AsmJSValidate.cpp, which validates this value, then adds one to it, wrapping the value to zero. This zero is then stored as the module's minHeapLength, allowing heaps of any (otherwise valid) size to be used.

In FoldMaskedArrayIndex, one is subtracted from the minHeapLength, wrapping back to 0xFFFFFFFF, which is assumed to be less than the heap size for bounds checking purposes. This function will then omit bounds checks for masks as large as 0x7FFFFFFF, even though much smaller heaps can be used. This allows asm.js code to read and write past the end of its heap.

To reproduce, see the attached proof of concept.


Actual results:

Arbitrary read/write.


Expected results:

Probably an error or something.
OS: Mac OS X → Windows 8.1
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Flags: needinfo?(luke)
Attached patch fix-change-heapSplinter Review
Arg, that's terrible.  Thanks for reporting!

Approval Request Comment
[Feature/regressing bug #]: bug 965880
[User impact if declined]: security vulnerability
[Describe test coverage new/current, TreeHerder]: simple test in patch
[Risks and why]: very low risk, just adds another validation failure case
Assignee: nobody → luke
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8589695 - Flags: review?(benj)
Attachment #8589695 - Flags: approval-mozilla-release?
Attachment #8589695 - Flags: approval-mozilla-beta?
Attachment #8589695 - Flags: approval-mozilla-aurora?
Comment on attachment 8589695 [details] [diff] [review]
fix-change-heap

Review of attachment 8589695 [details] [diff] [review]:
-----------------------------------------------------------------

Oh boy.
Attachment #8589695 - Flags: review?(benj) → review+
Comment on attachment 8589695 [details] [diff] [review]
fix-change-heap

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Fairly easy, if you know what's going on here.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Both the test and patch more or less point to the bug (which is the line after), if you know what to look for.

Which older supported branches are affected by this flaw?
FF35 and up

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The same patch should apply to all branches.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely to cause regressions.
Attachment #8589695 - Flags: sec-approval?
I assume that this is a sec-critical? I need to get a rating before sec-approval. 

Assuming it is, we'll want Beta and Aurora patches (or noms) as well.
Flags: needinfo?(luke)
Yes it is.  I requested branch landing approval in comment 1, or do you mean something different?
Flags: needinfo?(luke)
Keywords: sec-critical
Comment on attachment 8589695 [details] [diff] [review]
fix-change-heap

Sec-approval+ and branch approvals (except for release, which doesn't apply since this isn't a chemspill).
Attachment #8589695 - Flags: sec-approval?
Attachment #8589695 - Flags: sec-approval+
Attachment #8589695 - Flags: approval-mozilla-release?
Attachment #8589695 - Flags: approval-mozilla-beta?
Attachment #8589695 - Flags: approval-mozilla-beta+
Attachment #8589695 - Flags: approval-mozilla-aurora?
Attachment #8589695 - Flags: approval-mozilla-aurora+
Talking to abillings on irc, the current plan is to wait until April 20th to land on trunk (and subsequently branches) to minimize the window between landing and release.
Whiteboard: [checkin on 4/20]
Thanks for getting this organised so quickly. The patch looks good to me.
https://hg.mozilla.org/mozilla-central/rev/4c5586081154
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Flags: sec-bounty? → sec-bounty+
Whiteboard: [adv-main38+]
Alias: CVE-2015-2712
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.