Closed Bug 825721 (CVE-2013-0800) Opened 12 years ago Closed 12 years ago

OOB Write in pixman_fill_sse2

Categories

(Core :: Graphics, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox19 --- wontfix
firefox20 + fixed
firefox21 + fixed
firefox22 + fixed
firefox-esr17 20+ verified
b2g18 --- unaffected

People

(Reporter: inferno, Assigned: milan)

Details

(4 keywords, Whiteboard: [adv-main20+][adv-esr1705+])

Attachments

(4 files, 3 obsolete files)

Attached image Testcase (obsolete) —
>==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 >
Component: General → Graphics
Product: Firefox → Core
cairo boxes arrive with -1 for x1 or y1, and there is a strong assumption in the code that this will never happen.
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.
At least sec-high; chances are you will always overflow and not have a chance to inject your own data.
Keywords: sec-high
Keywords: crash, testcase
Could this testcase help us squash the elusive bug 549676?
Flags: sec-bounty?
(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.
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.
Assignee: nobody → milan
The crash is in fast path only.
If shape-rendering is set to optimizeSpeed or crispEdges, the crash is there. If set to auto or geometricPrecision, no crash.
(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.
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?
Flags: needinfo?(jmuizelaar)
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].
(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.
Attached image simpler testcase
Attachment #696848 - Attachment is obsolete: true
What's the significance of that tiny range of huge integers? Did the fuzzer just get lucky? It's not a power of two...
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.
Attachment #715614 - Flags: review?(jmuizelaar)
A few cairo calls, with some nasty arguments, and we crash.
Adding BenWa because this now has a simple GTest cairo unit test with the same crash. Very cool addition to our automated testing :-)
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?
Flags: needinfo?(dbolter)
(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.
Flags: needinfo?(dbolter) → needinfo?(dveditz)
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
Attachment #715614 - Flags: review?(jmuizelaar) → review+
Flags: needinfo?(jmuizelaar)
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.
Attachment #715614 - Attachment is obsolete: true
Attachment #719597 - Flags: review?(jmuizelaar)
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 :-)
Attachment #719597 - Flags: feedback?(bgirard)
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
Attachment #719597 - Flags: feedback?(bgirard) → feedback+
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.
Attachment #719597 - Attachment is obsolete: true
Attachment #719597 - Flags: review?(jmuizelaar)
Attachment #722244 - Flags: review?(jmuizelaar)
Attachment #722244 - Flags: review?(jmuizelaar) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
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.
Flags: sec-bounty?
Flags: sec-bounty+
Flags: needinfo?(dveditz)
Flags: in-testsuite+
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? :-)
(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 :-)
The security team could handle them if needed - I just think we shouldn't be inserting sheriffs into the workflow to avoid confusion :-)
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.
Attachment #722244 - Flags: sec-approval?
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.
Attachment #722244 - Flags: sec-approval?
Attachment #722244 - Flags: approval-mozilla-esr17+
Attachment #722244 - Flags: approval-mozilla-beta+
Attachment #722244 - Flags: approval-mozilla-aurora+
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.
(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).
Same as 722244 patch, but only extracted the minimal amount that fixes the bug, skipping the unit tests and the rest.
Attachment #729347 - Flags: review+
Comment on attachment 729347 [details] [diff] [review] Extracted the Cairo only patch for the beta. [Triage Comment]
Attachment #729347 - Flags: approval-mozilla-beta+
(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?
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.
(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.
Yeah, forget that. I'm just not going to land s-s bugs anymore unless they have sec-approval+ on them.
Attachment #722244 - Flags: approval-mozilla-esr17+
Attachment #722244 - Flags: approval-mozilla-beta+
Attachment #722244 - Flags: approval-mozilla-aurora+
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.
Attachment #729347 - Flags: approval-mozilla-esr17+
Attachment #729347 - Flags: approval-mozilla-aurora+
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).
Keywords: checkin-needed
Marking as "19 affected" since this affects ESR.
Whiteboard: [adv-main20+]
Alias: CVE-2013-0800
Confirmed crash with 17.0.4esr (non-ASan) Confirmed fixed with 17.0.5esr 2013-03-28 (custom local ASan build, changeset cd7d672b40ea)
Whiteboard: [adv-main20+] → [adv-main20+][adv-esr1705+]
Was this ever reported to cairo upstream?
I haven't yet. Just joined the e-mail list. Will do.
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: