Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 788959 - (CVE-2013-0762) Heap-use-after-free in imgRequest::OnStopFrame
: Heap-use-after-free in imgRequest::OnStopFrame
: crash, sec-critical, testcase
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: x86_64 All
: -- critical (vote)
: mozilla20
Assigned To: Josh Matthews [:jdm]
: Milan Sreckovic [:milan] (PTO through Oct 23)
: 812929 (view as bug list)
Depends on:
Blocks: 812929
  Show dependency treegraph
Reported: 2012-09-06 00:11 PDT by Abhishek Arya
Modified: 2014-07-24 13:41 PDT (History)
12 users (show)
dveditz: sec‑bounty+ in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase (746 bytes, text/html)
2012-09-06 07:30 PDT, Abhishek Arya
no flags Details
ASAN dump (18.29 KB, text/plain)
2012-10-29 11:03 PDT, Mats Palmgren (:mats)
no flags Details
Clear the listener associated with cancelled bullet frame image requests. (4.00 KB, patch)
2012-11-20 15:56 PST, Josh Matthews [:jdm]
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr10+
akeybl: approval‑mozilla‑esr17+
abillings: sec‑approval+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-09-06 00:11:20 PDT
Reproduces on trunk. Testcase with better stack coming.

==6483== ERROR: AddressSanitizer heap-use-after-free on address 0x7f12bb7cef80 at pc 0x7f12ebf2a6ad bp 0x7fff1ecacb70 sp 0x7fff1ecacb68
READ of size 8 at 0x7f12bb7cef80 thread T0
    #0 0x7f12ebf2a6ac in nsQueryInterface::operator()(nsID const&, void**) const objdir-ff-asan/xpcom/build/nsCOMPtr.cpp:14
    #1 0x7f12e91b069d in nsCOMPtr ../../dist/include/nsCOMPtr.h:554
    #2 0x7f12e919e955 in imgRequest::OnStopFrame(imgIRequest*, unsigned int) image/src/imgRequest.cpp:645
    #3 0x7f12e914d3cb in mozilla::image::Decoder::PostFrameStop() image/src/Decoder.cpp:246
0x7f12bb7cef80 is located 0 bytes inside of 24-byte region [0x7f12bb7cef80,0x7f12bb7cef98)
freed by thread T0 here:
    #0 0x42b630 in free 
    #1 0x7f12e948ca4d in nsBulletListener::Release() layout/generic/nsBulletFrame.cpp:1622
previously allocated by thread T0 here:
    #0 0x42b6f0 in __interceptor_malloc 
    #1 0x7f12ef58f3a8 in moz_xmalloc memory/mozalloc/mozalloc.cpp:57
    #2 0x7f12e94a86d4 in nsFrame::Init(nsIContent*, nsIFrame*, nsIFrame*) layout/generic/nsFrame.cpp:535
Shadow byte and word:
  0x1fe2576f9df0: fd
  0x1fe2576f9df0: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1fe2576f9dd0: fd fd fd fd fd fd fd fd
  0x1fe2576f9dd8: fd fd fd fd fd fd fd fd
  0x1fe2576f9de0: fa fa fa fa fa fa fa fa
  0x1fe2576f9de8: fa fa fa fa fa fa fa fa
=>0x1fe2576f9df0: fd fd fd fd fd fd fd fd
  0x1fe2576f9df8: fd fd fd fd fd fd fd fd
  0x1fe2576f9e00: fa fa fa fa fa fa fa fa
  0x1fe2576f9e08: fa fa fa fa fa fa fa fa
  0x1fe2576f9e10: 00 00 00 00 00 00 00 00
Stats: 352M malloced (405M for red zones) by 832581 calls
Stats: 60M realloced by 88678 calls
Stats: 304M freed by 665969 calls
Stats: 197M really freed by 173839 calls
Stats: 612M (156759 full pages) mmaped in 153 calls
  mmaps   by size class: 8:557022; 9:49146; 10:81900; 11:12282; 12:3072; 13:2048; 14:1280; 15:256; 16:448; 17:1280; 18:240; 19:40; 20:20; 21:2;
  mallocs by size class: 8:655210; 9:61844; 10:88875; 11:13927; 12:4293; 13:2966; 14:2222; 15:488; 16:815; 17:1614; 18:259; 19:47; 20:19; 21:2;
  frees   by size class: 8:513952; 9:46898; 10:84377; 11:10477; 12:3126; 13:2543; 14:1600; 15:435; 16:741; 17:1594; 18:165; 19:45; 20:16;
  rfrees  by size class: 8:114982; 9:29585; 10:13571; 11:8791; 12:1614; 13:1699; 14:1294; 15:249; 16:554; 17:1311; 18:137; 19:37; 20:15;
Stats: malloc large: 1941 small slow: 3643
==6483== ABORTING
Comment 1 Abhishek Arya 2012-09-06 07:30:55 PDT
Created attachment 658876 [details]

Reproduces reliably with a couple of firefox instances run simultaneously.
Comment 2 Mats Palmgren (:mats) 2012-10-29 11:01:51 PDT
Who's triaging sec bugs in Core/ImageLib?
Comment 3 Mats Palmgren (:mats) 2012-10-29 11:03:06 PDT
Created attachment 676232 [details]
ASAN dump

==7177== ERROR: AddressSanitizer heap-use-after-free on address 0x7fe0bbe21b80 at pc 0x7fe0d75b79c4 bp 0x7ffff64bafd0 sp 0x7ffff64bafc8
READ of size 8 at 0x7fe0bbe21b80 thread T0
    #0 0x7fe0d75b79c4 in nsQueryInterface::operator()(nsID const&, void**) const xpcom/build/nsCOMPtr.cpp:14
    #1 0x7fe0cb6ed163 in nsCOMPtr<imgIOnloadBlocker>::assign_from_qi(nsQueryInterface, nsID const&) ../../dist/include/nsCOMPtr.h:1144
    #2 0x7fe0cb6ece61 in nsCOMPtr ../../dist/include/nsCOMPtr.h:555
    #3 0x7fe0cb6e13a8 in nsCOMPtr ../../dist/include/nsCOMPtr.h:555
    #4 0x7fe0cb6e10bb in imgRequestProxy::BlockOnload() image/src/imgRequestProxy.cpp:741
    #5 0x7fe0cb7050d9 in imgStatusTracker::SendBlockOnload(imgRequestProxy*) image/src/imgStatusTracker.cpp:736
    #6 0x7fe0cb704a6d in imgStatusTrackerObserver::OnStartDecode() image/src/imgStatusTracker.cpp:64
    #7 0x7fe0cb548900 in mozilla::image::Decoder::Init() image/src/Decoder.cpp:47
    #8 0x7fe0cb55e352 in mozilla::image::RasterImage::InitDecoder(bool) image/src/RasterImage.cpp:2523
    #9 0x7fe0cb59f22e in mozilla::image::RasterImage::RequestDecodeCore(mozilla::image::RasterImage::RequestDecodeType) image/src/RasterImage.cpp:2732
    #10 0x7fe0cb59e6b8 in mozilla::image::RasterImage::StartDecoding() image/src/RasterImage.cpp:2672
    #11 0x7fe0cb5723e5 in mozilla::image::RasterImage::WantDecodedFrames() image/src/RasterImage.cpp:2657
    #12 0x7fe0cb571d0f in mozilla::image::RasterImage::GetImgFrame(unsigned int) image/src/RasterImage.cpp:794
    #13 0x7fe0cb56ecca in mozilla::image::RasterImage::GetDrawableImgFrame(unsigned int) image/src/RasterImage.cpp:802
    #14 0x7fe0cb572a37 in mozilla::image::RasterImage::GetCurrentDrawableImgFrame() image/src/RasterImage.cpp:850
    #15 0x7fe0cb5a8ce2 in mozilla::image::RasterImage::Draw(gfxContext*, gfxPattern::GraphicsFilter, gfxMatrix const&, gfxRect const&, nsIntRect const&, nsIntSize const&, unsigned int) image/src/RasterImage.cpp:3018
    #16 0x7fe0cbe7f0f5 in DrawImageInternal layout/base/nsLayoutUtils.cpp:3812
    #17 0x7fe0cbe805d3 in nsLayoutUtils::DrawSingleImage(nsRenderingContext*, imgIContainer*, gfxPattern::GraphicsFilter, nsRect const&, nsRect const&, unsigned int, nsRect const*) layout/base/nsLayoutUtils.cpp:3928
    #18 0x7fe0cc30b7ce in nsBulletFrame::PaintBullet(nsRenderingContext&, nsPoint, nsRect const&) layout/generic/nsBulletFrame.cpp:245
Comment 4 Robert Kaiser 2012-11-18 19:03:13 PST
What's the status here? This is marked tracking# for 18, which is entering beta this week.
Comment 5 Josh Matthews [:jdm] 2012-11-19 08:46:35 PST
I'm having trouble acquiring a working build of an ASAN-enabled clang, so I may not be the best owner here.
Comment 6 Abhishek Arya 2012-11-19 08:48:02 PST
(In reply to Josh Matthews [:jdm] from comment #5)
> I'm having trouble acquiring a working build of an ASAN-enabled clang, so I
> may not be the best owner here.

You can use latest one from here -
Comment 7 Josh Matthews [:jdm] 2012-11-20 13:35:40 PST
Crumbs, I reproduced it the first time, and now can't seem to trigger the error no matter how many times I refresh/restart.
Comment 8 Josh Matthews [:jdm] 2012-11-20 13:37:35 PST
My mistake, comment 7 was for bug 812929.
Comment 9 Josh Matthews [:jdm] 2012-11-20 15:56:49 PST
Created attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

With this patch I wasn't able to reproduce the intermittent crash in many minutes of frequent refreshing through multiple tabs.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2012-11-21 22:52:36 PST
Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

Comment 11 Josh Matthews [:jdm] 2012-11-22 05:49:44 PST
Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

[Security approval request comment]
How easily can the security issue be deduced from the patch?
Not easily at all, in my opinion.

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

Which older supported branches are affected by this flaw?
Presumably all of them.

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?
They should be exceedingly easily.

How likely is this patch to cause regressions; how much testing does it need?
My intuition says not likely; the functionality change involved in dealing with cancelled requests should not be noticeable. I don't believe this should require much testing.
Comment 12 Al Billings [:abillings] 2012-11-26 12:54:04 PST
Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.


Please prepare a patch for affected (supported) branches though.

Could this use an automated test?
Comment 15 Josh Matthews [:jdm] 2012-11-28 07:36:49 PST
The same patch applies to all affected branches. Do I need to do anything special, or can I just land it?
Comment 16 Joe Drew (not getting mail) 2012-11-28 07:38:13 PST
You need to ask for approval to land.
Comment 17 Josh Matthews [:jdm] 2012-11-28 07:58:02 PST
Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Lost in the depths of time.
User impact if declined: Possible exploit vector.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Slight, but unlikely, chance of unknown regression in behaviour when cancelling image requests for bullets. 
String or UUID changes made by this patch: None.
Comment 20 Ed Morley [:emorley] 2012-11-29 07:00:56 PST
Comment 21 Joe Drew (not getting mail) 2012-12-03 17:36:57 PST
*** Bug 812929 has been marked as a duplicate of this bug. ***
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-03 10:36:41 PST
Does this need an in-testsuite testcase?

Note You need to log in before you can comment on or make changes to this bug.