Last Comment Bug 825721 - (CVE-2013-0800) OOB Write in pixman_fill_sse2
(CVE-2013-0800)
: OOB Write in pixman_fill_sse2
Status: RESOLVED FIXED
[adv-main20+][adv-esr1705+]
: crash, sec-high, testcase
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 All
: -- normal (vote)
: mozilla22
Assigned To: Milan Sreckovic [:milan]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-31 19:49 PST by Abhishek Arya
Modified: 2014-07-24 14:38 PDT (History)
16 users (show)
dveditz: sec‑bounty+
dveditz: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
fixed
+
fixed
+
fixed
20+
verified
unaffected


Attachments
Testcase (559 bytes, image/svg+xml)
2012-12-31 19:49 PST, Abhishek Arya
no flags Details
simpler testcase (217 bytes, image/svg+xml)
2013-02-13 17:22 PST, Jesse Ruderman
no flags Details
Clamp negative boundaries and disallow negative sizes for pixman boxes. (2.46 KB, patch)
2013-02-19 11:50 PST, Milan Sreckovic [:milan]
jmuizelaar: review+
Details | Diff | Review
Crashing Cairo unit test. (833 bytes, text/plain)
2013-02-26 13:50 PST, Milan Sreckovic [:milan]
no flags Details
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version. (7.94 KB, patch)
2013-02-28 11:15 PST, Milan Sreckovic [:milan]
bgirard: feedback+
Details | Diff | Review
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version. (7.91 KB, patch)
2013-03-07 07:15 PST, Milan Sreckovic [:milan]
jmuizelaar: review+
Details | Diff | Review
Extracted the Cairo only patch for the beta. (2.51 KB, patch)
2013-03-25 18:15 PDT, Milan Sreckovic [:milan]
milan: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr17+
Details | Diff | Review

Description Abhishek Arya 2012-12-31 19:49:05 PST
Created attachment 696848 [details]
Testcase

>==10500== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f72101bb970 at pc 0x7f724f461205 bp 0x7fffc65c2270 sp 0x7fffc65c2268
>WRITE of size 16 at 0x7f72101bb970 thread T0
>    #0 0x7f724f461204 in pixman_fill_sse2 src/gfx/cairo/libpixman/src/pixman-sse2.c:362
>    #1 0x7f724f4547fb in sse2_fill src/gfx/cairo/libpixman/src/pixman-sse2.c:5904
>    #2 0x7f724f28d406 in _pixman_implementation_fill src/gfx/cairo/libpixman/src/pixman-implementation.c:182
>    #3 0x7f724f28ab33 in delegate_fill src/gfx/cairo/libpixman/src/pixman-implementation.c:62
>    #4 0x7f724f28d406 in _pixman_implementation_fill src/gfx/cairo/libpixman/src/pixman-implementation.c:182
>    #5 0x7f724eeef8db in _moz_pixman_fill src/gfx/cairo/libpixman/src/pixman.c:773
>    #6 0x7f724d8d6e93 in _composite_boxes src/gfx/cairo/cairo/src/cairo-image-surface.c:2940
>    #7 0x7f724d8c48fe in _clip_and_composite_boxes src/gfx/cairo/cairo/src/cairo-image-surface.c:3034
>    #8 0x7f724d8c7dd8 in _clip_and_composite_trapezoids src/gfx/cairo/cairo/src/cairo-image-surface.c:3190
>    #9 0x7f724d8c66e8 in _clip_and_composite_polygon src/gfx/cairo/cairo/src/cairo-image-surface.c:3567
>    #10 0x7f724d895820 in _cairo_image_surface_fill src/gfx/cairo/cairo/src/cairo-image-surface.c:3758
>    #11 0x7f724da967ff in _cairo_surface_fill src/gfx/cairo/cairo/src/cairo-surface.c:2348
>    #12 0x7f724d857d9f in _cairo_gstate_fill src/gfx/cairo/cairo/src/cairo-gstate.c:1295
>    #13 0x7f724d77f6a6 in _moz_cairo_fill_preserve src/gfx/cairo/cairo/src/cairo.c:2459
>    #14 0x7f724ca6d497 in gfxContext::Fill() src/gfx/thebes/gfxContext.cpp:303
>    #15 0x7f7245a3678e in nsSVGPathGeometryFrame::Render(nsRenderingContext*) src/layout/svg/nsSVGPathGeometryFrame.cpp:594
>    #16 0x7f7245a35103 in nsSVGPathGeometryFrame::PaintSVG(nsRenderingContext*, nsIntRect const*) src/layout/svg/nsSVGPathGeometryFrame.cpp:188
>    #17 0x7f7245a37e1f in non-virtual thunk to nsSVGPathGeometryFrame::PaintSVG(nsRenderingContext*, nsIntRect const*) src/layout/svg/nsSVGPathGeometryFrame.cpp:224
>    #18 0x7f7245a8819a in nsSVGUtils::PaintFrameWithEffects(nsRenderingContext*, nsIntRect const*, nsIFrame*) src/layout/svg/nsSVGUtils.cpp:922
>    #19 0x7f7245a156e3 in nsSVGMaskFrame::ComputeMaskAlpha(nsRenderingContext*, nsIFrame*, gfxMatrix const&, float) src/layout/svg/nsSVGMaskFrame.cpp:109
>    #20 0x7f7245a04bce in nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext*, nsIFrame*, nsRect const&, nsDisplayListBuilder*, mozilla::layers::LayerManager*) src/layout/svg/nsSVGIntegrationUtils.cpp:517
>    #21 0x7f723f09efc1 in nsDisplaySVGEffects::PaintAsLayer(nsDisplayListBuilder*, nsRenderingContext*, mozilla::layers::LayerManager*) src/layout/base/nsDisplayList.cpp:4364
>    #22 0x7f723ed3ae36 in mozilla::PaintInactiveLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsDisplayItem*, gfxContext*, nsRenderingContext*) src/layout/base/FrameLayerBuilder.cpp:1977
>    #23 0x7f723ed35b81 in mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*) src/layout/base/FrameLayerBuilder.cpp:3320
>    #24 0x7f724ce18600 in mozilla::layers::BasicThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*) src/gfx/layers/basic/BasicThebesLayer.h:94
>    #25 0x7f724ce17caf in mozilla::layers::BasicShadowableThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*) src/gfx/layers/basic/BasicThebesLayer.cpp:400
>    #26 0x7f724ce107f3 in mozilla::layers::BasicThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicThebesLayer.cpp:187
>    #27 0x7f724ce152eb in mozilla::layers::BasicShadowableThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicThebesLayer.cpp:301
>    #28 0x7f724ce15ce4 in non-virtual thunk to mozilla::layers::BasicShadowableThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicThebesLayer.cpp:312
>    #29 0x7f724cd9ddd6 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintContext&, gfxContext*) src/gfx/layers/basic/BasicLayerManager.cpp:819
>    #30 0x7f724cd9b0da in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicLayerManager.cpp:934
>    #31 0x7f724cd9e3b6 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintContext&, gfxContext*) src/gfx/layers/basic/BasicLayerManager.cpp:835
>    #32 0x7f724cd9b0da in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicLayerManager.cpp:934
>    #33 0x7f724cd9e3b6 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintContext&, gfxContext*) src/gfx/layers/basic/BasicLayerManager.cpp:835
>    #34 0x7f724cd9b0da in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) src/gfx/layers/basic/BasicLayerManager.cpp:934
>    #35 0x7f724cd964d2 in mozilla::layers::BasicLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/basic/BasicLayerManager.cpp:590
>    #36 0x7f724cd94c3c in mozilla::layers::BasicLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/basic/BasicLayerManager.cpp:509
>    #37 0x7f724cda88f4 in mozilla::layers::BasicShadowLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) src/gfx/layers/basic/BasicLayerManager.cpp:1144
>    #38 0x7f723f03fc9d in nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) const src/layout/base/nsDisplayList.cpp:1164
>    #39 0x7f723f03cdec in nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) const src/layout/base/nsDisplayList.cpp:1026
>    #40 0x7f723f1e7166 in nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) src/layout/base/nsLayoutUtils.cpp:1976
>    #41 0x7f723f340d61 in PresShell::Paint(nsIView*, nsRegion const&, unsigned int) src/layout/base/nsPresShell.cpp:5358
>    #42 0x7f72434dfdbb in nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) src/view/src/nsViewManager.cpp:428
>    #43 0x7f72434f36e0 in nsViewManager::ProcessPendingUpdates() src/view/src/nsViewManager.cpp:1216
>    #44 0x7f723f3c627e in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:947
>    #45 0x7f723f3dc63b in mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver*, long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:164
>    #46 0x7f723f3dbc52 in mozilla::RefreshDriverTimer::Tick() src/layout/base/nsRefreshDriver.cpp:156
>    #47 0x7f723f3db0fb in mozilla::RefreshDriverTimer::TimerTick(nsITimer*, void*) src/layout/base/nsRefreshDriver.cpp:181
>    #48 0x7f724c4f615b in nsTimerImpl::Fire() src/xpcom/threads/nsTimerImpl.cpp:482
>    #49 0x7f724c4f75e4 in nsTimerEvent::Run() src/xpcom/threads/nsTimerImpl.cpp:565
>    #50 0x7f724c4b99cf in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
>    #51 0x7f724c12e355 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:237
>    #52 0x7f7249d2ad7c in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
>    #53 0x7f724c7aafe2 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:215
>    #54 0x7f724c7aae19 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:208
>    #55 0x7f724c7aacee in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
>    #56 0x7f724911e947 in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
>    #57 0x7f7247c4c3b5 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:288
>    #58 0x7f723cf3ec14 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3823
>    #59 0x7f723cf447fa in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
>    #60 0x7f723cf475c0 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4088
>    #61 0x41d8d6 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
>    #62 0x41b0f0 in main src/browser/app/nsBrowserApp.cpp:279
>    #63 0x7f725f01176c in
>0x7f72101bb970 is located 10384 bytes to the left of 65536-byte region [0x7f72101be200,0x7f72101ce200)
>freed by thread T8 (JS Sour~ Thread) here:
>    #0 0x40f942 in __interceptor_free
>    #1 0x7f7254c34cc9 in js_free(void*) src/./../../dist/include/js/Utility.h:165
>    #2 0x7f7254c32832 in zlib_free(void*, void*) src/js/src/jsutil.cpp:42
>    #3 0x7f724d184285 in MOZ_Z_deflateEnd src/modules/zlib/src/deflate.c:996
>    #4 0x7f7254c32967 in js::Compressor::~Compressor() src/js/src/jsutil.cpp:63
>    #5 0x7f72546bd1e0 in js::SourceCompressorThread::internalCompress() src/js/src/jsscript.cpp:1003
>    #6 0x7f72546bb127 in js::SourceCompressorThread::threadLoop() src/js/src/jsscript.cpp:1030
>    #7 0x7f72546bae52 in js::SourceCompressorThread::compressorThread(void*) src/js/src/jsscript.cpp:905
>    #8 0x7f725e590a59 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:156
>    #9 0x41485a in __asan::AsanThread::ThreadStart()
>previously allocated by thread T8 (JS Sour~ Thread) here:
>    #0 0x40fa22 in malloc
>    #1 0x7f7254c34de3 in js_malloc(unsigned long) src/./../../dist/include/js/Utility.h:148
>    #2 0x7f7254c326a6 in zlib_alloc(void*, unsigned int, unsigned int) src/js/src/jsutil.cpp:36
>    #3 0x7f724d182cd7 in MOZ_Z_deflateInit2_ src/modules/zlib/src/deflate.c:301
>    #4 0x7f724d1803f7 in MOZ_Z_deflateInit_ src/modules/zlib/src/deflate.c:207
>    #5 0x7f7254c32c02 in js::Compressor::init() src/js/src/jsutil.cpp:76
>    #6 0x7f72546bc6e0 in js::SourceCompressorThread::internalCompress() src/js/src/jsscript.cpp:971
>    #7 0x7f72546bb127 in js::SourceCompressorThread::threadLoop() src/js/src/jsscript.cpp:1030
>    #8 0x7f72546bae52 in js::SourceCompressorThread::compressorThread(void*) src/js/src/jsscript.cpp:905
>    #9 0x7f725e590a59 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:156
>    #10 0x41485a in __asan::AsanThread::ThreadStart()
>Thread T8 (JS Sour~ Thread) created by T0 here:
>    #0 0x40d1f4 in __interceptor_pthread_create
>    #1 0x7f725e581d55 in _PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:393
>    #2 0x7f725e57fb46 in PR_CreateThread src/nsprpub/pr/src/pthreads/ptthread.c:476
>    #3 0x7f72546bb78a in js::SourceCompressorThread::init() src/js/src/jsscript.cpp:921
>    #4 0x7f72536883e6 in JSRuntime::init(unsigned int) src/js/src/jsapi.cpp:960
>    #5 0x7f725368c5da in JS_NewRuntime(unsigned int, JSUseHelperThreads) src/js/src/jsapi.cpp:1131
>    #6 0x7f7246bcef85 in XPCJSRuntime::XPCJSRuntime(nsXPConnect*) src/js/xpconnect/src/XPCJSRuntime.cpp:2451
>    #7 0x7f7246bd3774 in XPCJSRuntime::newXPCJSRuntime(nsXPConnect*) src/js/xpconnect/src/XPCJSRuntime.cpp:2546
>    #8 0x7f72468d257d in nsXPConnect::nsXPConnect() src/js/xpconnect/src/nsXPConnect.cpp:85
>    #9 0x7f72468d4801 in nsXPConnect::GetXPConnect() src/js/xpconnect/src/nsXPConnect.cpp:145
>    #10 0x7f7240a6000c in nsContentUtils::Init() src/content/base/src/nsContentUtils.cpp:340
>    #11 0x7f723ed03b81 in nsLayoutStatics::Initialize() src/layout/build/nsLayoutStatics.cpp:152
>    #12 0x7f723ecac11e in Initialize() src/layout/build/nsLayoutModule.cpp:389
>    #13 0x7f724c4545be in nsComponentManagerImpl::KnownModule::Load() src/xpcom/components/nsComponentManager.cpp:708
>    #14 0x7f724c458d45 in nsFactoryEntry::GetFactory() src/xpcom/components/nsComponentManager.cpp:1749
>    #15 0x7f724c45c032 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1031
>    #16 0x7f724c442b65 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1427
>    #17 0x7f724c43f812 in nsGetServiceFromCategory::operator()(nsID const&, void**) const src/xpcom/components/nsComponentManager.cpp:166
>    #18 0x7f724c0e7b72 in nsCOMPtr_base::assign_from_helper(nsCOMPtr_helper const&, nsID const&) src/objdir-ff-asan-sym/xpcom/build/nsCOMPtr.cpp:110
>    #19 0x7f724c460c9f in nsCOMPtr<mozilla::ModuleLoader>::operator=(nsCOMPtr_helper const&) src/../../dist/include/nsCOMPtr.h:691
>    #20 0x7f724c453cc4 in nsComponentManagerImpl::LoaderForExtension(nsACString_internal const&) src/xpcom/components/nsComponentManager.cpp:1455
>    #21 0x7f724c4532e0 in nsComponentManagerImpl::KnownModule::EnsureLoader() src/xpcom/components/nsComponentManager.cpp:685
>    #22 0x7f724c454205 in nsComponentManagerImpl::KnownModule::Load() src/xpcom/components/nsComponentManager.cpp:696
>    #23 0x7f724c458d45 in nsFactoryEntry::GetFactory() src/xpcom/components/nsComponentManager.cpp:1749
>    #24 0x7f724c45c032 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1031
>    #25 0x7f724c442b65 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1427
>    #26 0x7f724c0f7e5d in CallGetService(char const*, nsID const&, void**) src/objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:60
>    #27 0x7f724c0fc219 in nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const src/objdir-ff-asan-sym/xpcom/build/nsComponentManagerUtils.cpp:256
>    #28 0x7f724c0e77d7 in nsCOMPtr_base::assign_from_gs_contractid_with_error(nsGetServiceByContractIDWithError const&, nsID const&) src/objdir-ff-asan-sym/xpcom/build/nsCOMPtr.cpp:101
>    #29 0x7f724795ccaf in nsCOMPtr<nsISupports>::operator=(nsGetServiceByContractIDWithError const&) src/../../../../dist/include/nsCOMPtr.h:1003
>    #30 0x7f724795c463 in nsAppStartupNotifier::Observe(nsISupports*, char const*, unsigned short const*) src/embedding/components/appstartup/src/nsAppStartupNotifier.cpp:62
>    #31 0x7f723cf3c303 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3658
>    #32 0x7f723cf447fa in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3890
>    #33 0x7f723cf475c0 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4088
>    #34 0x41d8d6 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
>    #35 0x41b0f0 in main src/browser/app/nsBrowserApp.cpp:279
>    #36 0x7f725f01176c in
>Shadow bytes around the buggy address:
>  0x1fee420376d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee420376e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee420376f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>=>0x1fee42037720: fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]fa
>  0x1fee42037730: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037740: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x1fee42037770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>Shadow byte legend (one shadow byte represents 8 application bytes):
>  Addressable:           00
>  Partially addressable: 01 02 03 04 05 06 07
>  Heap left redzone:     fa
>  Heap righ redzone:     fb
>  Freed Heap region:     fd
>  Stack left redzone:    f1
>  Stack mid redzone:     f2
>  Stack right redzone:   f3
>  Stack partial redzone: f4
>  Stack after return:    f5
>  Stack use after scope: f8
>  Global redzone:        f9
>  Global init order:     f6
>  Poisoned by user:      f7
>  ASan internal:         fe
>Stats: 240M malloced (451M for red zones) by 317154 calls
>Stats: 46M realloced by 16904 calls
>Stats: 216M freed by 213235 calls
>Stats: 101M really freed by 144972 calls
>Stats: 556M (556M-0M) mmaped; 139 maps, 0 unmaps
>  mmaps   by size class: 10:208845; 11:12282; 12:2048; 13:1536; 14:1280; 15:384; 16:960; 17:1280; 18:48; 19:40; 20:24;
>  mallocs by size class: 10:289278; 11:19039; 12:2422; 13:1691; 14:1481; 15:401; 16:1349; 17:1358; 18:73; 19:40; 20:22;
>  frees   by size class: 10:189445; 11:16471; 12:1491; 13:1492; 14:1332; 15:287; 16:1259; 17:1342; 18:59; 19:38; 20:19;
>  rfrees  by size class: 10:131090; 11:10181; 12:729; 13:671; 14:710; 15:182; 16:820; 17:558; 18:26; 19:4; 20:1;
>Stats: malloc large: 1493 small slow: 3726
>Stats: StackDepot: 0 ids; 0M mapped
>==10500== ABORTING
>
Comment 1 Milan Sreckovic [:milan] 2013-01-03 08:46:41 PST
cairo boxes arrive with -1 for x1 or y1, and there is a strong assumption in the code that this will never happen.
Comment 2 Milan Sreckovic [:milan] 2013-01-03 14:23:04 PST
Bentley-Ottmann in Cairo comes back with INT32_MIN (not -1) in the line segments, and those eventually pollute the traps, which then pollute the regions.
Comment 3 Milan Sreckovic [:milan] 2013-01-04 07:07:59 PST
At least sec-high; chances are you will always overflow and not have a chance to inject your own data.
Comment 4 Daniel Veditz [:dveditz] 2013-01-07 17:13:34 PST
Could this testcase help us squash the elusive bug 549676?
Comment 5 Milan Sreckovic [:milan] 2013-01-08 06:51:54 PST
(In reply to Daniel Veditz [:dveditz] from comment #4)
> Could this testcase help us squash the elusive bug 549676?

Possibly.  I'm tracing through this, but it's a bit of a pain with the cairo switching between fixed and regular representations.  Stopping the crash is "easy enough", I'm trying to find the problem in the computational geometry.
Comment 6 Milan Sreckovic [:milan] 2013-01-09 13:06:14 PST
We're getting in trouble with negative bounds in Cairo - the funny part is that elsewhere (e.g., DrawTargetCG::Init), we assumes that Cairo can only deal with 0 < w/h < 32768, but in this case we get stuck with 5e18.
Comment 7 Milan Sreckovic [:milan] 2013-01-10 10:30:16 PST
The crash is in fast path only.
Comment 8 Milan Sreckovic [:milan] 2013-01-21 14:00:53 PST
If shape-rendering is set to optimizeSpeed or crispEdges, the crash is there.  If set to auto or geometricPrecision, no crash.
Comment 9 Milan Sreckovic [:milan] 2013-01-21 14:14:20 PST
(In reply to Milan Sreckovic [:milan] from comment #8)
> If shape-rendering is set to optimizeSpeed or crispEdges, the crash is
> there.  If set to auto or geometricPrecision, no crash.

Equivalently, relevant to this crash, we only crash with cairo_set_antialias(CAIRO_ANTIALIAS_NONE).  CAIRO_ANTIALIAS_DEFAULT is fine.
Comment 10 Milan Sreckovic [:milan] 2013-01-29 09:45:09 PST
Jeff, here's a question; do we patch this with a simple x1 = (x1 < 0) ? 0 : x1; etc, making sure that we don't have this kind of OOB write, then open a bug to find the cause (at which point it wouldn't be security, and wouldn't have the six week clock on it), or keep digging until we have a cause, because the bug isn't tracked against a release?
Comment 11 Milan Sreckovic [:milan] 2013-01-30 09:49:29 PST
Some random information.  This happens when the radius of the circle used as the mask is very large, but not too large. With some quick testing, r="5761126194342788607" does not crash, but one larger, r="5761126194342788608" does.  If you keep increasing the radius, the last one that crashes (in this range, at least), is r="5761126744098603520px", and one larger doesn't crash again r="5761126744098603521px".  The polygonization of the circle has an issue with the inclusive range [5761126744098603520px, 5761126744098603520px].
Comment 12 Milan Sreckovic [:milan] 2013-01-30 11:43:24 PST
(In reply to Milan Sreckovic [:milan] from comment #11)
> ...inclusive range
> [5761126744098603520px, 5761126744098603520px].

Should be [5761126194342788608px,5761126744098603520px].  Not that it is significant, but this is where a unit test would come in handy.
Comment 13 Jesse Ruderman 2013-02-13 17:22:53 PST
Created attachment 713731 [details]
simpler testcase
Comment 14 Jesse Ruderman 2013-02-13 17:31:22 PST
What's the significance of that tiny range of huge integers?  Did the fuzzer just get lucky?  It's not a power of two...
Comment 15 Milan Sreckovic [:milan] 2013-02-19 11:50:41 PST
Created attachment 715614 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes.

If we give some nasty data to cairo, in some cases we get back negative box boundaries. It's deterministic, though some nasty data is handled just fine (see Comment 12).  This is a quick fix to make it safe; we can chase down the cause (likely in Bentley-Ottmann code) in a separate bug if required.  Or wait for Cairo to fix it.
Comment 16 Milan Sreckovic [:milan] 2013-02-26 13:50:57 PST
Created attachment 718640 [details]
Crashing Cairo unit test.

A few cairo calls, with some nasty arguments, and we crash.
Comment 17 Milan Sreckovic [:milan] 2013-02-26 13:56:34 PST
Adding BenWa because this now has a simple GTest cairo unit test with the same crash.  Very cool addition to our automated testing :-)
Comment 18 Milan Sreckovic [:milan] 2013-02-27 07:29:27 PST
David, is it inappropriate to file a cairo bug with the cairo specific steps to reproduce, without Firefox mentioned, before we fix the problem on our side?  Or would that be considered a security breach?
Comment 19 David Bolter [:davidb] 2013-02-27 11:10:09 PST
(In reply to Milan Sreckovic [:milan] from comment #18)
> David, is it inappropriate to file a cairo bug with the cairo specific steps
> to reproduce, without Firefox mentioned, before we fix the problem on our
> side?  Or would that be considered a security breach?

Deferring that one to Dan.
Comment 20 Jeff Muizelaar [:jrmuizel] 2013-02-27 13:19:50 PST
Comment on attachment 715614 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes.

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

I wanted to look at this in more detail but I think it's better to just take it now. This we'll need a patch in gfx/cairo/ as well and an addition to the README
Comment 21 Milan Sreckovic [:milan] 2013-02-28 11:15:20 PST
Created attachment 719597 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.

Jeff, the source code for cairo-image-surface.c is the same as you reviewed, but I wanted to check if the patch in gfx/cairo and addition to README is what you had in mind.
Comment 22 Milan Sreckovic [:milan] 2013-03-06 09:31:44 PST
Comment on attachment 719597 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.

Feedback on BenWa, he wanted to see a GTest addition :-)
Comment 23 Benoit Girard (:BenWa) 2013-03-06 10:10:03 PST
Comment on attachment 719597 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.

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

f+ on GTest

::: gfx/2d/unittest/TestCairo.cpp
@@ +9,5 @@
> +namespace mozilla {
> +namespace layers {
> +
> +void TryCircle(double centerX, double centerY, double radius) {
> +	printf("TestCairo:TryArcs centerY %f, radius %f\n",centerY,radius);

2 space instead of tabs

@@ +20,5 @@
> +
> +  cairo_set_antialias(cairo, CAIRO_ANTIALIAS_NONE);
> +  cairo_arc(cairo, 0.0, centerY, radius, 0.0, 6.2831853071795862);
> +  cairo_fill_preserve(cairo);
> +  ASSERT_TRUE(1); // enough that we didn't crash

I don't think this assert is needed. Returning from a test is considered a pass.

@@ +42,5 @@
> +
> +	// OK:
> +	TryCircle(1.0, 0.0, 5761126469220696064.0);
> +
> +	// This was the crash in 825721.  Not that centerY has to be non-zero,

s/Not/Note
Comment 24 Milan Sreckovic [:milan] 2013-03-07 07:15:55 PST
Created attachment 722244 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.

 Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version. Addressed BenWa's f+ comments.
Comment 25 Ryan VanderMeulen [:RyanVM] 2013-03-20 12:43:03 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b00533cf4ce
Comment 26 Ed Morley [:emorley] 2013-03-21 06:16:17 PDT
https://hg.mozilla.org/mozilla-central/rev/1b00533cf4ce
Comment 28 Daniel Veditz [:dveditz] 2013-03-25 15:18:52 PDT
Assuming this doesn't affect b2g18 because no sse chip, but that it affects everything else.

Since this landed with a testcase we now have to scramble to get this into all our upcoming releases, which probably means no beta testing whatsoever of the patch at this point. This is the situation the Security-Approval process was intended to prevent.

https://wiki.mozilla.org/Security/Bug_Approval_Process

Please request approval on the patch (retroactively) and answer the security questions there. That will help us decide how badly we're affected and whether we must land immediately everywhere or can skip some of the branches.

Answering comment 18: a cairo bug would be OK, but if they had a security-bug process that would be nice to use. we can reach the cairo folks out of band though to explain the importance of certain bugs if necessary.
Comment 29 Ed Morley [:emorley] 2013-03-25 15:38:56 PDT
I believe we should just prohibit the use of checkin-needed for security bugs full stop. Ryan please can you make sure that they are excluded from your saved searches, to ensure this doesn't happen in the future? :-)
Comment 30 Milan Sreckovic [:milan] 2013-03-25 15:49:54 PDT
(In reply to Ed Morley [:edmorley UTC+0] from comment #29)
> I believe we should just prohibit the use of checkin-needed for security
> bugs full stop. Ryan please can you make sure that they are excluded from
> your saved searches, to ensure this doesn't happen in the future? :-)

There goes my way to check in patches :-)
Comment 31 Ed Morley [:emorley] 2013-03-25 15:53:58 PDT
The security team could handle them if needed - I just think we shouldn't be inserting sheriffs into the workflow to avoid confusion :-)
Comment 32 Milan Sreckovic [:milan] 2013-03-25 15:57:05 PDT
Comment on attachment 722244 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.

[Security approval request comment] - based on comment 28
How easily could an exploit be constructed based on the patch?
We check the invalid parameters just before the call, so we do give away the information as to where it happens. If there is a way to exploit this, this probably gives a good hint where to start.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Very easily based on the unit test that has been added.  It actually gives you the parameters to set on SVG to cause this crash.  In other words, it is equivalent to having this whole bug being public.

Which older supported branches are affected by this flaw?
This is a bug in Cairo, so it goes back a long way.

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The code patch, without the unit tests, should port cleanly.

How likely is this patch to cause regressions; how much testing does it need?
Not likely to cause a regression, we are taking out the scenario where we try to allocate memory using <0 number.
Comment 33 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-25 17:23:37 PDT
Comment on attachment 722244 [details] [diff] [review]
Clamp negative boundaries and disallow negative sizes for pixman boxes. Add a unit test, update Cairo README and add a patch file for Cairo directory. Same source code as the reviewed version.



[Triage Comment]
Since this has already been landed to central it's too late for a sec-approval request, we need to get this on branches and we're about to build our final FF20 beta so pre-approving the patch based on the presumed ease of backporting referenced in the sec-approval form.
Comment 34 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-25 17:28:09 PDT
My bad, I missed Dan's request for a retroactive sec-approval request. Based on the potential for exploit and the low risk of this patch I'm still certain we can take this in our final beta and Milan has agreed to do the landing asap.
Comment 35 Ryan VanderMeulen [:RyanVM] 2013-03-25 18:10:36 PDT
(In reply to Ed Morley [:edmorley UTC+0] from comment #31)
> The security team could handle them if needed - I just think we shouldn't be
> inserting sheriffs into the workflow to avoid confusion :-)

How about whoever does the sec-approval+ sets checkin? at the same time? Seems like a decent compromise to me. (But ultimately, I can accept some responsibility for this landing as I typically do take note of the sec rating before landing but apparently didn't do so in this case).
Comment 36 Milan Sreckovic [:milan] 2013-03-25 18:15:49 PDT
Created attachment 729347 [details] [diff] [review]
Extracted the Cairo only patch for the beta.

Same as 722244 patch, but only extracted the minimal amount that fixes the bug, skipping the unit tests and the rest.
Comment 37 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-25 18:20:45 PDT
Comment on attachment 729347 [details] [diff] [review]
Extracted the Cairo only patch for the beta.

[Triage Comment]
Comment 38 Nick Thomas [:nthomas] 2013-03-25 18:24:46 PDT
Comment on attachment 729347 [details] [diff] [review]
Extracted the Cairo only patch for the beta.

https://hg.mozilla.org/releases/mozilla-beta/rev/f47fc0d38ede
Comment 39 Ryan VanderMeulen [:RyanVM] 2013-03-26 08:16:11 PDT
(In reply to lsblakk@mozilla.com from comment #37)
> Comment on attachment 729347 [details] [diff] [review]
> Extracted the Cairo only patch for the beta.
> 
> [Triage Comment]

I'm assuming this is the one we want to land on esr17. What about Aurora?
Comment 40 Milan Sreckovic [:milan] 2013-03-26 09:56:58 PDT
The same patch as for beta applies to all other non-trunk versions, so if we're putting it into Aurora, it should be the same patch as for beta.
Comment 41 Al Billings [:abillings] 2013-03-26 10:49:06 PDT
(In reply to Ryan VanderMeulen [:RyanVM] from comment #35)
> (In reply to Ed Morley [:edmorley UTC+0] from comment #31)
> > The security team could handle them if needed - I just think we shouldn't be
> > inserting sheriffs into the workflow to avoid confusion :-)
> 
> How about whoever does the sec-approval+ sets checkin? at the same time?
> Seems like a decent compromise to me. (But ultimately, I can accept some
> responsibility for this landing as I typically do take note of the sec
> rating before landing but apparently didn't do so in this case).

Generally, I do sec-approval (or Dan does if I don't get to it). I have no idea about your checkin flags or process there so someone would need to point me to something. I'm a program manager, not a developer.
Comment 42 Ryan VanderMeulen [:RyanVM] 2013-03-26 10:52:57 PDT
Yeah, forget that. I'm just not going to land s-s bugs anymore unless they have sec-approval+ on them.
Comment 43 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-26 11:02:13 PDT
Comment on attachment 729347 [details] [diff] [review]
Extracted the Cairo only patch for the beta.



[Triage Comment]

Moving flags to the patch without tests for uplift to branches.
Comment 44 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-26 11:05:29 PDT
Flagging checkin-needed so this can be landed to ESR17 branch before Thursday's go to build on the ESR that will ship alongside FF20 and to Aurora branch as well (milan doesn't have perms to push to those branches).
Comment 45 Al Billings [:abillings] 2013-03-26 11:28:58 PDT
Marking as "19 affected" since this affects ESR.
Comment 47 Matt Wobensmith [:mwobensmith][:matt:] 2013-03-28 11:45:13 PDT
Confirmed crash with 17.0.4esr (non-ASan)
Confirmed fixed with 17.0.5esr 2013-03-28 (custom local ASan build, changeset cd7d672b40ea)
Comment 48 Mike Hommey [:glandium] 2013-04-02 12:10:56 PDT
Was this ever reported to cairo upstream?
Comment 49 Milan Sreckovic [:milan] 2013-04-02 12:23:05 PDT
I haven't yet.  Just joined the e-mail list.  Will do.
Comment 50 Milan Sreckovic [:milan] 2013-04-02 12:36:02 PDT
Cairo bug https://bugs.freedesktop.org/show_bug.cgi?id=63045

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