Closed Bug 1190526 (CVE-2015-7179) Opened 4 years ago Closed 4 years ago

Overflow in VertexBufferInterface::reserveVertexSpace causes memory-safety bug

Categories

(Core :: Graphics, defect)

39 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox40 + wontfix
firefox41 + verified
firefox42 + fixed
firefox43 + fixed
firefox-esr38 41+ fixed
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-v2.2r --- unaffected
b2g-master --- unaffected

People

(Reporter: q1, Assigned: kyle_fung)

References

Details

(Keywords: csectype-bounds, sec-critical, Whiteboard: [adv-main41+][adv-esr38.3+])

Attachments

(2 files, 1 obsolete file)

VertexBufferInterface::reserveVertexSpace (gfx\angle\src\libGLESv2\renderer\d3d\VertexBuffer.cpp) can incur an overflow with a specially-crafted set of shader attribute arrays. The overflow causes VertexBufferInterface::storeVertexAttributes to fail to allocate a large-enough buffer, then to write the contents of one or more shader attribute arrays (whose contents an attacker can prescribe) far beyond the buffer's end.

This bug can be manifested under Win64 with D3D 11 (see attached proof-of-concept [0] and details below) and probably also under Linux x64. Someone with more knowledge of WebGL might also be able to manifest this bug under Win32 and other 32-bit platforms.

Details:
------------------------------------------------------------------------------------------------------

The bug is in VertexBufferInterface::reserveVertexSpace:

135: gl::Error VertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib, GLsizei count, GLsizei instances)

136: {
137:    gl::Error error(GL_NO_ERROR);
138:
139:    unsigned int requiredSpace;
140:    error = mVertexBuffer->getSpaceRequired(attrib, count, instances, &requiredSpace);
141:    if (error.isError())
142:    {
143:        return error;
144:    }
145:
146:    // Protect against integer overflow
147:    if (mReservedSpace + requiredSpace < mReservedSpace)
148:    {
149:        return gl::Error(GL_OUT_OF_MEMORY, "Unable to reserve %u extra bytes in internal vertex buffer, "
150:                         "it would result in an overflow.", requiredSpace);
151:    }
152:
153:    mReservedSpace += requiredSpace;
154:
155:    // Align to 16-byte boundary
156:    mReservedSpace = rx::roundUp (mReservedSpace, 16u);
157:
158:    return gl::Error(GL_NO_ERROR);
159: }

The checks on line 147-51 close the overflow window, but the round-up on line 156 reopens it just a crack. If 
the WebGL program  uses 8 attribute arrays of size 0x1FFFFFF8, mReservedSpace on line 156 rounds up each time
(from 0x1FFFFFF8 to 0x20000000 on the 1st array, 0x3ffffff8 to 0x40000000 on the 2nd) and finally overflows from 0xFFFFFFF8 to 0 on the last array.

Later, when VertexBufferInterface::storeVertexAttributes is called to save the attributes into the buffer, it calls StreamingVertexBufferInterface::reserveSpace  with that same buffer size (0). reserveSpace then leaves
the existing default buffer of length 0x100000 bytes [1] in place:

209: gl::Error StreamingVertexBufferInterface::reserveSpace(unsigned int size)
210: {
211:     unsigned int curBufferSize = getBufferSize();
212:     if (size > curBufferSize)
213:     {
214:         gl::Error error = setBufferSize(std::max(size, 3 * curBufferSize / 2));
215:         if (error.isError())
216:         {
217:             return error;
218:         }
219:         setWritePosition(0);
220:     }
221:     else if (getWritePosition() + size > curBufferSize)
222:     {
223:         gl::Error error = discard();
224:         if (error.isError())
225:         {
226:             return error;
227:         }
228:         setWritePosition(0);
229:     }
230: 
231:     return gl::Error(GL_NO_ERROR);
232: }

(size == 0, so control skips from line 212 to line 221, and thence to line 231).

Finally, VertexBufferInterface::storeVertexAttributes is called to copy an entire attribute array into the (0x100000-byte) buffer:

116:    error = mVertexBuffer->storeVertexAttributes(attrib, currentValue, start, count, instances, mWritePosition);

Since all of the proof-of-concept attribute arrays are 0x1FFFFFF8 bytes, this causes a potentially huge overrun. In testing the POC several times, this has had various effects:

1. Writing into a structure from which the video driver extracts a pointer, resulting in an attempt to read an invalid address (and probably other undetected corruption before the exception). This example is included, below. I saw this problem twice in different guises.

2. Writing into a function pointer, causing the video driver to attempt to call a function at an invalid address.

3. Writing into a function pointer, causing nss3.dll!PR_GetEnv to attempt to call a function at an invalid address.

4. A stack overflow with unknown corruption beforehand.

5. The display going blank, then partially repainting, with the message "Display driver nvlddmkm stopped responding and has successfully recovered" popping up, followed by an exception hitting an inaccessible page.

6. Hitting an inaccessible page, causing an exception, after overwriting varied amounts of unowned memory with no visible effects.

------------------------------------------------------------------------------------------------------
Manifesting the bug 

The following crash occured while running the attached proof-of-concept program. The setup was to run FF 40b8 x64 on Win7 SP1 (I wasn't able to find 39.0 x64. Where is it distributed?). I attached the VS debugger to FF, then opened 4 windows and navigated to the following sites in 3 of them:

   Dynamic procedural terrain (http://alteredqualia.com/three/examples/webgl_terrain_dynamic.html )
   Bill Nye Reading Mean Tweets (https://www.youtube.com/watch?v=mm4Rwyi-k08 )
   WebGL Aquarium (http://webglsamples.org/aquarium/aquarium.html )

[See note [2]]

I then ran the proof-of-concept program in the 4th window. At the "unresponsive script" alert box, I clicked "continue". A few seconds later the following crash appeared in VS:

Exception thrown at 0x000007FEEA9CE79A (nvwgf2umx.dll) in firefox.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF.

Investigating, I found that the actual address that was read to cause the exception was different. Reading the code stream:

000007FEEA9CE760  mov         qword ptr [rsp+10h],rbx  
000007FEEA9CE765  push        rdi  
000007FEEA9CE766  mov         rcx,qword ptr [r9+20h]  
000007FEEA9CE76A  or          r11,0FFFFFFFFFFFFFFFFh  
000007FEEA9CE76E  xor         r10d,r10d  
000007FEEA9CE771  xor         ebx,ebx  
000007FEEA9CE773  mov         rdi,rdx  
000007FEEA9CE776  test        rcx,rcx  
000007FEEA9CE779  je          000007FEEA9CE806  
000007FEEA9CE77F  mov         rdx,qword ptr [rcx+40h]  
000007FEEA9CE783  mov         qword ptr [rsp+10h],rsi  
000007FEEA9CE788  mov         rax,qword ptr [rdx]  
000007FEEA9CE78B  test        rax,rax  
000007FEEA9CE78E  je          000007FEEA9CE7BD  
000007FEEA9CE790  mov         r10,qword ptr [rax+1E8h]  
000007FEEA9CE797  mov         rbx,rdx  
> 000007FEEA9CE79A  mov         rax,qword ptr [r10+118h]  
000007FEEA9CE7A1  mov         r11,qword ptr [r10+110h]  
000007FEEA9CE7A8  cmp         r11,rax  
000007FEEA9CE7AB  cmovbe      r11,rax  
000007FEEA9CE7AF  mov         rax,qword ptr [r10+120h]  
000007FEEA9CE7B6  cmp         r11,rax  

We find that r10 at the crashing instruction is 0x433a0000433a0000, which is exactly the bad data that the POC writes [3]. r10 came from [rax+1E8h], which contained:

0x000000003D384F98  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C
0x000000003D384FA8  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C
0x000000003D384FB8  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C
0x000000003D384FC8  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C
0x000000003D384FD8  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C
0x000000003D384FE8  00 00 3a 43 00 00 3a 43 00 00 3a 43 00 00 3a 43  ..:C..:C..:C..:C

The thread's registers were:

RAX = 000000003D384DB0 RBX = 000000000F36D540 RCX = 000000000F36B180 RDX = 000000000F36D540 
RSI = 0000000000000002 RDI = 000000000FBFE958 R8  = 000000000FBFE9A8 R9  = 000000000F36D770 
R10 = 433A0000433A0000 R11 = FFFFFFFFFFFFFFFF R12 = 0000000000000002 R13 = 0000000000000002 
R14 = 00000000542CA6A0 R15 = 000000000F36D770 RIP = 000007FEEA9CE79A RSP = 000000000FBFE910 
RBP = 00000000004C2DD0 EFL = 00010202 

And the thread's stack was:

>	nvwgf2umx.dll!000007feea9ce79a()	Unknown
 	nvwgf2umx.dll!000007feea94b040()	Unknown
 	nvwgf2umx.dll!000007feea94ac14()	Unknown
 	nvwgf2umx.dll!000007feea3b944e()	Unknown
 	nvwgf2umx.dll!000007feea2c2df4()	Unknown
 	nvwgf2umx.dll!000007feea3ea279()	Unknown
 	d3d11.dll!CResource<struct ID3D11Resource>::Map<0,5>(class CContext *,class CResource<struct 

ID3D11Resource> *,unsigned int,enum D3D11_MAP,unsigned int,struct D3D11_MAPPED_SUBRESOURCE *)	Unknown
 	d3d11.dll!CContext::ID3D11DeviceContext1_Map_<1>(struct ID3D11DeviceContext1 *,struct ID3D11Resource 

*,unsigned int,enum D3D11_MAP,unsigned int,struct D3D11_MAPPED_SUBRESOURCE *)	Unknown
 	xul.dll!mozilla::layers::CompositorD3D11::UpdateConstantBuffers() Line 1347	C++
 	xul.dll!mozilla::layers::CompositorD3D11::ClearRect(const 

mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> & aRect) Line 600	C++
 	xul.dll!mozilla::layers::CompositorD3D11::BeginFrame(const nsIntRegion & aInvalidRegion, const 

mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> * aClipRectIn, const 

mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> & aRenderBounds, 

mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> * aClipRectOut, 

mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> * aRenderBoundsOut) Line 1076	C++
 	xul.dll!mozilla::layers::LayerManagerComposite::Render() Line 718	C++
 	xul.dll!mozilla::layers::LayerManagerComposite::EndTransaction(void (mozilla::layers::PaintedLayer *, 

gfxContext *, const nsIntRegion &, mozilla::layers::DrawRegionClip, const nsIntRegion &, void *) * aCallback, 

void * aCallbackData, mozilla::layers::LayerManager::EndTransactionFlags aFlags) Line 319	C++
 	xul.dll!mozilla::layers::LayerManagerComposite::EndEmptyTransaction

(mozilla::layers::LayerManager::EndTransactionFlags aFlags) Line 262	C++
 	xul.dll!mozilla::layers::CompositorParent::CompositeToTarget(mozilla::gfx::DrawTarget * aTarget, const 

mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> * aRect) Line 1143	C++
 	xul.dll!mozilla::layers::CompositorVsyncScheduler::Composite(mozilla::TimeStamp aVsyncTimestamp) Line 

519	C++
 	xul.dll!RunnableMethod<SoftwareDisplay,void (__cdecl SoftwareDisplay::*)(mozilla::TimeStamp) 

__ptr64,Tuple1<mozilla::TimeStamp> >::Run() Line 311	C++
 	xul.dll!MessageLoop::DoWork() Line 456	C++
 	xul.dll!base::MessagePumpForUI::DoRunLoop() Line 217	C++
 	xul.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 78	C++
 	xul.dll!MessageLoop::RunHandler() Line 227	C++
 	xul.dll!MessageLoop::Run() Line 201	C++
 	xul.dll!base::Thread::ThreadMain() Line 173	C++
 	xul.dll!`anonymous namespace'::ThreadFunc(void * closure) Line 27	C++
 	kernel32.dll!BaseThreadInitThunk()	Unknown
 	ntdll.dll!RtlUserThreadStart()	Unknown

and the thread's description was:

Not Flagged	>	0x00000CF4	0x00	Worker Thread	xul.dll!`anonymous namespace'::ThreadFunc	

nvwgf2umx.dll!000007feea9ce79a	Normal


BUT the main thread was still in memcpy, still overwriting memory that it didn't own:

Not Flagged		0x000012B0	0x00	Main Thread	Main Thread	msvcr120.dll!memcpy	Normal

and its stack was:

 	msvcr120.dll!memcpy() Line 357	Unknown
>	libGLESv2.dll!rx::VertexBuffer11::storeVertexAttributes(const gl::VertexAttribute & attrib, const 

gl::VertexAttribCurrentValueData & currentValue, int start, int count, int instances, unsigned int offset) Line 

122	C++
 	libGLESv2.dll!rx::VertexBufferInterface::storeVertexAttributes(const gl::VertexAttribute & attrib, 

const gl::VertexAttribCurrentValueData & currentValue, int start, int count, int instances, unsigned int * 

outStreamOffset) Line 116	C++
 	libGLESv2.dll!rx::VertexDataManager::storeAttribute(const gl::VertexAttribute & attrib, const 

gl::VertexAttribCurrentValueData & currentValue, rx::TranslatedAttribute * translated, int start, int count, 

int instances) Line 295	C++
 	libGLESv2.dll!rx::VertexDataManager::prepareVertexData(const gl::State & state, int start, int count, 

rx::TranslatedAttribute * translated, int instances) Line 131	C++
 	libGLESv2.dll!rx::Renderer11::applyVertexBuffer(const gl::State & state, int first, int count, int 

instances) Line 994	C++
 	libGLESv2.dll!gl::Context::drawArrays(unsigned int mode, int first, int count, int instances) Line 1786	

C++
 	libGLESv2.dll!glDrawArrays(unsigned int mode, int first, int count) Line 1387	C++
 	xul.dll!mozilla::gl::GLContext::fDrawArrays(unsigned int mode, int first, int count) Line 1144	C++
 	xul.dll!mozilla::WebGLContext::DrawArrays(unsigned int mode, int first, int count) Line 142	C++
 	xul.dll!mozilla::dom::WebGLRenderingContextBinding::drawArrays(JSContext * cx, JS::Handle<JSObject *> 

obj, mozilla::WebGLContext * self, const JSJitMethodCallArgs & args) Line 10758	C++
 	xul.dll!mozilla::dom::GenericBindingMethod(JSContext * cx, unsigned int argc, JS::Value * vp) Line 2615	

C++
 	xul.dll!js::Invoke(JSContext * cx, JS::CallArgs args, js::MaybeConstruct construct) Line 753	C++
 	xul.dll!Interpret(JSContext * cx, js::RunState & state) Line 2962	C++
 	xul.dll!js::RunScript(JSContext * cx, js::RunState & state) Line 683	C++
 	xul.dll!js::Invoke(JSContext * cx, JS::CallArgs args, js::MaybeConstruct construct) Line 756	C++
 	xul.dll!js::Invoke(JSContext * cx, const JS::Value & thisv, const JS::Value & fval, unsigned int argc, 

const JS::Value * argv, JS::MutableHandle<JS::Value> rval) Line 790	C++
 	xul.dll!mozilla::dom::EventHandlerNonNull::Call(JSContext * cx, JS::Handle<JS::Value> aThisVal, 

mozilla::dom::Event & event, JS::MutableHandle<JS::Value> aRetVal, mozilla::ErrorResult & aRv) Line 260	C++
 	xul.dll!mozilla::dom::EventHandlerNonNull::Call<nsISupports * __ptr64>(nsISupports * const & thisVal, 

mozilla::dom::Event & event, JS::MutableHandle<JS::Value> aRetVal, mozilla::ErrorResult & aRv, const char * 

aExecutionReason, mozilla::dom::CallbackObject::ExceptionHandling aExceptionHandling, JSCompartment * 

aCompartment) Line 351	C++
 	xul.dll!mozilla::JSEventHandler::HandleEvent(nsIDOMEvent * aEvent) Line 216	C++
 	xul.dll!mozilla::EventListenerManager::HandleEventInternal(nsPresContext * aPresContext, 

mozilla::WidgetEvent * aEvent, nsIDOMEvent * * aDOMEvent, mozilla::dom::EventTarget * aCurrentTarget, 

nsEventStatus * aEventStatus) Line 1129	C++
 	xul.dll!mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem> & 

aChain, mozilla::EventChainPostVisitor & aVisitor, mozilla::EventDispatchingCallback * aCallback, 

mozilla::ELMCreationDetector & aCd) Line 301	C++
 	xul.dll!mozilla::EventDispatcher::Dispatch(nsISupports * aTarget, nsPresContext * aPresContext, 

mozilla::WidgetEvent * aEvent, nsIDOMEvent * aDOMEvent, nsEventStatus * aEventStatus, 

mozilla::EventDispatchingCallback * aCallback, nsTArray<mozilla::dom::EventTarget *> * aTargets) Line 638	

C++
 	xul.dll!nsDocumentViewer::LoadComplete(nsresult aStatus) Line 1000	C++
 	xul.dll!nsDocShell::EndPageLoad(nsIWebProgress * aProgress, nsIChannel * aChannel, nsresult aStatus) 

Line 7562	C++
 	xul.dll!nsDocShell::OnStateChange(nsIWebProgress * aProgress, nsIRequest * aRequest, unsigned int 

aStateFlags, nsresult aStatus) Line 7371	C++
 	xul.dll!nsDocLoader::DoFireOnStateChange(nsIWebProgress * const aProgress, nsIRequest * const aRequest, 

int & aStateFlags, const nsresult aStatus) Line 1250	C++
 	xul.dll!nsDocLoader::doStopDocumentLoad(nsIRequest * request, nsresult aStatus) Line 829	C++
 	xul.dll!nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) Line 721	C++
 	xul.dll!nsDocLoader::OnStopRequest(nsIRequest * aRequest, nsISupports * aCtxt, nsresult aStatus) Line 

606	C++
 	xul.dll!nsLoadGroup::RemoveRequest(nsIRequest * request, nsISupports * ctxt, nsresult aStatus) Line 652	

C++
 	xul.dll!nsDocument::DoUnblockOnload() Line 9160	C++
 	xul.dll!nsDocument::UnblockOnload(bool aFireSync) Line 9089	C++
 	xul.dll!nsDocument::DispatchContentLoadedEvents() Line 5225	C++
 	xul.dll!nsRunnableMethodImpl<void (__cdecl imgRequestProxy::*)(void) __ptr64,1>::Run() Line 811	C++
 	xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 872	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate) Line 95	C++
 	xul.dll!MessageLoop::RunHandler() Line 227	C++
 	xul.dll!MessageLoop::Run() Line 201	C++
 	xul.dll!nsBaseAppShell::Run() Line 167	C++
 	xul.dll!nsAppShell::Run() Line 180	C++
 	xul.dll!nsAppStartup::Run() Line 281	C++
 	xul.dll!XREMain::XRE_mainRun() Line 4079	C++
 	xul.dll!XREMain::XRE_main(int argc, char * * argv, const nsXREAppData * aAppData) Line 4170	C++
 	xul.dll!XRE_main(int argc, char * * argv, const nsXREAppData * aAppData, unsigned int aFlags) Line 4260	

C++
 	firefox.exe!do_main(int argc, char * * argv, nsIFile * xreDirectory) Line 214	C++
 	firefox.exe!NS_internal_main(int argc, char * * argv) Line 480	C++
 	firefox.exe!wmain(int argc, wchar_t * * argv) Line 138	C++
 	firefox.exe!__tmainCRTStartup() Line 255	C
 	kernel32.dll!BaseThreadInitThunk()	Unknown
 	ntdll.dll!RtlUserThreadStart()	Unknown

Examining the libGLESv2.dll!rx::VertexBuffer11::storeVertexAttributes frame, we find that the code used the following parameters:

  0x000000003bed0000 pData
+ 0x00000000000e0a20 offset
= 0x000000003BFB0A20 attribute buffer base
  0x000000003C0B0A1F attribute buffer end

  0x000000003D384F98 is the address from which the video driver read its pointer

Examining the main thread code memcpy frame, we see that the thread stopped at:

000007FEF6BBC623  nop         word ptr [rax+rax]  
000007FEF6BBC630  mov         rax,qword ptr [rdx+rcx]  
--- No source file -------------------------------------------------------------
> 000007FEF6BBC634  mov         r10,qword ptr [rdx+rcx+8]  
000007FEF6BBC639  add         rcx,20h  
000007FEF6BBC63D  mov         qword ptr [rcx-20h],rax  
000007FEF6BBC641  mov         qword ptr [rcx-18h],r10  
000007FEF6BBC645  mov         rax,qword ptr [rdx+rcx-10h]  
000007FEF6BBC64A  mov         r10,qword ptr [rdx+rcx-8]  
000007FEF6BBC64F  dec         r9  
000007FEF6BBC652  mov         qword ptr [rcx-10h],rax  
000007FEF6BBC656  mov         qword ptr [rcx-8],r10  
000007FEF6BBC65A  jne         MoveSmall+190h (07FEF6BBC630h)  
000007FEF6BBC65C  and         r8,1Fh  
000007FEF6BBC660  jmp         mcpy00aa+73h (07FEF6BBC457h)  

with registers:

RAX = 433A0000433A0000 RBX = 000000000023B230 RCX = 000000003D48F820 RDX = 000000006404F5E0 
RSI = 0000000000000000 RDI = 00000000494C1800 R8  = 000000001FFFFFF8 R9  = 0000000000F5908F 
R10 = 433A0000433A0000 R11 = 000000003BFB0A20 R12 = 00000000111BEDC0 R13 = 000000002AFBB400 
R14 = 000000003BFB0A20 R15 = 0000000000000004 RIP = 000007FEF6BBC634 RSP = 000000000023B0D8 
RBP = 000000000023B171 EFL = 00000212 

So the last data it wrote was at

0x000000003D48F820 - 8 == 0x000000003D48F818


This means that memcpy wrote

  0x000000003D48F818 - 0x000000003BFB0A20 == 0x00000000014DEDF8

bytes, beginning at the buffer's base, extending far beyond its end, including the memory from which the video driver read its pointer, and terminating only when the OS suspended the process's threads due to the exception in the video driver thread.


------------------------------------------------------------------------------------------------------
[0] Extract poc.js, poc.htm, and glMatrix-0.9.5.min.js from the attachment. Save them all in the same folder and load poc.htm from that folder.

[1] This is set by VertexDataManager::VertexDataManager using the constant INITIAL_STREAM_BUFFER_SIZE.

[2] It's probably easiest to reproduce obviously adverse effects from this bug by running 2 windows of "Dynamic Procedural Terrain" and one of "Bill Nye Reading Mean Tweets". You might need to try it several times. You can witness the overwriting directly by putting a breakpoint on VertexBuffer11::storeVertexAttributes and stepping into its call to vertexFormatInfo.copyFunction.

[3] See poc.js line 149, which assigns the (float) attribute array elements the value 0xba. This is represented as 0x433a0000.
Summary: Overflow in VertexBuffer11::getSpaceRequired causes memory-safety bug → Overflow in VertexBufferInterface::reserveVertexSpace causes memory-safety bug
There is a similar bug in VertexBufferInterface::storeVertexAttributes at line 130:

130:    mWritePosition = rx::roundUp(mWritePosition, 16u);

but I have not yet examined whether it can be leveraged into a vulnerability.
Flags: sec-bounty?
Assignee: nobody → kfung
Attached patch rounded-check.patch (obsolete) — Splinter Review
Haven't observed the crash yet, but I think this is the bounds check that needs to get added.
Attachment #8647145 - Flags: review?(jmuizelaar)
(In reply to kfung from comment #3)
> Created attachment 8647145 [details] [diff] [review]
> rounded-check.patch
> 
> Haven't observed the crash yet, but I think this is the bounds check that
> needs to get added.

You'll need an x64 version of Firefox and lots of RAM. I used FF 40b8 x64 on Win7 SP1 with D3D 11 and 12 GB RAM.

Also, you might want to fix the similar bug described in comment 1.
Upstream fixed this bug rather differently, and I think more solidly. https://code.google.com/p/chromium/issues/detail?id=518206 .
(In reply to q1 from comment #5)
> Upstream fixed this bug rather differently, and I think more solidly.
> https://code.google.com/p/chromium/issues/detail?id=518206 .

I don't have permission to view that link.
Sorry about that. Here's the diff. Note that the base version is different, so slight merging will be needed:

GoogleGit
Sign in
chromium / angle/angle / 709dc46cbd06c98b7450d702ade3210eef831a70^! / .
commit	709dc46cbd06c98b7450d702ade3210eef831a70[log][tgz]	
author	Jamie Madill <jmadill@chromium.org>	Mon Aug 10 18:28:54 2015
committer	Jamie Madill <jmadill@chromium.org>	Fri Aug 14 17:48:48 2015
tree	6b844e8e965442666b1cbd687534386bbcff1332
parent	fa9744b09e2478c75a25fd1b497469d429e81591[diff]

D3D: Fix buffer overflow in VertexBuffer.cpp.

Under certain situations an integer overflow could lead to ANGLE
writing to places where it shouldn't.

BUG=518206

Change-Id: I9217685daecb160a4072fbf79c26e5bee9f4621e
Reviewed-on: https://chromium-review.googlesource.com/293820
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Jamie Madill <jmadill@chromium.org>

diff --git a/src/libANGLE/renderer/d3d/VertexBuffer.cpp b/src/libANGLE/renderer/d3d/VertexBuffer.cpp
index 19bd548..129d0f9 100644
--- a/src/libANGLE/renderer/d3d/VertexBuffer.cpp
+++ b/src/libANGLE/renderer/d3d/VertexBuffer.cpp

@@ -102,7 +102,12 @@
         return error;
     }
 
-    if (mWritePosition + spaceRequired < mWritePosition)
+    // Align to 16-byte boundary
+    unsigned int alignedSpaceRequired = roundUp(spaceRequired, 16u);
+
+    // Protect against integer overflow
+    if (!IsUnsignedAdditionSafe(mWritePosition, alignedSpaceRequired) ||
+        alignedSpaceRequired < spaceRequired)
     {
         return gl::Error(GL_OUT_OF_MEMORY, "Internal error, new vertex buffer write position would overflow.");
     }
@@ -125,10 +130,7 @@
         *outStreamOffset = mWritePosition;
     }
 
-    mWritePosition += spaceRequired;
-
-    // Align to 16-byte boundary
-    mWritePosition = roundUp(mWritePosition, 16u);
+    mWritePosition += alignedSpaceRequired;
 
     return gl::Error(GL_NO_ERROR);
 }
@@ -144,17 +146,18 @@
         return error;
     }
 
+    // Align to 16-byte boundary
+    unsigned int alignedRequiredSpace = roundUp(requiredSpace, 16u);
+
     // Protect against integer overflow
-    if (mReservedSpace + requiredSpace < mReservedSpace)
+    if (!IsUnsignedAdditionSafe(mReservedSpace, alignedRequiredSpace) ||
+        alignedRequiredSpace < requiredSpace)
     {
         return gl::Error(GL_OUT_OF_MEMORY, "Unable to reserve %u extra bytes in internal vertex buffer, "
                          "it would result in an overflow.", requiredSpace);
     }
 
-    mReservedSpace += requiredSpace;
-
-    // Align to 16-byte boundary
-    mReservedSpace = roundUp(mReservedSpace, 16u);
+    mReservedSpace += alignedRequiredSpace;
 
     return gl::Error(GL_NO_ERROR);
 }

diff --git a/src/tests/gl_tests/BufferDataTest.cpp b/src/tests/gl_tests/BufferDataTest.cpp
index afd3c68..d8158a8 100644
--- a/src/tests/gl_tests/BufferDataTest.cpp
+++ b/src/tests/gl_tests/BufferDataTest.cpp

@@ -28,7 +28,7 @@
         mAttribLocation = -1;
     }
 
-    virtual void SetUp()
+    void SetUp() override
     {
         ANGLETest::SetUp();
 
@@ -72,7 +72,7 @@
         ASSERT_GL_NO_ERROR();
     }
 
-    virtual void TearDown()
+    void TearDown() override
     {
         glDeleteBuffers(1, &mBuffer);
         glDeleteProgram(mProgram);
@@ -221,7 +221,7 @@
         setConfigDepthBits(24);
     }
 
-    virtual void SetUp()
+    void SetUp() override
     {
         ANGLETest::SetUp();
 
@@ -267,7 +267,7 @@
         ASSERT_GL_NO_ERROR();
     }
 
-    virtual void TearDown()
+    void TearDown() override
     {
         glDeleteBuffers(2, mBuffers);
         glDeleteBuffers(1, &mElementBuffer);
@@ -401,10 +401,108 @@
 }
 
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
+ANGLE_INSTANTIATE_TEST(BufferDataTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());
 ANGLE_INSTANTIATE_TEST(BufferDataTestES3, ES3_D3D11());
-
-// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(IndexedBufferCopyTest, ES3_D3D11());
 
-// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
-ANGLE_INSTANTIATE_TEST(BufferDataTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());
+#ifdef _WIN64
+
+// Test a bug where an integer overflow bug could trigger a crash in D3D.
+// The test uses 8 buffers with a size just under 0x2000000 to overflow max uint
+// (with the internal D3D rounding to 16-byte values) and trigger the bug.
+// Only handle this bug on 64-bit Windows for now. Harder to repro on 32-bit.
+class BufferDataOverflowTest : public ANGLETest
+{
+  protected:
+    BufferDataOverflowTest()
+        : mProgram(0)
+    {
+    }
+
+    ~BufferDataOverflowTest()
+    {
+        if (!mBuffers.empty())
+        {
+            glDeleteBuffers(static_cast<GLsizei>(mBuffers.size()), &mBuffers[0]);
+        }
+        if (mProgram != 0u)
+        {
+            glDeleteProgram(mProgram);
+        }
+    }
+
+    std::vector<GLuint> mBuffers;
+    GLuint mProgram;
+};
+
+// See description above.
+TEST_P(BufferDataOverflowTest, VertexBufferIntegerOverflow)
+{
+    // These values are special, to trigger the rounding bug.
+    unsigned int numItems = 0x7FFFFFE;
+    GLsizei bufferCnt = 8;
+
+    mBuffers.resize(bufferCnt);
+
+    std::stringstream vertexShaderStr;
+
+    for (GLsizei bufferIndex = 0; bufferIndex < bufferCnt; ++bufferIndex)
+    {
+        vertexShaderStr << "attribute float attrib" << bufferIndex << ";\n";
+    }
+
+    vertexShaderStr << "attribute vec2 position;\n"
+                       "varying float v_attrib;\n"
+                       "void main() {\n"
+                       "  gl_Position = vec4(position, 0, 1);\n"
+                       "  v_attrib = 0.0;\n";
+
+    for (GLsizei bufferIndex = 0; bufferIndex < bufferCnt; ++bufferIndex)
+    {
+        vertexShaderStr << "v_attrib += attrib" << bufferIndex << ";\n";
+    }
+
+    vertexShaderStr << "}";
+
+    const std::string &fragmentShader =
+        "varying highp float v_attrib;\n"
+        "void main() {\n"
+        "  gl_FragColor = vec4(v_attrib, 0, 0, 1);\n"
+        "}";
+
+    mProgram = CompileProgram(vertexShaderStr.str(), fragmentShader);
+    ASSERT_NE(0u, mProgram);
+    glUseProgram(mProgram);
+
+    glGenBuffers(bufferCnt, &mBuffers[0]);
+
+    std::vector<GLfloat> data(numItems, 1.0f);
+
+    for (GLsizei bufferIndex = 0; bufferIndex < bufferCnt; ++bufferIndex)
+    {
+        glBindBuffer(GL_ARRAY_BUFFER, mBuffers[bufferIndex]);
+        glBufferData(GL_ARRAY_BUFFER, numItems * sizeof(float), &data[0], GL_DYNAMIC_DRAW);
+
+        std::stringstream attribNameStr;
+        attribNameStr << "attrib" << bufferIndex;
+
+        GLint attribLocation = glGetAttribLocation(mProgram, attribNameStr.str().c_str());
+        ASSERT_NE(-1, attribLocation);
+
+        glVertexAttribPointer(attribLocation, 1, GL_FLOAT, GL_FALSE, 4, nullptr);
+        glEnableVertexAttribArray(attribLocation);
+    }
+
+    GLint positionLocation = glGetAttribLocation(mProgram, "position");
+    ASSERT_NE(-1, positionLocation);
+    glDisableVertexAttribArray(positionLocation);
+    glVertexAttrib2f(positionLocation, 1.0f, 1.0f);
+
+    EXPECT_GL_NO_ERROR();
+    glDrawArrays(GL_TRIANGLES, 0, numItems);
+    EXPECT_GL_ERROR(GL_OUT_OF_MEMORY);
+}
+
+ANGLE_INSTANTIATE_TEST(BufferDataOverflowTest, ES3_D3D11());
+
+#endif  // _WIN64

Powered by Gitiles
The exact same fix as used in upstream.
Attachment #8647145 - Attachment is obsolete: true
Attachment #8647145 - Flags: review?(jmuizelaar)
Attachment #8648764 - Flags: review?(jmuizelaar)
Which versions does this affect? 
I'll track from 42+ for now.
The bug was reproducible by the reporter on 40, so it should affect at least all versions 40+.
The bug was present in the sources at least as far back as 38.0.1.
Attachment #8648764 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8648764 [details] [diff] [review]
overflow-check.patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
As detailed in the report, someone could cause an overflow with a specially crafted set of vertex attribute arrays.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Not really. Just mentions that we get rid of an overflow.

Which older supported branches are affected by this flaw?
At least as far back as 38.

If not all supported branches, which bug introduced the flaw?
This bug was introduced into ANGLE, so I'm not entirely sure.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I do not. They should probably fit into the other branches.

How likely is this patch to cause regressions; how much testing does it need?
There should be no regressions, and no testing is needed.
Attachment #8648764 - Flags: sec-approval?
[Tracking Requested - why for this release]:

(In reply to Kyle Fung from comment #12)
> Do comments in the patch, the check-in comment, or tests included in the
> patch paint a bulls-eye on the security problem?
> Not really. Just mentions that we get rid of an overflow.

"Overflow" is a word that causes immediate drooling in exploit writers, but I can't think of a better check-in message other than just say nothing but the bug number (but that also raises suspicions). Unfortunately the upstream check-in has the commit message "fix buffer overflow" and a test that shows how to trigger it so we shouldn't worry too much what we say in this case.

We might want to consider this as a 40.0.x ride-along if the opportunity comes up (I know we're looking at a linux top-crash).
Comment on attachment 8648764 [details] [diff] [review]
overflow-check.patch

sec-approval = dveditz

a=dveditz for aurora and beta.
Attachment #8648764 - Flags: sec-approval?
Attachment #8648764 - Flags: sec-approval+
Attachment #8648764 - Flags: approval-mozilla-beta+
Attachment #8648764 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/c69c21713dc8
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Flags: sec-bounty? → sec-bounty+
Comment on attachment 8648764 [details] [diff] [review]
overflow-check.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Exploiters could write into unowned memory.
Fix Landed on Version: 41
Risk to taking this patch (and alternatives if risky): No risks.
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(kyle_fung)
Attachment #8648764 - Flags: approval-mozilla-esr38?
Group: core-security → core-security-release
Comment on attachment 8648764 [details] [diff] [review]
overflow-check.patch

Taking this for ESR38 too.
Attachment #8648764 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Flags: qe-verify+
I've been trying to confirm the fix pushed for this bug, but I'm not seeing any difference between an affected build (e.g. 40.0b9 x64) and a fixed one (e.g. 41.0b8 x64/x86). Both builds are showing the "unresponsive script" alert box, but I was unable to reproduce the crash.

I've used Windows 10 Pro x64 with gfx.driver-init.feature-d3d11;true. Kyle, am I missing something here?
Flags: needinfo?(kyle_fung)
(In reply to Andrei Vaida, QA [:avaida] from comment #22)
> I've been trying to confirm the fix pushed for this bug, but I'm not seeing
> any difference between an affected build (e.g. 40.0b9 x64) and a fixed one
> (e.g. 41.0b8 x64/x86). Both builds are showing the "unresponsive script"
> alert box, but I was unable to reproduce the crash.
> 
> I've used Windows 10 Pro x64 with gfx.driver-init.feature-d3d11;true. Kyle,
> am I missing something here?

You do need lots of RAM so that your OS doesn't thrash. I used a system with 12 GB of RAM.
I didn't actually get the opportunity to reproduce the crash either. Though from the description of the bug, it was apparent that the problem existed, so I made the necessary fixes.

Try with the same setup as q1@lastland.net from comment 4:
> You'll need an x64 version of Firefox and lots of RAM. I used FF 40b8 x64 on Win7 SP1 with D3D 11 
> and 12 GB RAM.
Flags: needinfo?(kyle_fung)
(In reply to q1 from comment #23)
> (In reply to Andrei Vaida, QA [:avaida] from comment #22)
> > I've been trying to confirm the fix pushed for this bug, but I'm not seeing
> > any difference between an affected build (e.g. 40.0b9 x64) and a fixed one
> > (e.g. 41.0b8 x64/x86). Both builds are showing the "unresponsive script"
> > alert box, but I was unable to reproduce the crash.
> > 
> > I've used Windows 10 Pro x64 with gfx.driver-init.feature-d3d11;true. Kyle,
> > am I missing something here?
> 
> You do need lots of RAM so that your OS doesn't thrash. I used a system with
> 12 GB of RAM.

Also make sure that the browser really is using D3D11 by checking about:support, and click "continue" at the "unresponsive script" box.
I retested this POC on 40.0b8 x64 on Win7 SP1 with 12 GB RAM, by extracting the files from the attachment (in case I made some mistake posting them). It crashes as expected. I also tried it on 40.0 x64 debug (my build with a few trivial unrelated changes) and it crashes that version in the same way. If it's a clue, my about:support says

   GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
   WebGL Renderer: Google Inc. -- ANGLE (NVIDIA GeForce GT 610 Direct3D11 vs_5_0 ps_5_0)
Whiteboard: [adv-main41+][adv-esr38.3+]
(In reply to q1 from comment #26)
> I retested this POC on 40.0b8 x64 on Win7 SP1 with 12 GB RAM, by extracting
> the files from the attachment (in case I made some mistake posting them). It
> crashes as expected. I also tried it on 40.0 x64 debug (my build with a few
> trivial unrelated changes) and it crashes that version in the same way. If
> it's a clue, my about:support says
> 
>    GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
>    WebGL Renderer: Google Inc. -- ANGLE (NVIDIA GeForce GT 610 Direct3D11
> vs_5_0 ps_5_0)

The fix should be in Firefox 41 and newer. Can you try and see if it still crashes with:
- Firefox 41 RC - http://ftp.mozilla.org/pub/firefox/nightly/41.0-candidates/build1/
- ESR 38.3.0 - http://ftp.mozilla.org/pub/firefox/nightly/38.3.0esr-candidates/build1/
Flags: needinfo?(q1)
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #27)
> (In reply to q1 from comment #26)
> > I retested this POC on 40.0b8 x64 on Win7 SP1 with 12 GB RAM, by extracting
> > the files from the attachment (in case I made some mistake posting them). It
> > crashes as expected. I also tried it on 40.0 x64 debug (my build with a few
> > trivial unrelated changes) and it crashes that version in the same way. If
> > it's a clue, my about:support says
> > 
> >    GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
> >    WebGL Renderer: Google Inc. -- ANGLE (NVIDIA GeForce GT 610 Direct3D11
> > vs_5_0 ps_5_0)
> 
> The fix should be in Firefox 41 and newer. Can you try and see if it still
> crashes with:
> - Firefox 41 RC -
> http://ftp.mozilla.org/pub/firefox/nightly/41.0-candidates/build1/
> - ESR 38.3.0 -
> http://ftp.mozilla.org/pub/firefox/nightly/38.3.0esr-candidates/build1/

I would test it, but neither of those builds have Win64 versions as far as I can tell. The POC definitely won't crash on Win32 (though it might be possible to produce one that would). BTW, why aren't there Win64 builds?
Flags: needinfo?(q1)
I see 41.0.b9 build 1 Win64, so I'll try that.
(In reply to q1 from comment #29)
> I see 41.0.b9 build 1 Win64, so I'll try that.

It doesn't crash.
(In reply to q1 from comment #30)
> (In reply to q1 from comment #29)
> > I see 41.0.b9 build 1 Win64, so I'll try that.
> 
> It doesn't crash.

I forgot that we have 64 bit builds only in Beta... the fix should be in Beta 9, and since it no longer crashes I'll mark this as Verified on 41.
Flags: qe-verify+
Alias: CVE-2015-7179
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.