Closed Bug 950604 (CVE-2014-1488) Opened 10 years ago Closed 10 years ago

Firefox reproducibly crashes when using asm.js code in workers and transferable objects

Categories

(Core :: JavaScript Engine, defect)

26 Branch
x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla27
Tracking Status
firefox26 - wontfix
firefox27 --- verified
firefox28 --- unaffected
firefox29 --- unaffected
firefox-esr17 --- unaffected
firefox-esr24 28+ verified
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.4 --- unaffected

People

(Reporter: Soeren.Balko, Assigned: sfink)

References

Details

(5 keywords, Whiteboard: [adv-main27+][adv-esr24.4+] appears fixed by bug 861925)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.55 Safari/537.36

Steps to reproduce:

I encountered a reproducible crash bug happening as a result of terminating a worker which passed an ArrayBuffer to the main thread as transferable object. The issue only occurs when the worker ran (Emscripten-generated) asm.js code. The precise steps taken are as follows (test case is attached):

(1) Compile an arbitrary C program into JS, making sure it generates asm.js-compliant code.
(2) Patch the generated JS code to make the internal emscripten HEAP (a large typed array) available to external JS functions
(3) Run the patched, generated JS code in a Web worker and pass the exposed HEAP array to the main UI thread (by using self.postMessage) as a "transferable object".
(4) Upon receiving the transferable object in the main thread, kill the worker by calling its terminate() function.

Please notice that when one of the preconditions (i.e., asm.js code working on top of the HEAP array, passing it to the main thread as transferable, killing the worker) is not met, Firefox does not crash. I only encountered the issue in conjunction with all three conditions.
 


Actual results:

Firefox crashes (other browsers like Chrome happily go along). 

I suspect a double freeing of the transferred ArrayBuffer when the worker is terminated. 


Expected results:

Firefox shouldn't crash, the worker should be terminated, the previously "transferred" array should continue to be accessible in the main thread.
Severity: normal → major
Thank you for the detailed bugreport!

Could you check if this is still reproducible in Firefox 29? (current Nightly version, available from https://nightly.mozilla.org/ - you may want to use a new profile for your tests just to be safe)
Severity: major → critical
Component: Untriaged → JavaScript Engine
Keywords: crash, testcase
Product: Firefox → Core
(In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #1)
> Thank you for the detailed bugreport!
> 
> Could you check if this is still reproducible in Firefox 29? (current
> Nightly version, available from https://nightly.mozilla.org/ - you may want
> to use a new profile for your tests just to be safe)

Oh, and in Firefox 26, if you go to about:crashes, can you copy some of the crash report IDs corresponding to this crash into the bugreport?
Flags: needinfo?(Soeren.Balko)
(In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #1)
> Thank you for the detailed bugreport!
> 
> Could you check if this is still reproducible in Firefox 29? (current
> Nightly version, available from https://nightly.mozilla.org/ - you may want
> to use a new profile for your tests just to be safe)

I tested both the current Nightly versions and can confirm that the bug does NOT occur with this version. I further checked the current Aurora version where it is also fixed.
Flags: needinfo?(Soeren.Balko)
(In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #2)
> (In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #1)
> > Thank you for the detailed bugreport!
> > 
> > Could you check if this is still reproducible in Firefox 29? (current
> > Nightly version, available from https://nightly.mozilla.org/ - you may want
> > to use a new profile for your tests just to be safe)
> 
> Oh, and in Firefox 26, if you go to about:crashes, can you copy some of the
> crash report IDs corresponding to this crash into the bugreport?

These crashes do not seem to appear in about:crashes. With that bug fixed in the upcoming releases, this is perhaps not needed, is it? Unless you still want to perform a root cause analysis in version 26.
(In reply to Soeren.Balko from comment #4)
> (In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #2)
> > (In reply to :Gijs Kruitbosch (Unavailable Dec 19 - Jan 2) from comment #1)
> > > Thank you for the detailed bugreport!
> > > 
> > > Could you check if this is still reproducible in Firefox 29? (current
> > > Nightly version, available from https://nightly.mozilla.org/ - you may want
> > > to use a new profile for your tests just to be safe)
> > 
> > Oh, and in Firefox 26, if you go to about:crashes, can you copy some of the
> > crash report IDs corresponding to this crash into the bugreport?
> 
> These crashes do not seem to appear in about:crashes. With that bug fixed in
> the upcoming releases, this is perhaps not needed, is it? Unless you still
> want to perform a root cause analysis in version 26.

An excellent question. I'm not sure because I'm not at home in js engine land. Looks like sfink worked on some of the relevant transferable object bugs... maybe he knows?
Flags: needinfo?(sphink)
Soeren: thanks for the bug report and test case!

I can repro this crash in Firefox 24, 25.0.1 and 26, but not ESR 17, Beta 27, Aurora 28, or Nightly 29. I believe the crash is not listed in about:crashes because of Breakpad bug 717758.

Crashed Thread:  48  DOM Worker
Exception Type:  EXC_CRASH (SIGABRT)
Exception Codes: 0x0000000000000000, 0x0000000000000000
Application Specific Information: abort() called
*** error for object 0x1c54cbff0: pointer being freed was not allocated
 
Thread 0:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff80dc4a1a mach_msg_trap + 10
1   libsystem_kernel.dylib        	0x00007fff80dc3d18 mach_msg + 64
2   com.apple.CoreFoundation      	0x00007fff8478c315 __CFRunLoopServiceMachPort + 181
3   com.apple.CoreFoundation      	0x00007fff8478b939 __CFRunLoopRun + 1161
4   com.apple.CoreFoundation      	0x00007fff8478b275 CFRunLoopRunSpecific + 309
5   com.apple.HIToolbox           	0x00007fff83fadf0d RunCurrentEventLoopInMode + 226
6   com.apple.HIToolbox           	0x00007fff83fadcb7 ReceiveNextEventCommon + 479
7   com.apple.HIToolbox           	0x00007fff83fadabc _BlockUntilNextEventMatchingListInModeWithFilter + 65
8   com.apple.AppKit              	0x00007fff87c6828e _DPSNextEvent + 1434
9   com.apple.AppKit              	0x00007fff87c678db -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
10  XUL                           	0x00000001025bef82 0x101800000 + 14413698
11  com.apple.AppKit              	0x00007fff87c5b9cc -[NSApplication run] + 553
12  XUL                           	0x00000001025c00ab 0x101800000 + 14418091
13  XUL                           	0x0000000102438db4 0x101800000 + 12815796
14  XUL                           	0x000000010180cb08 0x101800000 + 51976
15  XUL                           	0x000000010180cda0 0x101800000 + 52640
16  XUL                           	0x000000010180d0c4 XRE_main + 244
17  org.mozilla.firefox           	0x000000010000169b 0x100000000 + 5787
18  org.mozilla.firefox           	0x0000000100000d54 start + 52
Status: UNCONFIRMED → NEW
Ever confirmed: true
I used mozregression to bisect this crash to this mozilla-inbound regression range in Firefox 22:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3825fdbcec62&tochange=0a10eca0c521

Unfortunately this regression range is not very helpful because it includes bug 851964 ("Re-enable OdinMonkey on OSX"), so the crash was probably dormant before OdinMonkey was re-enabled.
Like Soeren, I am also using OS X 10.9.0.
Group: mozilla-corporation-confidential
Group: mozilla-corporation-confidential → core-security
I asked dveditz to mark this bug as a potential security issue because it is a "pointer being freed was not allocated" crash that includes a test case and is 100% reproducible on Firefox 26 and ESR 24.
Keywords: reproducible
I bisected the fix to this mozilla-inbound regression range in Firefox 27:

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=06c3ed54ed62&tochange=b87bbbb0c612

This crash was probably fixed by bug 861925.
Blocks: 861925
sfink: is the fix in bug 861925 reasonable and sufficient or do we need to dig further?

Assuming that bug is the fix the dependency ought to go the other direction.
No longer blocks: 861925
Depends on: 861925
Whiteboard: appears fixed by bug 861925
If bug 861925 fixes this, we should get it in the ESR24 shipping alongside FF27 - I've nominated it for now just to keep an eye out until we hear back from Steve on FF26 status.
Sorry for the long delay.

It's complicated.

Bug 861925 probably fixed this by rewriting the relevant code. It is a fairly big patch to backport, and it had some followup fixes that we'd need too: bug 931008 and bug 939472.

That said, none of these patches directly fixed this bug. It reminds me the most of bug 868700, but that landed in FF24. This bug's introduction might date all the way back to when transferables were first introduced in bug 720083, though that landed in FF18. It's possible that bug 868700 introduced it while fixing leaks.

I think the valid options are (1) backport bug 861925 + 931008 + 939472 or (2) make a targeted fix for esr24. I'm willing to do either. The tests around this stuff are pretty good, but if I'm missing some other now-fixed bug that these patches caused, then the backport won't have the test for that hypothetical bug. I could try running the current test suite against the patched esr24 and see if any of the failures seem related?

I'm thinking a targeted fix would probably be safest.
Flags: needinfo?(sphink)
Group: javascript-core-security
Steve: how hard would it be to craft a targeted fix for this?
Geo: Can you please test this on a b2g1.2 device and see if it is affected? It ought to have this bug, but it's possible that there are enough differences due to it being on ARM that it's not affected.  Once Steve comes up with a targeted fix for ESR 24 we'll have a better idea whether b2g1.2  is safe or not in theory, but a crash would settle the issue immediately.
Flags: needinfo?(gmealer)
Sure, I'll let you know what I see. Is running it in-browser (i.e. as content) going to be sufficient for a test?
Flags: needinfo?(gmealer) → needinfo?(dveditz)
Should be. If b2g is affected I'd expect it to crash the child process.
Flags: needinfo?(dveditz)
The only part of bug 861925 needed to fix this is that a neutered asmjs ArrayBuffer should not have its flags cleared. I could take the relevant part of bug 861925 (split setElementsHeader into updateElementsHeader and initElementsHeader), but I think a trivial targeted fix is slightly safer here.

This patch is for esr24 only.
Attachment #8364189 - Flags: review?(luke)
Comment on attachment 8364189 [details] [diff] [review]
Backport of a small bugfix that was part of bug 861925

Thanks!
Attachment #8364189 - Flags: review?(luke) → review+
(In reply to Steve Fink [:sfink] from comment #18)
> This patch is for esr24 only.

And b2g26 (if affected)?
Since this looks fixed, it seems like there's no more work to do on trunk.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
If you could put this up for approval quickly, Steve, that would be good, as we're almost at the end of a cycle.
Target Milestone: --- → mozilla27
Attached file bug950604.log
Tried on FxOS 1.2.0.0-prerelease, Build ID: 20140123004001, Platform version 26.0.

Browser (and OS) remained viable after clicking through warning. Didn't see any external effects, aside from landing on a blank screen. Everything seemed to remain responsive.

Attached logcat of that time period. I don't see anything that looks like a crash near the asm.js compilation log messages.

Log reflects actually running attached testcase twice in a row. (ran, clicked through, hit back, ran again, clicked through, switched tabs, all behaved fine). 

Had previously confirmed bad behavior on desktop browser, so pretty sure I didn't see anything like it.
Forgot to mention hardware: this was tested with the hamachi build on a buri.
Marking for a Firefox 27 security advisory.
Whiteboard: appears fixed by bug 861925 → [adv-main27+] appears fixed by bug 861925
Alias: CVE-2014-1488
This missed the ESR24.3 release. This should go into ESR soon since we've fixed it elsewhere.
Comment on attachment 8364189 [details] [diff] [review]
Backport of a small bugfix that was part of bug 861925

[Approval Request Comment]
Fix Landed on Version: 27
Risk to taking this patch (and alternatives if risky): low, it's been running on other branches for a while now
String or UUID changes made by this patch: none

Sorry, I got confused by my bugmail and thought that this had been rejected for esr. Looks like that was another bug entirely. This is a safe fix for a dangerous bug, but I don't really understand where we are in the lifecycle right now.
Attachment #8364189 - Flags: approval-mozilla-esr24?
Attachment #8364189 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
Whiteboard: [adv-main27+] appears fixed by bug 861925 → [adv-main27+][adv-esr24.4+] appears fixed by bug 861925
Group: javascript-core-security
Confirmed crash in 24.3esr.
Verified fixed in 24.3esrpre built on 2014-03-13.
Verified fixed in 27.0.1 release.
Status: RESOLVED → VERIFIED
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: