1166082 - (CVE-2015-2734) CairoTextureClientD3D9::BorrowDrawTarget using uninitialized memory

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

Security received the following email of a bug submission.

Subject: Security exploit submission
Date: Mon, 18 May 2015 15:05:20 -0700
From: Ron <[e-mail]>
To: [e-mail]

Greetings.

I have found a bug that causes Firefox to attempt to write to an address 
that is obtained from an area of the stack below the current stack 
pointer (i.e. uninitialized memory). This area of the stack usually 
contains the address of executable code (and thus writing to this 
address usually causes a crash, since code is write-protected), but the 
stack could, depending upon the path taken into the defective code, 
contain other addresses that could make this bug exploitable.

The bug is in CairoTextureClientD3D9::BorrowDrawTarget (line 695 of the 
38.0.1 version of gfx/layers/d3d9/TextureD3D9.cpp). This function calls 
mD3D9Surface -> LockRect (&rect, nullptr, 0) (which is the Windows 
DirectX9 function LockRect). If this call fails (as it does sometimes 
when many tabs are open under Windows XP SP3 32 bit), LockRect leaves 
rect uninitialized. However, BorrowDrawTarget does not check the return 
status, instead using the uninitialized rect to create a new 
gfxImageSurface.

Other code later uses the new gfxImageSurface. Typically this occurs via 
a call stack similar to the following (from Firefox 36.0.1), and a crash 
occurs while attempting to write to code at the address 0x2129cbd, but 
other code paths are possible, some of which probably permit an attacker 
to write arbitrary data into Firefox's address space.

*1      xul.dll!sse2_fill(pixman_implementation_t * imp=0x0955b000, 
unsigned int * bits=0x02129cbd, int stride=0x06561e66, int 
bpp=0x00000020, int x=0x00000000, int y=0x00000000, int 
width=0x00000010, int height=0x00000293, unsigned int filler=0x00000000)
 2      xul.dll!_composite_boxes(_cairo_image_surface * dst=0x00000040, 
_cairo_operator op=0x02129cbd, const _cairo_pattern * 
pattern=0x03156508, _cairo_boxes_t * boxes=0x0012e0f8, _cairo_antialias 
antialias=CAIRO_ANTIALIAS_NONE, _cairo_clip * clip=0x0012e11c, const 
_cairo_composite_rectangles * extents=0x0012e0b0)
 3      xul.dll!_clip_and_composite_boxes(_cairo_image_surface * 
dst=0x00000040, _cairo_operator op=0x02129cbd, const _cairo_pattern * 
src=0x03156508, _cairo_boxes_t * boxes=0x0012e0f8, _cairo_antialias 
antialias=CAIRO_ANTIALIAS_NONE, _cairo_composite_rectangles * 
extents=0x0012e0b0, _cairo_clip * clip=0x00000000)
  4      xul.dll!_cairo_image_surface_fill(void * 
abstract_surface=0x2b6d22f0, _cairo_operator op=CAIRO_OPERATOR_CLEAR, 
const _cairo_pattern * source=0x03156508, _cairo_path_fixed * 
path=0x17d662d4, _cairo_fill_rule fill_rule=CAIRO_FILL_RULE_WINDING, 
double tolerance=0.10000000000000001, _cairo_antialias 
antialias=CAIRO_ANTIALIAS_NONE, _cairo_clip * clip=0x0012e784)
 5      xul.dll!_cairo_surface_fill(_cairo_surface * 
surface=0x00000040, _cairo_operator op=0x02129cbd, const _cairo_pattern 
* source=0x03156508, _cairo_path_fixed * path=0x17d662d4, 
_cairo_fill_rule fill_rule=CAIRO_FILL_RULE_WINDING, double 
tolerance=0.10000000000000001, _cairo_antialias 
antialias=CAIRO_ANTIALIAS_NONE, _cairo_clip * clip=0x0012e784)
 6      xul.dll!_moz_cairo_fill_preserve(_cairo * cr=0x00000040)
 7      xul.dll!mozilla::gfx::DrawTargetCairo::ClearRect(const 
mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> & aRect={...})
 8 
xul.dll!mozilla::layers::RotatedContentBuffer::BorrowDrawTargetForPainting(mozilla::layers::RotatedContentBuffer::PaintState 
& aPaintState={...},
 9      xul.dll!mozilla::layers::ClientPaintedLayer::PaintThebes()
 10 
xul.dll!mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor 
* aReadback=0x0012eb14)
 11     xul.dll!mozilla::layers::ClientContainerLayer::RenderLayer()
 12 
xul.dll!mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor 
* aReadback=0x0012eb74)
 13     xul.dll!mozilla::layers::ClientContainerLayer::RenderLayer()
 14 
xul.dll!mozilla::layers::ClientLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor 
* aReadback=0x0012ebd4)
 15     xul.dll!mozilla::layers::ClientContainerLayer::RenderLayer()
 16 
xul.dll!mozilla::layers::ClientLayerManager::EndTransactionInternal(void 
(mozilla::layers::PaintedLayer *, gfxContext *, const nsIntRegion &, 
mozilla::layers::DrawRegionClip, const nsIntRegion &, void *)* 
aCallback=0x016f9a43, void * aCallbackData=0x0012f0a0, 
mozilla::layers::LayerManager::EndTransactionFlags __formal=0x0012ecb4)
 17 xul.dll!mozilla::layers::ClientLayerManager::EndTransaction(void 
(mozilla::layers::PaintedLayer *, gfxContext *, const nsIntRegion &, 
mozilla::layers::DrawRegionClip, const nsIntRegion &, void *)* 
aCallback=0x016f9a43, void * aCallbackData=0x0012f0a0, 
mozilla::layers::LayerManager::EndTransactionFlags aFlags=END_DEFAULT)
 18     xul.dll!nsDisplayList::PaintRoot(nsDisplayListBuilder * 
aBuilder=0x0012f0a0, nsRenderingContext * aCtx=0x00000000, unsigned int 
aFlags=0x0000000d)
 19     xul.dll!nsLayoutUtils::PaintFrame(nsRenderingContext * 
aRenderingContext=0x00000040, nsIFrame * aFrame=0x02129cbd, const 
nsRegion & aDirtyRegion={...}, unsigned int aBackstop=0xffffffff, 
unsigned int aFlags=0x00000304)
 20     xul.dll!PresShell::Paint(nsView * aViewToPaint=0x0e215830, 
const nsRegion & aDirtyRegion={...}, unsigned int aFlags=0x00000001)
 21     xul.dll!nsViewManager::ProcessPendingUpdatesPaint(nsIWidget * 
aWidget=0x0c136800)
 22     xul.dll!nsViewManager::ProcessPendingUpdatesForView(nsView * 
aView=0x0e215830, bool aFlushDirtyRegion=true)
 23     xul.dll!nsViewManager::ProcessPendingUpdates()
 24     xul.dll!nsRefreshDriver::Tick(__int64 
aNowEpoch=0x****************, mozilla::TimeStamp aNowTime={...})
 25     xul.dll!mozilla::RefreshDriverTimer::Tick()
 26     xul.dll!nsTimerImpl::Fire()
 27     xul.dll!nsTimerEvent::Run()
 28     xul.dll!nsThread::ProcessNextEvent(bool aMayWait=false, bool * 
aResult=0x0012fa44)
 29 xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate 
* aDelegate=0x03e66101)
 30     xul.dll!MessageLoop::RunHandler()
 31     xul.dll!MessageLoop::Run()
 32     xul.dll!nsBaseAppShell::Run()
 33     xul.dll!nsAppShell::Run()
 34     xul.dll!nsAppStartup::Run()
 35     xul.dll!XREMain::XRE_mainRun()
 36     xul.dll!XREMain::XRE_main(int argc=0x00000003, char * * 
argv=0x00154890, const nsXREAppData * aAppData=0x0012fd80)
 37     xul.dll!XRE_main(int argc=0x00000003, char * * argv=0x00154890, 
const nsXREAppData * aAppData=0x0012fd80, unsigned int aFlags=0x00000000)
 38     firefox.exe!do_main(int argc=0x00000040, char * * 
argv=0x02129cbd, nsIFile * xreDirectory=0x03e401c0)
 39     firefox.exe!NS_internal_main(int argc=0x00000040, char * * 
argv=0x02129cbd)
 40     firefox.exe!wmain(int argc=0x00154890, wchar_t * * 
argv=0x00154a08)
 41     firefox.exe!__tmainCRTStartup()
...

I have reproduced this crash in Firefox 36.0.1 under Windows XP SP3 
32-bit. Reproduction typically requires creating ~200 tabs initialized 
with assorted URLs (not all of which need to be loaded), then just 
switching between windows/tabs until Firefox crashes. The crash is more 
likely after sleeping or hibernating the computer for ~30 min. The crash 
also occurs more frequently under Firefox 36.0.

I have not yet reproduced this crash under the latest Firefox (38.0.1), 
but I have verified that the buggy code in 
CairoTextureClientD3D9::BorrowDrawTarget exists and is executed in that 
version, and that manually modifying the returned rect causes a crash 
just like the one in 36.0.1. Perhaps some other change has been made 
that makes it less likely that the LockRect call in BorrowDrawTarget 
will fail?

Crashes resulting from this bug might have been reported at 
https://bugzilla.mozilla.org/show_bug.cgi?id=1111301 (it's hard to 
tell), but no one else appears to have diagnosed the bug, so it seems 
that it is eligible for the bug bounty.

Sincerely,

Ronald Crane
Independent Security Researcher

Comment 1

[Andrew McCreight [:mccr8]]

Milan, can somebody look into this?  (We also have bug 1095510 on file for this crash signature, but I don't think we figured out anything there.)

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Check if the lock failed before proceeding. r=bas

Comment 3

[q1]

I have just reproduced this bug on Firefox 38.0.1, with the stack trace below.

The call from CairoTextureClientD3D9::BorrowDrawTarget into LockRect failed with status 0x8876086C (D3DERR_INVALIDCALL). At that point, rect.pBits contained 0x02105484 and rect.Pitch contained 0x216315a8. Note that 0x02105484 is code at gfxWindowsPlatform::GetD3D9Device+6, which is the return address from gfxWindowsPlatform::GetD3D9Device's call to gfxWindowsPlatform::GetD3D9DeviceManager.  gfxWindowsPlatform::GetD3D9Device, in turn, was called by CairoTextureClientD3D9::Lock. Thus, those calls populated the memory to which BorrowDrawTarget::rect was assigned. This is exactly the same scenario as in Firefox 36.0.1, where rect.pBits gets 0x2129cbd (=gfxWindowsPlatform::GetD3D9Device+6).

I currently have this code suspended in the debugger, so if there's anything you'd like me to inspect, please say so.

Here's the stack trace:

>	xul.dll!mozilla::layers::CairoTextureClientD3D9::BorrowDrawTarget()  Line 697	C++
 	xul.dll!mozilla::layers::CairoTextureClientD3D9::Lock(mozilla::layers::OpenMode aMode=OPEN_READ_WRITE)  Line 612 + 0x7 bytes	C++
 	xul.dll!mozilla::layers::ContentClientSingleBuffered::FinalizeFrame(const nsIntRegion & aRegionToDraw={...})  Line 670	C++
 	xul.dll!mozilla::layers::RotatedContentBuffer::BeginPaint(mozilla::layers::PaintedLayer * aLayer=0x16830ca0, unsigned int aFlags=0x00000004)  Line 558	C++
 	xul.dll!mozilla::layers::ContentClientRemoteBuffer::BeginPaintBuffer(mozilla::layers::PaintedLayer * aLayer=0x16830ca0, unsigned int aFlags=0x00000004)  Line 223 + 0x14 bytes	C++
 	xul.dll!mozilla::layers::ClientPaintedLayer::PaintThebes()  Line 55	C++
 	xul.dll!mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor * aReadback=0x0012eaec)  Line 132	C++
 	xul.dll!mozilla::layers::ClientContainerLayer::RenderLayer()  Line 71	C++
 	xul.dll!mozilla::layers::ClientLayerManager::EndTransactionInternal(void (mozilla::layers::PaintedLayer *, gfxContext *, const nsIntRegion &, mozilla::layers::DrawRegionClip, const nsIntRegion &, void *)* aCallback=0x01a07db3, void * aCallbackData=0x0012f098, mozilla::layers::LayerManager::EndTransactionFlags __formal=0x0012ebe4)  Line 275	C++
 	xul.dll!mozilla::layers::ClientLayerManager::EndTransaction(void (mozilla::layers::PaintedLayer *, gfxContext *, const nsIntRegion &, mozilla::layers::DrawRegionClip, const nsIntRegion &, void *)* aCallback=0x01a07db3, void * aCallbackData=0x0012f098, mozilla::layers::LayerManager::EndTransactionFlags aFlags=END_DEFAULT)  Line 318	C++
 	xul.dll!nsDisplayList::PaintRoot(nsDisplayListBuilder * aBuilder=0x0012f098, nsRenderingContext * aCtx=0x00000000, unsigned int aFlags=0x0000000d)  Line 1737	C++
 	xul.dll!nsLayoutUtils::PaintFrame(nsRenderingContext * aRenderingContext=0x7ffdf000, nsIFrame * aFrame=0x07ff3a64, const nsRegion & aDirtyRegion={...}, unsigned int aBackstop=0xffffffff, unsigned int aFlags=0x00000304)  Line 3199 + 0x1e bytes	C++
 	xul.dll!PresShell::Paint(nsView * aViewToPaint=0x138aee70, const nsRegion & aDirtyRegion={...}, unsigned int aFlags=0x00000001)  Line 6359 + 0x10 bytes	C++
 	xul.dll!nsViewManager::ProcessPendingUpdatesPaint(nsIWidget * aWidget=0x0efc2800)  Line 443 + 0x25 bytes	C++
 	xul.dll!nsViewManager::ProcessPendingUpdatesForView(nsView * aView=0x138aee70, bool aFlushDirtyRegion=true)  Line 380	C++
 	xul.dll!nsViewManager::ProcessPendingUpdates()  Line 1076	C++
 	xul.dll!nsRefreshDriver::Tick(__int64 aNowEpoch=0x****************, mozilla::TimeStamp aNowTime={...})  Line 1718	C++
 	xul.dll!mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver * driver=0x7ffdf000, __int64 jsnow=0x****************, mozilla::TimeStamp now={...})  Line 199	C++
 	xul.dll!mozilla::RefreshDriverTimer::Tick(__int64 jsnow=0x****************, mozilla::TimeStamp now={...})  Line 189 + 0x1a bytes	C++
 	xul.dll!mozilla::RefreshDriverTimer::TimerTick(nsITimer * aTimer=0x0895c740, void * aClosure=0x084d7220)  Line 213	C++
 	xul.dll!nsTimerImpl::Fire()  Line 631 + 0x6 bytes	C++
 	xul.dll!nsTimerEvent::Run()  Line 729	C++
 	xul.dll!nsThread::ProcessNextEvent(bool aMayWait=false, bool * aResult=0x0012fa54)  Line 855 + 0x6 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x03d67101)  Line 99 + 0x18 bytes	C++
 	xul.dll!MessageLoop::RunHandler()  Line 227	C++
 	xul.dll!MessageLoop::Run()  Line 201	C++
 	xul.dll!nsBaseAppShell::Run()  Line 166	C++
 	xul.dll!nsAppShell::Run()  Line 178 + 0x9 bytes	C++
 	xul.dll!nsAppStartup::Run()  Line 282	C++
 	xul.dll!XREMain::XRE_mainRun()  Line 4228 + 0xa bytes	C++
 	xul.dll!XREMain::XRE_main(int argc=0x00000000, char * * argv=0x00154890, const nsXREAppData * aAppData=0x0012fd00)  Line 4308 + 0x7 bytes	C++
 	xul.dll!XRE_main(int argc=0x00000003, char * * argv=0x00154890, const nsXREAppData * aAppData=0x0012fd80, unsigned int aFlags=0x00000000)  Line 4528	C++
 	firefox.exe!do_main(int argc=0x7ffdf000, char * * argv=0x07ff3a64, nsIFile * xreDirectory=0x03d421c0)  Line 294 + 0x13 bytes	C++
 	firefox.exe!NS_internal_main(int argc=0x7ffdf000, char * * argv=0x07ff3a64)  Line 669	C++
 	firefox.exe!wmain(int argc=0x00154890, wchar_t * * argv=0x00154a08)  Line 124	C++
 	firefox.exe!__tmainCRTStartup()  Line 255 + 0x12 bytes	C
 	kernel32.dll!7c816037() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]

Comment 4

[Bas Schouten (:bas.schouten)]

Comment on attachment 8607689 [details] [diff] [review]
Check if the lock failed before proceeding. r=bas

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

gfxCriticalError probably isn't a bad idea here so we understand why this is happening. Fwiw, I don't think this is particularly easy to exploit, but it's a bad bug.

::: gfx/layers/d3d9/CompositorD3D9.cpp
@@ +753,5 @@
>  
>    D3DLOCKED_RECT rect;
> +  if (FAILED(destSurf->LockRect(&rect, nullptr, D3DLOCK_READONLY)) ||
> +      !rect.pBits) {
> +    gfxWarning() << "Failed to lock rect in paint to target D3D9";

Could you get the error code and report it for these?

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Check if the lock failed before proceeding. Carry r=bas

Make it a critical warning.  https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf8ef7320d58

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c3d5d623a90

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Given that this affects multiple branches and has no security rating, I think this needs to get sec-approval first. Also, a non-obsolete patch being attached would be nice.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Yes, this needs sec-approval? set on a patch and the template filled out, as well as a security rating in order to go in.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Based on the original description and comment 4, this sounds like sec-moderate.

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Check if the lock failed before proceeding. Carry r=bas

[Security approval request comment]
How easily could an exploit be constructed based on the patch? See comment 4, we don't know of an easy way.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?  Not obviously, it isn't clear from the patch what would make the lock fail.

Which older supported branches are affected by this flaw?  33+.

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  Not a difficult rebase.

How likely is this patch to cause regressions; how much testing does it need?  This is low risk, it just stops a potential crash further down the line form where the problem happened.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 8610751 [details] [diff] [review]
Check if the lock failed before proceeding. Carry r=bas

sec-approval=dveditz

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccc7ee045063

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Bit overzealous there, oops...

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ccc7ee045063

Please request Aurora/Beta/esr38 approval on this when you get a chance.

Comment 15

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8610751 [details] [diff] [review]
Check if the lock failed before proceeding. Carry r=bas

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Using uninitialized memory, though not necessarily easy to exploit.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Debug builds will crash instead of using uninitialized memory. 
[String/UUID change made/needed]:

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8610751 [details] [diff] [review]
Check if the lock failed before proceeding. Carry r=bas

Approving for uplift to beta and aurora. While this is sec-moderate and has been a regression for a while, it also may fix some crashes and so I'd like to have it on beta. 

Please don't land this on aurora until tomorrow (as Kwierso and I discussed) since we aren't reenabling updates on aurora yet.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/15a7bce855fe

Comment 18

[Daniel Veditz [:dveditz]]

We don't see a way to corrupt memory or get execution control with these uninitialized values

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b984c2cafc4

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8610751 [details] [diff] [review]
Check if the lock failed before proceeding. Carry r=bas

We didn't see regressions from this patch. Taking it in ESR to.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/b3bd91502947