Closed Bug 942164 (CVE-2014-1486) Opened 6 years ago Closed 6 years ago

imgRequestProxy Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-2036)

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

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

People

(Reporter: curtisk, Assigned: jdm)

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [external-reporter][adv-main27+][adv-esr24.3+])

Attachments

(4 files, 2 obsolete files)

Date: Thu, 21 Nov 2013 14:21:29 -0600
From: ZDI Disclosures <zdi-disclosures@tippingpoint.com>
To: security@mozilla.org
Subject: ZDI-CAN-2036: New Vulnerability Report
-----//-----

ZDI-CAN-2036: Mozilla Firefox imgRequestProxy Use-After-Free Remote Code Execution Vulnerability


-- CVSS -----------------------------------------

6.8, AV:N/AC:M/Au:N/C:P/I:P/A:P


-- ABSTRACT -------------------------------------

HP's Zero Day Initiative has identified a vulnerability affecting the following products:

  Mozilla Firefox


-- VULNERABILITY DETAILS ------------------------

Flaw exists in image processing when source is set by server with content-type
"multipart/x-mixed-replace". When new image is getting loaded, new
imgRequestProxy object gets allocated by imgLoader::CreateNewProxyForRequest():

C:\mozilla-build\mozilla-central\image\src\imgLoader.cpp:644
------------------------------------------------------------
nsresult imgLoader::CreateNewProxyForRequest(imgRequest *aRequest, nsILoadGroup *aLoadGroup,
                                             imgINotificationObserver *aObserver,
                                             nsLoadFlags aLoadFlags, imgRequestProxy **_retval)
{
  LOG_SCOPE_WITH_PARAM(GetImgLog(), "imgLoader::CreateNewProxyForRequest", "imgRequest", aRequest);

  /* XXX If we move decoding onto separate threads, we should save off the
     calling thread here and pass it off to |proxyRequest| so that it call
     proxy calls to |aObserver|.
   */

  imgRequestProxy *proxyRequest = new imgRequestProxy();
  NS_ADDREF(proxyRequest);
...

Related call stack:
ChildEBP RetAddr
001def64 5a89a3ae xul!imgLoader::CreateNewProxyForRequest+0x98 [c:\mozilla-build\mozilla-central\image\src\imgloader.cpp @ 644]
001df188 5ab3dda8 xul!imgLoader::LoadImage+0xb6c [c:\mozilla-build\mozilla-central\image\src\imgloader.cpp @ 1771]
001df1f0 5ab6d966 xul!nsContentUtils::LoadImage+0x1f8 [c:\mozilla-build\mozilla-central\content\base\src\nscontentutils.cpp @ 2768]
001df22c 5afea673 xul!nsDocument::MaybePreLoadImage+0xce [c:\mozilla-build\mozilla-central\content\base\src\nsdocument.cpp @ 8518]
001df244 5afd5689 xul!nsHtml5TreeOpExecutor::PreloadImage+0x3a [c:\mozilla-build\mozilla-central\parser\html\nshtml5treeopexecutor.cpp @ 1076]
001df2b4 5afeb6e2 xul!nsHtml5SpeculativeLoad::Perform+0x10b [c:\mozilla-build\mozilla-central\parser\html\nshtml5speculativeload.cpp @ 32]
001df2ec 5afd6efc xul!nsHtml5TreeOpExecutor::RunFlushLoop+0xef [c:\mozilla-build\mozilla-central\parser\html\nshtml5treeopexecutor.cpp @ 508]
001df2f8 5b9b69a3 xul!nsHtml5ExecutorFlusher::Run+0x2d [c:\mozilla-build\mozilla-central\parser\html\nshtml5streamparser.cpp @ 133]
001df35c 5b96f8cc xul!nsThread::ProcessNextEvent+0x298 [c:\mozilla-build\mozilla-central\xpcom\threads\nsthread.cpp @ 622]
001df370 5b4eb8d5 xul!NS_ProcessNextEvent+0x3d [c:\mozilla-build\mozilla-central\xpcom\glue\nsthreadutils.cpp @ 238]
001df39c 5b9f3baa xul!mozilla::ipc::MessagePump::Run+0xc9 [c:\mozilla-build\mozilla-central\ipc\glue\messagepump.cpp @ 85]
001df3bc 5b9f40e7 xul!MessageLoop::RunInternal+0x3c [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 220]
001df3f0 5b9f45b2 xul!MessageLoop::RunHandler+0x4d [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 214]
001df410 5b44bdc1 xul!MessageLoop::Run+0x19 [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 188]
001df420 5b410231 xul!nsBaseAppShell::Run+0x47 [c:\mozilla-build\mozilla-central\widget\xpwidgets\nsbaseappshell.cpp @ 163]
001df430 5b255735 xul!nsAppShell::Run+0x11 [c:\mozilla-build\mozilla-central\widget\windows\nsappshell.cpp @ 113]

In frame Nr.4 one can see nsDocument::MaybePreLoadImage() call:

C:\mozilla-build\mozilla-central\content\base\src\nsDocument.cpp:8477
---------------------------------------------------------------------
void
nsDocument::MaybePreLoadImage(nsIURI* uri, const nsAString &amp;aCrossOriginAttr)
{
  // Early exit if the img is already present in the img-cache
  // which indicates that the "real" load has already started and
  // that we shouldn't preload it.
  int16_t blockingStatus;
  if (nsContentUtils::IsImageInCache(uri, static_cast<nsIDocument *>(this)) ||
      !nsContentUtils::CanLoadImage(uri, static_cast<nsIDocument *>(this),
                                    this, NodePrincipal(), &amp;blockingStatus)) {
    return;
  }

  nsLoadFlags loadFlags = nsIRequest::LOAD_NORMAL;
  switch (Element::StringToCORSMode(aCrossOriginAttr)) {
  case CORS_NONE:
    // Nothing to do
    break;
  case CORS_ANONYMOUS:
    loadFlags |= imgILoader::LOAD_CORS_ANONYMOUS;
    break;
  case CORS_USE_CREDENTIALS:
    loadFlags |= imgILoader::LOAD_CORS_USE_CREDENTIALS;
    break;
  default:
    MOZ_CRASH("Unknown CORS mode!");
  }

  // Image not in cache - trigger preload
  nsRefPtr<imgRequestProxy> request;
  nsresult rv =
    nsContentUtils::LoadImage(uri,
                              this,
                              NodePrincipal(),
                              mDocumentURI, // uri of document used as referrer
                              nullptr,       // no observer
                              loadFlags,
                              getter_AddRefs(request));

  // Pin image-reference to avoid evicting it from the img-cache before
  // the "real" load occurs. Unpinned in DispatchContentLoadedEvents and
  // unlink
  if (NS_SUCCEEDED(rv)) {
    mPreloadingImages.AppendObject(request);
  }
}
...

This will save reference to imgRequestProxy object in mPreloadingImages. After
image has loaded nsDocument::DispatchContentLoadedEvents() will be called:

C:\mozilla-build\mozilla-central\content\base\src\nsDocument.cpp:4611
---------------------------------------------------------------------
void
nsDocument::DispatchContentLoadedEvents()
{
  // If you add early returns from this method, make sure you're
  // calling UnblockOnload properly.

  // Unpin references to preloaded images
  mPreloadingImages.Clear();
...

This will call imgRequestProxy::Release() and imgRequestProxy object will be
freed:

C:\mozilla-build\mozilla-central\image\src\imgRequestProxy.cpp:94
-----------------------------------------------------------------
NS_IMPL_RELEASE(imgRequestProxy)
...

Related call stack:
ChildEBP RetAddr
001df25c 5a89e77e xul!imgRequestProxy::`scalar deleting destructor'+0x18
001df274 5a69795d xul!imgRequestProxy::Release+0x92 [c:\mozilla-build\mozilla-central\image\src\imgrequestproxy.cpp @ 94]
001df280 5a8929ab xul!nsCOMPtr<nsIRequest>::~nsCOMPtr<nsIRequest>+0x1e [c:\mozilla-build\mozilla-central\obj-i686-pc-mingw32\dist\include\nscomptr.h @ 514]
001df288 5a893774 xul!nsProgressNotificationProxy::~nsProgressNotificationProxy+0x1f [c:\mozilla-build\mozilla-central\image\src\imgloader.h @ 433]
001df29c 5b968f23 xul!nsProgressNotificationProxy::Release+0x95 [c:\mozilla-build\mozilla-central\image\src\imgloader.cpp @ 230]
001df2b0 5b9c6b00 xul!NS_ProxyRelease+0x43 [c:\mozilla-build\mozilla-central\xpcom\glue\nsproxyrelease.cpp @ 48]
001df2d8 5b9c6c49 xul!nsInterfaceRequestorAgg::~nsInterfaceRequestorAgg+0x35 [c:\mozilla-build\mozilla-central\xpcom\base\nsinterfacerequestoragg.cpp @ 55]
001df2ec 5b968eb2 xul!nsInterfaceRequestorAgg::Release+0x6b [c:\mozilla-build\mozilla-central\xpcom\base\nsinterfacerequestoragg.cpp @ 37]
001df2f8 5b9b69a3 xul!nsProxyReleaseEvent::Run+0xf [c:\mozilla-build\mozilla-central\xpcom\glue\nsproxyrelease.cpp @ 20]
001df35c 5b96f8cc xul!nsThread::ProcessNextEvent+0x298 [c:\mozilla-build\mozilla-central\xpcom\threads\nsthread.cpp @ 622]
001df370 5b4eb8d5 xul!NS_ProcessNextEvent+0x3d [c:\mozilla-build\mozilla-central\xpcom\glue\nsthreadutils.cpp @ 238]
001df39c 5b9f3baa xul!mozilla::ipc::MessagePump::Run+0xc9 [c:\mozilla-build\mozilla-central\ipc\glue\messagepump.cpp @ 85]
001df3bc 5b9f40e7 xul!MessageLoop::RunInternal+0x3c [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 220]
001df3f0 5b9f45b2 xul!MessageLoop::RunHandler+0x4d [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 214]
001df410 5b44bdc1 xul!MessageLoop::Run+0x19 [c:\mozilla-build\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 188]
001df420 5b410231 xul!nsBaseAppShell::Run+0x47 [c:\mozilla-build\mozilla-central\widget\xpwidgets\nsbaseappshell.cpp @ 163]

Though after imgRequestProxy is freed, another imgStatusTracker::SyncNotifyState()
call will use already freed imgRequestProxy object.

Triggering the bug is a bit hard, as it requires refreshing a lot of times.

-- CREDIT ---------------------------------------

This vulnerability was discovered by:

   Arthur Gerkis

-- 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 and acquired by Hewlett-Packard, 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. The ZDI does not re-sell the vulnerability details or any exploit code. Instead, upon notifying the affected product vendor, the ZDI provides its HP TippingPoint 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.

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/
gpgkeys: key 556D80C193A35B1C not found on keyserver


<ZDI-CAN-2036.zip.pgp>
Alias: ZDI-CAN-2036
Summary: imgRequestProxy Use-After-Free Remote Code Execution Vulnerability → imgRequestProxy Use-After-Free Remote Code Execution Vulnerability (ZDI-CAN-2036)
Attached file ZDI-CAN-2036.zip
The PoC (which I confirm crashes us quite nicely).
Password ZDI-CAN-2036
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [external-reporter]
Component: Security → ImageLib
An easy workaround would be to make mConsumers contain nsRefPtr<imgRequestProxy> instead of imgRequestProxy*. However, imgRequestProxy's destructor attempts to remove itself from the owner's consumers array, so we appear to have some ownership semantics mismatch happening.
What do I need to do to reproduce this? Do I need to run threaded.py and visit localhost?
(In reply to Josh Matthews [:jdm] from comment #3)
> What do I need to do to reproduce this? Do I need to run threaded.py and
> visit localhost?

Yes, just run python server and point browser to http://127.0.0.1/start, Firefox should crash then.
This seems to be occurring because in imgRequest::OnDataAvailable, when we resniff the mime type we create a new imgStatusTracker and adopt its consumers. All further operations on the imgRequest (such as adding and removing consumers) correctly see the new status tracker, but something is holding on to the old one and its consumers never change after this point.
Assignee: nobody → josh
Attached patch Refcount imgRequest consumers. (obsolete) — Splinter Review
The issue here is that the old Image that gets replaced in imgRequest retains the old status tracker, which in turn contains outdated consumers. I tried two other ways of attacking this: 1) adding a Reset method to imgStatusTracker and not replacing it with a new pointer, but this caused a number of assertion failures that I was not confident of solving correctly, 2) updating the old status tracker pointer in all the places I could find, but this also caused an assertion failure that suggested that I was breaking some invariants I didn't understand. Making mConsumers store refcounted pointers seems like the easiest, most correct way to fix this problem.
Attachment #8341940 - Flags: review?(seth)
Potentially to Firefox 19, since bug 505385 added the adoption that causes problems here.
Comment on attachment 8341940 [details] [diff] [review]
Refcount imgRequest consumers.

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

I'm always a bit fearful of changing ownership strength like this (since code may implicitly depend on it to not leak), but I went through the relevant code and I can't see a problem.

This makes me very happy, since I'd prefer to use smart pointers where we can. Do you happen to remember why the code was written to use raw pointers originally?

::: image/src/imgStatusTracker.h
@@ +305,5 @@
>    void FireFailureNotification();
>  
>    // Main thread only, since imgRequestProxy calls are expected on the main
>    // thread, and mConsumers is not threadsafe.
> +  static void SyncNotifyState(nsTObserverArray<nsRefPtr<imgRequestProxy> >& proxies,

We don't need to write "> >" anymore, do we? (Not that you need to change the patch, just thought that all the compilers we support can cope with ">>" now.)
Attachment #8341940 - Flags: review?(seth) → review+
It was like that before I got involved with imagelib in bug 505385, so I can't really comment.
What happens now? Who lands the patch, on which branches, and when?
Flags: needinfo?(abillings)
(In reply to Josh Matthews [:jdm] from comment #11)
> What happens now? Who lands the patch, on which branches, and when?

Set the sec-approval? flag on the patch, fill out the text, and Al will let you know.
Flags: needinfo?(abillings)
In terms of who lands it, the developer usually lands the patch on mozilla-central. It is also their responsibility to land it on branches, but RyanVM usually graciously lands patches to branches when the checkin-needed keyword is set. In terms of which branches, presumably everything except release. Al will tell you when.  We're very early in the cycle, so probably not for a week or two.
You'll have to get aurora and beta and whatever else approval for the patch, after it lands on trunk, for backporting, though sometimes Al will set the + for those directly.
Comment on attachment 8341940 [details] [diff] [review]
Refcount imgRequest consumers.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? Not easily. The underlying lifetime problem is clear, but how to take advantage of it is quite obscure (and not visible in this patch).

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

Which older supported branches are affected by this flaw? All.

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

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

How likely is this patch to cause regressions; how much testing does it need? Unlikely to cause regressions.
Attachment #8341940 - Flags: sec-approval?
Comment on attachment 8341940 [details] [diff] [review]
Refcount imgRequest consumers.

sec-approval+ for trunk. We'll want Aurora, Beta, and ESR24 patches once this lands there as well.

What can we do about getting this onto B2G 1.2 as well?
For some reason I thought I had run this through try. My mistake.
Attachment #8341940 - Flags: sec-approval? → sec-approval+
Attached patch Interdiff (obsolete) — Splinter Review
This fixes the leaks for me.
Attachment #8350702 - Flags: review?(seth)
Try push is green for the tests that count.
NI on :seth to help with review here. so this can get landed on branches at the earliest and get the desired baketime/testing before release.
Flags: needinfo?(seth)
Comment on attachment 8350702 [details] [diff] [review]
Interdiff

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

Looks good.

Sorry for the long delay on this review; I've been on PTO until today.
Attachment #8350702 - Flags: review?(seth) → review+
Flags: needinfo?(seth)
Josh, if this can still land in Fx27 provided it is safe and low risk, this would have to land soon and also get uplifted by tomorrow in preparation for our beta on Thursday.
Flags: needinfo?(josh)
The original inbound patch applies to aurora without any conflicts.
Attachment #8363885 - Attachment description: Store weak references to imgRequest consumers. → ESR24 - Store weak references to imgRequest consumers.
Attachment #8363922 - Attachment description: Store weak references to imgRequest consumers. → Beta - Store weak references to imgRequest consumers.
Attachment #8341940 - Attachment is obsolete: true
Attachment #8350702 - Attachment is obsolete: true
Attachment #8363923 - Attachment description: Store weak references to imgRequest consumers. → Inbound/Aurora - Store weak references to imgRequest consumers.
NI on :jdm to help request approval on the aurora/beta patch with risk analysis so this can get in to a beta for Fx27. Our final beta is going to go-to-build tomorrow, so lets get this landed there .
Flags: needinfo?(josh)
Comment on attachment 8363923 [details] [diff] [review]
Inbound/Aurora - Store weak references to imgRequest consumers.

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
User impact if declined: Potential for UAF.
Testing completed (on m-c, etc.): Try runs; hasn't yet merged to m-c for some reason.
Risk to taking this patch (and alternatives if risky): Low. This is basically just adding safe null checks.
String or IDL/UUID changes made by this patch: None
Attachment #8363923 - Flags: approval-mozilla-beta?
Attachment #8363923 - Flags: approval-mozilla-aurora?
Flags: needinfo?(josh)
fixed on central https://hg.mozilla.org/mozilla-central/rev/4fecee90e6a6
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Attachment #8363923 - Flags: approval-mozilla-beta?
Attachment #8363923 - Flags: approval-mozilla-beta+
Attachment #8363923 - Flags: approval-mozilla-aurora?
Attachment #8363923 - Flags: approval-mozilla-aurora+
Attachment #8364558 - Attachment description: Store weak references to imgRequest consumers. → b2g26 - Store weak references to imgRequest consumers.
Whiteboard: [external-reporter] → [external-reporter][adv-main27+][adv-esr24.3+]
Alias: CVE-2014-1486
I`m trying to reproduce the crash in the nightly from 2013-11-22 in order to verify the fix but I am stuck.
I started a server with python using "python -m SimpleHTTPServer" then launched in Firefox "http://127.0.0.1:port/folder" - folder is the file where I extracted the content from ZDI-CAN-2036.zip.
If I hit threaded.py FF opens the content of the file and if I hit poc.html I get a blank page with a missing image but no crash. 
Is there something I`m missing or doing wrong?
Flags: needinfo?(ax330d)
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #39)
> I`m trying to reproduce the crash in the nightly from 2013-11-22 in order to
> verify the fix but I am stuck.
> I started a server with python using "python -m SimpleHTTPServer" then
> launched in Firefox "http://127.0.0.1:port/folder" - folder is the file
> where I extracted the content from ZDI-CAN-2036.zip.
> If I hit threaded.py FF opens the content of the file and if I hit poc.html
> I get a blank page with a missing image but no crash. 
> Is there something I`m missing or doing wrong?

You should start only threaded.py - when requesting "http://127.0.0.1/start" from browser, it will serve "poc.html" page and this page will make another request from image, source named "multi" (last request will be served from the same threaded.py server too).
Flags: needinfo?(ax330d)
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20140126 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20140126 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20140126 Firefox/24.0
24.3.0ESR - buildID: 20140128141233

Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
27.0RC - buildID: 20140127194636

Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Aurora 28.0a2 - buildID: 20140129004017

Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:29.0) Gecko/20100101 Firefox/29.0
Nightly 29.0a1 - buildID: 20140129030602

Thanks Arthur! 
I was able to reproduce the crash in a nightly older then 2014-01-23. 
Verified as fixed on the builds from above using Ubuntu 13.04x32, Mac OS X 10.9.1 and Windows 7x64.
Status: RESOLVED → VERIFIED
Group: core-security
You need to log in before you can comment on or make changes to this bug.