788959 - (CVE-2013-0762) Heap-use-after-free in imgRequest::OnStopFrame

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Abhishek Arya]

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]

Attachment: Testcase

Reproduces reliably with a couple of firefox instances run simultaneously.

Comment 2

[Mats Palmgren (inactive)]

Who's triaging sec bugs in Core/ImageLib?

Comment 3

[Mats Palmgren (inactive)]

Attachment: 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]

What's the status here? This is marked tracking# for 18, which is entering beta this week.

Comment 5

[Josh Matthews [:jdm]]

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]

(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 - http://commondatastorage.googleapis.com/chromium-browser-clang/index.html?path=Linux_x64/

Comment 7

[Josh Matthews [:jdm]]

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]]

My mistake, comment 7 was for bug 812929.

Comment 9

[Josh Matthews [:jdm]]

Attachment: 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 [:bzbarsky]]

Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

r=me

Comment 11

[Josh Matthews [:jdm]]

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?
No.

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 - ex-MoCo]]

Comment on attachment 683774 [details] [diff] [review]
Clear the listener associated with cancelled bullet frame image requests.

sec-approval+. 

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

Could this use an automated test?

Comment 14

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/667a3f65df08

Comment 15

[Josh Matthews [:jdm]]

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)]

You need to ask for approval to land.

Comment 17

[Josh Matthews [:jdm]]

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 18

[Josh Matthews [:jdm]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/caaefcf4754b
https://hg.mozilla.org/releases/mozilla-beta/rev/f76346c5aa0b
https://hg.mozilla.org/releases/mozilla-esr17/rev/e2f06b1f7dcf

Comment 19

[Josh Matthews [:jdm]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/e2a6bfad78e6

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/667a3f65df08

Comment 22

[u279076]

Does this need an in-testsuite testcase?