Closed Bug 814026 (CVE-2013-0754) Opened 12 years ago Closed 12 years ago

ListenerManager Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1591)

Categories

(Core :: DOM: Workers, defect)

Product:
Core
Component:
DOM: Workers
Version:
12 Branch
Platform:
All
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox18 + fixed
firefox19 + fixed
firefox20 + fixed
firefox-esr10 --- verified
firefox-esr17 18+ verified
b2g18 --- fixed

People

(Reporter: curtisk, Assigned: bent.mozilla)

References

Details

(Keywords: csectype-uaf, sec-critical, testcase, Whiteboard: [adv-main18+][adv-esr17+][adv-esr10+])

Attachments

(1 file, 1 obsolete file)

Patch, v2
2.93 KB, patch
mrbkap
: review+
bajaj
: approval-mozilla-aurora+
bajaj
: approval-mozilla-beta+
lsblakk
: approval-mozilla-release-
lsblakk
: approval-mozilla-esr10+
bajaj
: approval-mozilla-esr17+
Details | Diff | Splinter Review
Attached file PoC — 
-- CVSS -----------------------------------------
7.5, AV:N/AC:L/Au:N/C:P/I:P/A:P

-- ABSTRACT -------------------------------------
TippingPoint has identified a vulnerability affecting the following products:

  Mozilla Firefox

-- VULNERABILITY DETAILS ------------------------
Version(s) Tested: Mozilla Firefox 12.0
Platform(s) Tested: Windows XP SP3

--------------------- 
Vulnerability details 
---------------------

The vulnerable function is ListenerManager::DispatchEvent within dom/workers/ListenerManager.cpp
From the researcher:
Registered event listeners are stored in temporary |js::Vector| before being
called one by one. When the number of |jsval|s held in |listeners| exceeds 10,
storage space for them is allocated on the heap. Thus, they are unreachable
from garbage collector roots. During user-provided event handling callback, one
is free to remove/unregister all the listeners and force garbage collection. |jsval|s kept in |listeners| will become dangling pointers.

The code below triggers the vulnerability: 

--------------- 
Begin Code Snip 
---------------
<!DOCTYPE html>
<!--
     Mozilla Firefox 12.0
     ListenerManager dangling pointer
     Windows XP SP3 x86 code exec
     regenrecht@o2.pl May 2012
-->
<html>
<head>

</head>
<body onload="run();">
</body>
</html>


--------------- 
End Code Snip 
---------------
This code will create a listener that will erase all subsequent listeners and replace them with memory that'll result in EIP being c1c2c3c4

-- CREDIT ---------------------------------------
This vulnerability was discovered by:

   regenrecht

-- FURTHER DETAILS ------------------------------
If supporting files were contained with this report they are provided within a password protected ZIP file. The password is the ZDI candidate number in the form: ZDI-CAN-XXXX where XXXX is the ID number.

Please confirm receipt of this report. We expect all vendors to remediate ZDI vulnerabilities within 180 days of the reported date. If you are ready to release a patch at any point leading up the the deadline please coordinate with us so that we may release our advisory detailing the issue. If the 180 day deadline is reached and no patch has been made available we will release a limited public advisory with our own mitigations so that the public can protect themselves in the absence of a patch. Please keep us updated regarding the status of this issue and feel free to contact us at any time:


Zero Day Initiative
zdi-disclosures@tippingpoint.com

The PGP key used for all ZDI vendor communications is available from:

     http://www.zerodayinitiative.com/documents/zdi-pgp-key.asc

-- INFORMATION ABOUT THE ZDI ---------------------
Established by TippingPoint, The Zero Day Initiative (ZDI) represents a best-of-breed model for rewarding security researchers for responsibly disclosing discovered vulnerabilities.

The ZDI is unique in how the acquired vulnerability information is used. TippingPoint does not re-sell the vulnerability details or any exploit code. Instead, upon notifying the affected product vendor, TippingPoint provides its customers with zero day protection through its intrusion prevention technology. Explicit details regarding the specifics of the vulnerability are not exposed to any parties until an official vendor patch is publicly available. Furthermore, with the altruistic aim of helping to secure a broader user base, TippingPoint provides this vulnerability information confidentially to security vendors (including competitors) who have a vulnerability protection or mitigation product.

Please contact us for further information or refer to:

    http://www.zerodayinitiative.com

-- DISCLOSURE POLICY ----------------------------
Our vulnerability disclosure policy is available online at:

    http://www.zerodayinitiative.com/advisories/disclosure_policy/
There is a reason why using the normal event handling also with worker objects would be really nice :/
Attached patch Patch, v1 (obsolete) — — Splinter Review 
This should work. Branches have different filenames but the same basic code.
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Attachment #684057 - Flags: review?(bzbarsky)
Comment on attachment 684057 [details] [diff] [review]
Patch, v1

r=me
Attachment #684057 - Flags: review?(bzbarsky) → review+
Keywords: testcase
Alias: ZDI-CAN-1591
Summary: ListenerManager Use-After-Free Remote Code Execution Vulnerability → ListenerManager Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1591)
https://hg.mozilla.org/integration/mozilla-inbound/rev/c19edad3d359
Alias: ZDI-CAN-1591
Summary: ListenerManager Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1591) → ListenerManager Use-After-Free Remote Code Execution Vulnerability
Oops.
Alias: ZDI-CAN-1591
Summary: ListenerManager Use-After-Free Remote Code Execution Vulnerability → ListenerManager Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-1591)
Backed out for crashes on Android:
eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=17246531&tree=Mozilla-Inbound

and on a later push (when bz's bustage was backed out, so easier to see what's left):
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=461a2225b7ba
https://tbpl.mozilla.org/php/getParsedLog.php?id=17247031&tree=Mozilla-Inbound

{
PROCESS-CRASH | /tests/Harness_sanity/test_SpecialPowersExtension.html | application crashed (minidump found)
Crash dump filename: /tmp/tmpcoqkYP/138e614b-fa04-f34e-3924bba1-621d3812.dmp
Operating system: Android
                  0.0.0 Linux 2.6.32.9-00002-gd8084dc-dirty #1 SMP PREEMPT Wed Feb 2 11:32:06 PST 2011 armv7l nvidia/harmony/harmony/harmony:2.2/FRF91/20110202.102810:eng/test-keys
CPU: arm
     0 CPUs

Crash reason:  SIGSEGV
Crash address: 0xfffff000

Thread 4 (crashed)
 0  libxul.so!js::gc::MarkString(JSTracer*, js::EncapsulatedPtr<JSAtom, unsigned int>*, char const*) [Heap.h : 989 + 0x0]
     r4 = 0x50f25110    r5 = 0xfffff000    r6 = 0x00000001    r7 = 0x5571918c
     r8 = 0x56e46cf8    r9 = 0x00000004   r10 = 0x55899740    fp = 0x56e46d30
     sp = 0x4e7a7490    lr = 0x5534fe0b    pc = 0x553f8bd0
    Found by: given as instruction pointer in context
 1  libxul.so!JSScript::markChildren(JSTracer*) [jsscript.cpp : 2561 + 0x7]
     r4 = 0x57202e98    r5 = 0x50f25110    r6 = 0x00000001    r7 = 0x5571918c
     r8 = 0x56e46cf8    r9 = 0x00000004   r10 = 0x55899740    fp = 0x56e46d30
     sp = 0x4e7a74a0    pc = 0x5534fe0b
    Found by: call frame info
 2  libxul.so!PushMarkStack [Marking.cpp : 838 + 0x7]
     r4 = 0x00080000    r5 = 0x57200000    r6 = 0x00000001    r7 = 0x000005d3
     r8 = 0x56e46cf8    r9 = 0x00000004   r10 = 0x0000002c    fp = 0x56e46d30
     sp = 0x4e7a74e8    pc = 0x553f6b11
    Found by: call frame info
 3  libxul.so!js::gc::MarkScriptUnbarriered [Marking.cpp : 137 + 0x7]
     r4 = 0x50f25110    r5 = 0x57202000    r6 = 0x4e7a7668    r7 = 0x56cf9100
     r8 = 0x56e46cf8    r9 = 0x00000004   r10 = 0x0000002c    fp = 0x56e46d30
     sp = 0x4e7a7500    pc = 0x553f759f
    Found by: call frame info
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/668f0085f1ef
Ok, I've poked around this a bunch and can't figure out how adding additional roots could trigger crashes in GC.

CC'ing a few JS folks, any pointers or help you can provide would be wonderful!
Blake and I sat down to look at this for a few hours and figured out that the problem really is somehow specific to JS::AutoValueVector:

1. If we replace the autorooter with manual JS_AddRoot/JS_RemoveRoot calls then we don't crash (so it's not somehow the act of marking these listener objects that is the problem).

2. Blake was also concerned that we were somehow being called with a JSContext from a different thread (and hence racing) but we've satisfied ourselves that this is not happening.

3. The autorooter seems to be working correctly, cx->runtime->autoGCRooters is being set/unset correctly, so the problem must happen during the actual tracing.

4. The heap is definitely being corrupted and the crashes are all over the place.

Blake, did I leave anything out?
How difficult is it to reproduce this? What sort of debugging environment do you have? It sounds like it only happens on ARM?
We were testing on B2G, actually. But it looks like all you have to do is apply this patch and build/run on ARM. Fennec and B2G both use workers so they both crash almost instantly.
That summary sounds right to me.
For the record: I built a b2g debug build (both with optimization and without) and this stopped crashing. As far as I know, the only crashing configuration is --disable-debug --enable-optimize. Fun times.
I'd like to help out with this, but I don't have access to an ARM system. Blake, if you're in MV some time, could we take a look at this?
(In reply to Bill McCloskey (:billm) from comment #13)
> I'd like to help out with this, but I don't have access to an ARM system.
> Blake, if you're in MV some time, could we take a look at this?

Hey Bill, I'll be in MV on Friday, I'll find you when I get in.
Hey Blake, Bill, did you guys make any progress on Friday? Release management folks want to know what we're planning on doing here and I don't know what to tell them :)
FYI this still crashes android builds on tinderbox (https://tbpl.mozilla.org/?tree=Try&rev=377d8b6326a9).
Attached patch Patch, v2 — — Splinter Review 
This is the slow awful patch that should avoid the security problem. We'll need to replace someday with the v1 patch once we figure out why it crashes android.
Attachment #693236 - Flags: review?(mrbkap)
Comment on attachment 693236 [details] [diff] [review]
Patch, v2

Please file a followup on removing this hack. Also, when we check this into the branches, bug 820185 will have to land there at the same time.
Attachment #693236 - Flags: review?(mrbkap) → review+
Attachment #684057 - Attachment is obsolete: true
Comment on attachment 693236 [details] [diff] [review]
Patch, v2

[Approval Request Comment]
User impact if declined: Reported vulnerability
Fix Landed on Version: mozilla-central
Risk to taking this patch (and alternatives if risky): Theoretically some sort of GC crash could result but this patch is designed to avoid GC crashes so it shouldn't be likely.
String or UUID changes made by this patch: None
Attachment #693236 - Flags: approval-mozilla-release?
Attachment #693236 - Flags: approval-mozilla-esr17?
Attachment #693236 - Flags: approval-mozilla-esr10?
Attachment #693236 - Flags: approval-mozilla-beta?
Attachment #693236 - Flags: approval-mozilla-b2g18?
Attachment #693236 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to ben turner [:bent] from comment #20)
> Comment on attachment 693236 [details] [diff] [review]
> Patch, v2
> 
> [Approval Request Comment]
> User impact if declined: Reported vulnerability
> Fix Landed on Version: mozilla-central
> Risk to taking this patch (and alternatives if risky): Theoretically some
> sort of GC crash could result but this patch is designed to avoid GC crashes
> so it shouldn't be likely.
> String or UUID changes made by this patch: None

I will approve it on needed branches for now, but please use the sec-approval flag in future before landing on sec-crit,sec-high patches on m-c .Thanks !
Attachment #693236 - Flags: approval-mozilla-esr17?
Attachment #693236 - Flags: approval-mozilla-esr17+
Attachment #693236 - Flags: approval-mozilla-beta?
Attachment #693236 - Flags: approval-mozilla-beta+
Attachment #693236 - Flags: approval-mozilla-aurora?
Attachment #693236 - Flags: approval-mozilla-aurora+
Comment on attachment 693236 [details] [diff] [review]
Patch, v2

approving for esr10, please go ahead and land there.  minusing for mozilla-release since we are not chemspilling for this bug and since we are a couple weeks from releasing 18 it is unlikely we will need this on release.
Attachment #693236 - Flags: approval-mozilla-release?
Attachment #693236 - Flags: approval-mozilla-release-
Attachment #693236 - Flags: approval-mozilla-esr10?
Attachment #693236 - Flags: approval-mozilla-esr10+
according to comment 18 bug 820185  needs to land on branches too so please nominate for tracking on the branches that do need it.
(In reply to bhavana bajaj [:bajaj] from comment #21)
> please use the sec-approval flag in future

Ugh, I botched that. Sorry!
Comment on attachment 693236 [details] [diff] [review]
Patch, v2

Because this is landing to mozilla-beta, it'll also land to mozilla-b2g18.
Attachment #693236 - Flags: approval-mozilla-b2g18?
My backport to esr10 didn't work so I backed it out: https://hg.mozilla.org/releases/mozilla-esr10/rev/162298a38b45

Ben, can you do a proper backport? I have to run to a family event.
status-firefox-esr10: fixed → affected
Whiteboard: [adv-main18+][adv-esr17+][adv-esr10+]
Is it possible to get the testcase for this bug in testsuite?
Flags: in-testsuite?
Alias: CVE-2013-0754
Confirmed crash on 10.0.11 and 17.0.1 ESR, confirmed fix on 10.0.12 and 17.0.2 ESR.
status-firefox-esr10: fixed → verified
status-firefox-esr17: fixed → verified
(In reply to Blake Kaplan (:mrbkap) from comment #18)
> Please file a followup on removing this hack.

Bug 835826.
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: