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

Overflow in VertexBufferInterface::reserveVertexSpace causes memory-safety bug

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Version:
39 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla43
Tracking Flags:
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, reporter-external, sec-critical, Whiteboard: [adv-main41+][adv-esr38.3+])

Attachments

(2 files, 1 obsolete file)

poc.js, poc.html, glMatrix-0.9.5.min.js. Extract and use as in note 0.
29.36 KB, text/plain
Details
overflow-check.patch
2.85 KB, patch
jrmuizel
: review+
dveditz
: approval-mozilla-aurora+
dveditz
: approval-mozilla-beta+
lizzard
: approval-mozilla-esr38+
dveditz
: sec-approval+
Details | Diff | Splinter Review
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
Attached patch overflow-check.patch — — Splinter Review
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.
status-firefox40: --- → ?
status-firefox41: --- → ?
status-firefox42: --- → ?
status-firefox43: --- → ?
tracking-firefox41: --- → ?
tracking-firefox42: --- → +
tracking-firefox43: --- → +
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).
status-firefox40: wontfix → affected
tracking-firefox40: --- → ?
tracking-firefox41: --- → +
tracking-firefox-esr38: --- → 41+
See Also: → http://code.google.com/p/chromium/issues/detail?id=518206
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: 9 years ago
status-firefox43: affected → fixed
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.
status-firefox41: fixed → verified
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.

Attachment

General

Creator:
Created:
Updated:
Size: