Closed Bug 1170390 (CVE-2015-4512) Opened 9 years ago Closed 9 years ago

AddressSanitizer READ of size 1364 gfx/2d/DataSurfaceHelpers.cpp:81

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Version:
42 Branch
Platform:
Unspecified
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox40 --- wontfix
firefox41 + fixed
firefox42 + fixed
firefox-esr38 - wontfix

People

(Reporter: rs, Assigned: lsalzman)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-bounds, sec-moderate, Whiteboard: [post-critsmash-triage][gfx-noted][adv-main41+] sec-high when using Cairo xlib 16-bit)

Attachments

(8 files, 1 obsolete file)

firefox-asan.log
61.91 KB, text/x-log
Details
part 1 - detect 16bpp cairo xlib surface format
1.79 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess
11.30 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
part 3 - add gfxASurface::GetSurfaceFormat for retrieving precise surface format where necessary
3.09 KB, patch
jrmuizel
: review+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
part 4 - use Optimal2DFormatForContent over OptimalFormatForContent to avoid enum conversions
5.23 KB, patch
jrmuizel
: review+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
upliftable to 41, part 1 - detect 16bpp cairo xlib surface format
2.18 KB, patch
lsalzman
: review+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
upliftable to 41, part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess
16.24 KB, patch
lsalzman
: review+
ritu
: approval-mozilla-beta+
Details | Diff | Splinter Review
cairo-esr38-v2.patch
28.79 KB, patch
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.81 Safari/537.36

Steps to reproduce:

Version: firefox-41.0a1.en-US.linux-x86_64-asan (latest Mozilla ASAN build)
OS: Ubuntu 15.04




Actual results:

  =================================================================
==25926==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f9b380986a7 at pc 0x7f9b5a205a94 bp 0x7fffa3c79bb0 sp 0x7fffa3c79ba8
READ of size 1364 at 0x7f9b380986a7 thread T0 (Web Content)
    #0 0x7f9b5a205a93 in mozilla::gfx::CopySurfaceDataToPackedArray(unsigned char*, unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, int, int) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:81
    #1 0x7f9b5a205cf3 in mozilla::gfx::SurfaceToPackedBGRA(mozilla::gfx::DataSourceSurface*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:132
    #2 0x7f9b5be43519 in mozilla::dom::CanvasRenderingContext2D::GetImageBuffer(unsigned char**, int*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1597
    #3 0x7f9b5c104cee in mozilla::dom::HTMLCanvasElement::ToBlob(JSContext*, mozilla::dom::FileCallback&, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/html/HTMLCanvasElement.cpp:594
    #4 0x7f9b5bb7f32b in mozilla::dom::HTMLCanvasElementBinding::toBlob(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./HTMLCanvasElementBinding.cpp:345
    #5 0x7f9b5bdbeb56 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2609
    #6 0x7f9b5f7f8ff2 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #7 0x7f9b5f7bbe82 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:720:16
    #8 0x7f9b5f7eac5e in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:2961
    #9 0x7f9b5f7d7fcb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #10 0x7f9b5f7bbfe5 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:747
    #11 0x7f9b5f785ae4 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:784
    #12 0x7f9b600720a2 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4392
    #13 0x7f9b5a788456 in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/base/nsFrameMessageManager.cpp:1253
    #14 0x7f9b5a7873cc in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/base/nsFrameMessageManager.cpp:1070
    #15 0x7f9b5caa5ead in mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/ipc/TabChild.cpp:2738
    #16 0x7f9b5caa5f4f in non-virtual thunk to mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/ipc/Unified_cpp_dom_ipc0.cpp:2742
    #17 0x7f9b599d6971 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/ipc/ipdl/./PBrowserChild.cpp:2547
    #18 0x7f9b59ad4f7a in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:5316
    #19 0x7f9b595965a0 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1279
    #20 0x7f9b59594e63 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1198
    #21 0x7f9b5958c63c in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1182
    #22 0x7f9b59513661 in MessageLoop::RunTask(Task*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:361
    #23 0x7f9b595140df in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:369
    #24 0x7f9b595146ba in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:456
    #25 0x7f9b5959c237 in mozilla::ipc::DoWorkRunnable::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessagePump.cpp:220
    #26 0x7f9b58dc392a in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/xpcom/threads/nsThread.cpp:846
    #27 0x7f9b58e424de in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #28 0x7f9b5959b8bc in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessagePump.cpp:127
    #29 0x7f9b5959c680 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessagePump.cpp:289
    #30 0x7f9b59513321 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #31 0x7f9b595131c8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #32 0x7f9b5cea8566 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/widget/nsBaseAppShell.cpp:165
    #33 0x7f9b5e4956a6 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:745
    #34 0x7f9b5959c518 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessagePump.cpp:259
    #35 0x7f9b59513321 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #36 0x7f9b595131c8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #37 0x7f9b5e494a90 in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:581
    #38 0x49c779 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:236
    #39 0x7f9b55dc8a3f in __libc_start_main /build/buildd/glibc-2.21/csu/libc-start.c:289
    #40 0x49ba3c in _start (/home/revskills/Browsers/firefox/plugin-container+0x49ba3c)

0x7f9b380986a7 is located 679 bytes to the right of 175104-byte region [0x7f9b3806d800,0x7f9b38098400)
allocated by thread T0 (Web Content) here:
    #0 0x484588 in __interceptor_posix_memalign /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:132
    #1 0x7f9b5a5f1855 in TryAllocAlignedBytes(unsigned long) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/thebes/gfxImageSurface.cpp:92
    #2 0x7f9b5a5f1425 in gfxImageSurface::AllocateAndInit(long, int, bool) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/thebes/gfxImageSurface.cpp:129
    #3 0x7f9b5a57b0ff in gfxPlatformGtk::CreateOffscreenSurface(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, gfxContentType) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/thebes/gfxPlatformGtk.cpp:140
    #4 0x7f9b5a5738d0 in gfxPlatform::CreateDrawTargetForBackend(mozilla::gfx::BackendType, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/thebes/gfxPlatform.cpp:1217
    #5 0x7f9b5a573b45 in gfxPlatform::CreateOffscreenCanvasDrawTarget(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/thebes/gfxPlatform.cpp:1233
    #6 0x7f9b5a367008 in mozilla::layers::LayerManager::CreateDrawTarget(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/layers/Layers.cpp:158
    #7 0x7f9b5be408a6 in mozilla::dom::CanvasRenderingContext2D::EnsureTarget(mozilla::dom::CanvasRenderingContext2D::RenderingMode) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1376
    #8 0x7f9b5be43d40 in mozilla::dom::CanvasRenderingContext2D::Save() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1648
    #9 0x7f9b5b4d4b90 in mozilla::dom::CanvasRenderingContext2DBinding::save(JSContext*, JS::Handle<JSObject*>, mozilla::dom::CanvasRenderingContext2D*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/bindings/./CanvasRenderingContext2DBinding.cpp:1810
    #10 0x7f9b5bdbeb56 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/bindings/BindingUtils.cpp:2609
    #11 0x7f9b5f7f8ff2 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jscntxtinlines.h:235
    #12 0x7f9b5f7bbe82 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:720:16
    #13 0x7f9b5f7eac5e in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:2961
    #14 0x7f9b5f7d7fcb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #15 0x7f9b5f7bbfe5 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:747
    #16 0x7f9b5f785ae4 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/vm/Interpreter.cpp:784
    #17 0x7f9b600720a2 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/js/src/jsapi.cpp:4392
    #18 0x7f9b5a788456 in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/base/nsFrameMessageManager.cpp:1253
    #19 0x7f9b5a7873cc in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/base/nsFrameMessageManager.cpp:1070
    #20 0x7f9b5caa5ead in mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/dom/ipc/TabChild.cpp:2738
    #21 0x7f9b5caa5f4f in non-virtual thunk to mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/dom/ipc/Unified_cpp_dom_ipc0.cpp:2742
    #22 0x7f9b599d6971 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/ipc/ipdl/./PBrowserChild.cpp:2547
    #23 0x7f9b59ad4f7a in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:5316
    #24 0x7f9b595965a0 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1279
    #25 0x7f9b59594e63 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1198
    #26 0x7f9b5958c63c in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/glue/MessageChannel.cpp:1182
    #27 0x7f9b59513661 in MessageLoop::RunTask(Task*) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:361
    #28 0x7f9b595140df in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:369
    #29 0x7f9b595146ba in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/ipc/chromium/src/base/message_loop.cc:456

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:81 mozilla::gfx::CopySurfaceDataToPackedArray(unsigned char*, unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, int, int)
Shadow bytes around the buggy address:
  0x0ff3e700b080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b090: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b0a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b0b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b0c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0ff3e700b0d0: fa fa fa fa[fa]fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b0e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b0f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ff3e700b120: 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 right 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
  Contiguous container OOB:fc
  ASan internal:           fe
==25926==ABORTING
> Steps to reproduce:
>
> Version: firefox-41.0a1.en-US.linux-x86_64-asan (latest Mozilla ASAN build)
> OS: Ubuntu 15.04

Between standard allocation for the image and the OBR is a huge gap of what went wrong. Do you have the testcase you were running that caused this?
Component: Untriaged → Canvas: 2D
Keywords: testcase-wanted
Product: Firefox → Core
Reporter told me on IRC that there was no repro case as of yet. This was during setup for some ASAN testing with one of our ASAN builds.
Attached file firefox-asan.log — 
Attached full console debug (same firefox debug ASAN build)
Whiteboard: [gfx-noted]
Not sure it's actionable without STR, but assigning to Lee anyway.
Assignee: nobody → lsalzman
(In reply to Milan Sreckovic [:milan] from comment #4)
> Not sure it's actionable without STR, but assigning to Lee anyway.

Inspecting the code, I can't see any obvious reason the overflow would occur. Stride is correct, size is correct, memory region is allocated to ensure it can fit stride * height, or would otherwise have failed earlier.

So, I think this would have to be caught in a debugger session (requiring being able to reliably reproduce) to root this one out...
Can you give us more info on how you found this? We need steps to reproduce. Thank you.
Flags: needinfo?(rs)
Doing some testing, I think the problem is related to this setup: 
vncserver :1 -geometry 1440x900 -depth 16 (by default)

I can reproduce the issue allways there, just browsing some file system files (eg: file:///). However I can't reproduce the same issue with: vncserver :1 -geometry 1440x900 -depth 24 or just Xvfb

Can anyone confirm this?
Flags: needinfo?(rs)
Flags: needinfo?(mwobensmith)
Francisco, were you going to provide us with more test files, etc.?
Flags: needinfo?(mwobensmith) → needinfo?(rs)
Can Factory::CreateDrawTargetForCairoSurface create something that isn't 32-bit (e.g., 16-bit vnc scenario), yet still have the DataSourceSurface think that it's SurfaceFormat::B8G8R8A8 or SurfaceFormat::B8G8R8X8, and get through the checks in SurfaceToPackedBGRA function?

Worth a code trace and maybe some assertions/MOZ_CRASHes even without the test file...
I wonder if this is related to bug 1105861...
As per comment 7 and 9, is related to the setup 16-bit vnc. I tried to reproduce the bug on a clean Ubuntu VM using the latest Firefox ASAN debug build. You don't need a testcase file just the next scenario for -depth 16 (or setup Xorg with DefaultDepth 16):

firefox-42.0a1.en-US.linux-x86_64-asan

export ASAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer-3.6
export ASAN_OPTIONS=symbolize=1
vncserver :1 -geometry 1440x900 -depth 16
export DISPLAY=:1
./firefox-bin

Browsing just file:/// I can reproduce it again like the attached file log before:

==7055==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fe0014ff0bf at pc 0x7fe0233e6884 bp 0x7ffc4d325090 sp 0x7ffc4d325088
READ of size 1920 at 0x7fe0014ff0bf thread T0 (Web Content)
--DOCSHELL 0x61a000138c80 == 8 [pid = 6976] [id = 11]
--DOCSHELL 0x61a00018b480 == 7 [pid = 6976] [id = 9]
--DOMWINDOW == 20 (0x619000ad3480) [pid = 6976] [serial = 29] [outer = 0x619000b4e180] [url = about:blank]
--DOMWINDOW == 19 (0x619000b7ac80) [pid = 6976] [serial = 28] [outer = 0x619000b4e180] [url = about:blank]
--DOMWINDOW == 18 (0x619000a80780) [pid = 6976] [serial = 22] [outer = 0x619000dc3c80] [url = about:blank]
--DOMWINDOW == 17 (0x619000114980) [pid = 6976] [serial = 25] [outer = 0x6190004e3e80] [url = about:blank]
--DOMWINDOW == 16 (0x619000b4e180) [pid = 6976] [serial = 27] [outer = (nil)] [url = about:blank]
    #0 0x7fe0233e6883 in mozilla::gfx::CopySurfaceDataToPackedArray(unsigned char*, unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, int, int) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:83
    #1 0x7fe0233e6ae3 in mozilla::gfx::SurfaceToPackedBGRA(mozilla::gfx::DataSourceSurface*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:134
    #2 0x7fe02509c859 in mozilla::dom::CanvasRenderingContext2D::GetImageBuffer(unsigned char**, int*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1633
    #3 0x7fe02536546e in mozilla::dom::HTMLCanvasElement::ToBlob(JSContext*, mozilla::dom::FileCallback&, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/html/HTMLCanvasElement.cpp:594
    #4 0x7fe024dc41e1 in mozilla::dom::HTMLCanvasElementBinding::toBlob(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dom/bindings/./HTMLCanvasElementBinding.cpp:345
    #5 0x7fe02500e306 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/bindings/BindingUtils.cpp:2560
    #6 0x7fe028b63bb2 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/jscntxtinlines.h:235
    #7 0x7fe028b27b9f in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:699:16
    #8 0x7fe028b554a7 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:2959
    #9 0x7fe028b425cb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:655
    #10 0x7fe028b27d04 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:731
    #11 0x7fe028aef9f6 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:768
    #12 0x7fe0293f0332 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/jsapi.cpp:4560
    #13 0x7fe0239920e6 in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/base/nsFrameMessageManager.cpp:1249
    #14 0x7fe02399105c in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/base/nsFrameMessageManager.cpp:1066
    #15 0x7fe025d6df1d in mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/ipc/TabChild.cpp:2762
    #16 0x7fe025d6dfbf in non-virtual thunk to mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dom/ipc/Unified_cpp_dom_ipc0.cpp:2766
    #17 0x7fe022b842d4 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/ipc/ipdl/./PBrowserChild.cpp:2517
    #18 0x7fe022c90d17 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:5435
    #19 0x7fe0226ff680 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1279
    #20 0x7fe0226fdf43 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1198
    #21 0x7fe0226f571c in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1182
    #22 0x7fe02267b771 in MessageLoop::RunTask(Task*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:364
    #23 0x7fe02267c1ef in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:372
    #24 0x7fe02267c7ca in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:459
    #25 0x7fe022705317 in mozilla::ipc::DoWorkRunnable::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:220
    #26 0x7fe021f440ee in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/threads/nsThread.cpp:848
    #27 0x7fe021fb976e in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #28 0x7fe02270499c in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:127
    #29 0x7fe022705760 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:289
    #30 0x7fe02267b431 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #31 0x7fe02267b2d8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #32 0x7fe02619cd46 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/widget/nsBaseAppShell.cpp:165
    #33 0x7fe0277b5996 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:778
    #34 0x7fe0227055f8 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessagePump.cpp:259
    #35 0x7fe02267b431 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:234
    #36 0x7fe02267b2d8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:201
    #37 0x7fe0277b4d80 in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:614
    #38 0x49ad79 in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/app/../contentproc/plugin-container.cpp:236
    #39 0x7fe01eedba3f in __libc_start_main /build/buildd/glibc-2.21/csu/libc-start.c:289
    #40 0x49a03c in _start (/home/osboxes/firefox/plugin-container+0x49a03c)

0x7fe0014ff0bf is located 959 bytes to the right of 288000-byte region [0x7fe0014b8800,0x7fe0014fed00)
allocated by thread T0 (Web Content) here:
    #0 0x482b88 in __interceptor_posix_memalign /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:132
    #1 0x7fe023801165 in TryAllocAlignedBytes(unsigned long) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/thebes/gfxImageSurface.cpp:92
    #2 0x7fe023800d35 in gfxImageSurface::AllocateAndInit(long, int, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/thebes/gfxImageSurface.cpp:129
    #3 0x7fe02378774f in gfxPlatformGtk::CreateOffscreenSurface(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, gfxContentType) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/thebes/gfxPlatformGtk.cpp:140
    #4 0x7fe0237800d0 in gfxPlatform::CreateDrawTargetForBackend(mozilla::gfx::BackendType, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/thebes/gfxPlatform.cpp:1217
    #5 0x7fe02357fa04 in mozilla::layers::PersistentBufferProviderBasic::PersistentBufferProviderBasic(mozilla::layers::LayerManager*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::SurfaceFormat, mozilla::gfx::BackendType) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/layers/PersistentBufferProvider.cpp:22
    #6 0x7fe023563513 in mozilla::layers::LayerManager::CreatePersistentBufferProvider(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/layers/Layers.cpp:167
    #7 0x7fe025099a5a in mozilla::dom::CanvasRenderingContext2D::EnsureTarget(mozilla::dom::CanvasRenderingContext2D::RenderingMode) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1398
    #8 0x7fe02509d080 in mozilla::dom::CanvasRenderingContext2D::Save() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/canvas/CanvasRenderingContext2D.cpp:1684
    #9 0x7fe0246d71b0 in mozilla::dom::CanvasRenderingContext2DBinding::save(JSContext*, JS::Handle<JSObject*>, mozilla::dom::CanvasRenderingContext2D*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dom/bindings/./CanvasRenderingContext2DBinding.cpp:1810
    #10 0x7fe02500e306 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/bindings/BindingUtils.cpp:2560
    #11 0x7fe028b63bb2 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/jscntxtinlines.h:235
    #12 0x7fe028b27b9f in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:699:16
    #13 0x7fe028b554a7 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:2959
    #14 0x7fe028b425cb in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:655
    #15 0x7fe028b27d04 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:731
    #16 0x7fe028aef9f6 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/vm/Interpreter.cpp:768
    #17 0x7fe0293f0332 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/js/src/jsapi.cpp:4560
    #18 0x7fe0239920e6 in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/base/nsFrameMessageManager.cpp:1249
    #19 0x7fe02399105c in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/base/nsFrameMessageManager.cpp:1066
    #20 0x7fe025d6df1d in mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/dom/ipc/TabChild.cpp:2762
    #21 0x7fe025d6dfbf in non-virtual thunk to mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dom/ipc/Unified_cpp_dom_ipc0.cpp:2766
    #22 0x7fe022b842d4 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/ipc/ipdl/./PBrowserChild.cpp:2517
    #23 0x7fe022c90d17 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:5435
    #24 0x7fe0226ff680 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1279
    #25 0x7fe0226fdf43 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1198
    #26 0x7fe0226f571c in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/glue/MessageChannel.cpp:1182
    #27 0x7fe02267b771 in MessageLoop::RunTask(Task*) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:364
    #28 0x7fe02267c1ef in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:372
    #29 0x7fe02267c7ca in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/ipc/chromium/src/base/message_loop.cc:459

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/gfx/2d/DataSurfaceHelpers.cpp:83 mozilla::gfx::CopySurfaceDataToPackedArray(unsigned char*, unsigned char*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, int, int)
Shadow bytes around the buggy address:
  0x0ffc80297dc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297dd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297de0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297df0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0ffc80297e10: fa fa fa fa fa fa fa[fa]fa fa fa fa fa fa fa fa
  0x0ffc80297e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297e40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297e50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0ffc80297e60: 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 right redzon==7055==ABORTING
Flags: needinfo?(rs)
This fixes one of the main symptoms of this bug, that source surfaces were being created with 32bpp formats for 16bpp xlib surfaces because GfxFormatForCairoSurface did not attempt to detect this possibility.

No other Cairo backends appear to allow 16bpp surfaces, so this should be the only platform necessary to account for.
Attachment #8630279 - Flags: review?(jmuizelaar)
CanvasRenderingClient2D was not apparently expecting to have a 16bpp draw target in the first place.

gfxPlatform::CreateDrawTargetForBackend was converting the format to a content type, so that it could be passed to CreateOffscreenSurface, which then gets the optimal format for that content type, thus losing the desired bpp information.

This makes CreateOffscreenSurface take an explicit image format instead, so that bpp information is not lost, and thus CanvasRenderingClient2D will actually get the 32bpp draw target it expected.
Attachment #8630281 - Flags: review?(jmuizelaar)
This fixes other places in Thebes that were mistakenly using OptimalFormatForContent assuming it was a reliable way to get the format of a surface.

This is achieved by adding gfxASurface::GetSurfaceFormat(), which is a wrapper around the fixed GfxFormatForCairoSurface.
Attachment #8630282 - Flags: review?(jmuizelaar)
This is more an incidental finding and cleanup, but in a lot of places we were using OptimalFormatForContent and then doing enum conversions to go from thebes image formats to moz2d surface formats.

This patch just makes such places directly use Optimal2DFormatForContent which avoids the need for enum conversions, both slightly reducing out dependencies on thebes and hopefully avoiding future gotchas like this one that could result from lossy conversions.
Attachment #8630283 - Flags: review?(jmuizelaar)
OS: Unspecified → Linux
Attachment #8630279 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8630281 [details] [diff] [review]
part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess

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

This is a nice patch
Attachment #8630281 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8630282 [details] [diff] [review]
part 3 - add gfxASurface::GetSurfaceFormat for retrieving precise surface format where necessary

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

This one too.
Attachment #8630282 - Flags: review?(jmuizelaar) → review+
Attachment #8630283 - Flags: review?(jmuizelaar) → review+
Nice.  This is not a common scenario, and a very large patch, I suggest we do not attempt an uplift, but just let it ride the trains.
(In reply to Milan Sreckovic [:milan] from comment #18)
> Nice.  This is not a common scenario, and a very large patch, I suggest we
> do not attempt an uplift, but just let it ride the trains.

If an upliftable spotfix was desired for this particular issue, we could just use the the parts 1 and 2 patches that deal with GfxFormatForCairoSurface and CreateOffscreenSurface, so at least this particular canvas trigger would be safe. There are some bits that would need to be adapted since there were recent code changes in the area.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to Lee Salzman [:eihrul] from comment #19)
> (In reply to Milan Sreckovic [:milan] from comment #18)
> > Nice.  This is not a common scenario, and a very large patch, I suggest we
> > do not attempt an uplift, but just let it ride the trains.
> 
> If an upliftable spotfix was desired for this particular issue, we could
> just use the the parts 1 and 2 patches that deal with
> GfxFormatForCairoSurface and CreateOffscreenSurface, so at least this
> particular canvas trigger would be safe. There are some bits that would need
> to be adapted since there were recent code changes in the area.

Agree with Lee Salzman, I think it's a nice set of patches and seems a good way to fix this.
Needs a sec rating and/or sec-approval before it can land.
https://wiki.mozilla.org/Security/Bug_Approval_Process
Keywords: checkin-needed
Flags: needinfo?(lsalzman)
Comment on attachment 8630279 [details] [diff] [review]
part 1 - detect 16bpp cairo xlib surface format

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
The bug regards creating 16bpp surfaces where 32bpp surfaces are expected, so a surface could be created that allows someone to read an amount of random memory following it in the heap the size of the 16bpp surface. If someone created a bunch of such surfaces they might be able to spy some confidential information sitting in the heap.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Not specifically.

> Which older supported branches are affected by this flaw?
Theoretically all branches that support an accessible 2D canvas.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Due to the fact that the patch touches code that was modified recently in central, these patches for the moment only work against central and would need to be backported.

The bug however is confined to users of Cairo and Xlib with 16bpp display depth, so this should only affect Linux users who for some reason use 16bpp display depth. The risk to our main bulk of users on Windows is none, since they can't use 16bpp depth surfaces with our Cairo backend, and by default they use Direct2D instead of Cairo. On Linux most users do not use 16bpp depth anyway so this should only affect a small number of such users there too.

> How likely is this patch to cause regressions; how much testing does it need?
I do not expect it to cause regressions as it does not appear to cause regressions on try. For most platforms, the bug should cause no change in behavior of the code. Only in the case of Linux users with 16bpp display depth will alter the behavior of the code, so even if it did cause a regression, I would only expect it to affect it in this case. Since the behavior of this case before patching was already severely buggy, I do not think this is a worry.
Flags: needinfo?(lsalzman)
Attachment #8630279 - Flags: sec-approval?
Comment on attachment 8630281 [details] [diff] [review]
part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess

See above sec-approval comments.
Attachment #8630281 - Flags: sec-approval?
Comment on attachment 8630282 [details] [diff] [review]
part 3 - add gfxASurface::GetSurfaceFormat for retrieving precise surface format where necessary

See above sec-approval comments.
Attachment #8630282 - Flags: sec-approval?
Comment on attachment 8630283 [details] [diff] [review]
part 4 - use Optimal2DFormatForContent over OptimalFormatForContent to avoid enum conversions

See above sec-approval comments.
Attachment #8630283 - Flags: sec-approval?
Keywords: csectype-bounds, sec-moderate
Comment on attachment 8630279 [details] [diff] [review]
part 1 - detect 16bpp cairo xlib surface format

Clearing sec-approval?. sec-moderates don't need sec-approval+ to land.
Attachment #8630279 - Flags: sec-approval?
Attachment #8630281 - Flags: sec-approval?
Attachment #8630282 - Flags: sec-approval?
Attachment #8630283 - Flags: sec-approval?
The first part of comment 23 describes a sec-high bug, the ability to read significant chunks of leftover memory. Were you downgrading it based on the difficulty of making sense of that memory (doesn't matter) or based on the claim that only some small fraction of our Linux userbase would be vulnerable? At a quick glance I couldn't find any telemetry on what sorts of display depth people have. Do we have those numbers or is it based on an educated guess?
Flags: sec-bounty?
Flags: needinfo?(lsalzman)
(In reply to Daniel Veditz [:dveditz] from comment #28)
> The first part of comment 23 describes a sec-high bug, the ability to read
> significant chunks of leftover memory. Were you downgrading it based on the
> difficulty of making sense of that memory (doesn't matter) or based on the
> claim that only some small fraction of our Linux userbase would be
> vulnerable? At a quick glance I couldn't find any telemetry on what sorts of
> display depth people have. Do we have those numbers or is it based on an
> educated guess?

Based on the fact that it can only apply to a small subset of our users and the educated guess that most people would not use 16bpp by default within that subset, so we're looking at a small percentage of a small percentage.
Flags: needinfo?(lsalzman)
[Tracking Requested - why for this release]:
Given the heavier use of Linux of ESR (especially in the distro versions) we should fix this on ESR when we fix it on mainline. We should uplift this to at least Aurora (41).
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
status-firefox-esr38: --- → affected
tracking-firefox41: --- → ?
tracking-firefox42: --- → +
tracking-firefox-esr38: --- → ?
Whiteboard: [gfx-noted] → [gfx-noted] sec-high affecting small subset of users
Hi Daniel, gfx sec-high or sec-moderate?.I was not sure about impact because eg: LTS distros keep running it with old 16bpp setups like DoD.
I'm asking given the difference between the security ratings in whiteboard and keywords. Thanks
Flags: needinfo?(dveditz)
Keywords: checkin-needed
The keyword is saying the overall security impact for Mozilla-shipped versions of Firefox is sec-moderate. The whiteboard note is a warning or acknowlegement to back-porters that in some contexts they may wish to consider it sec-high.

It's all guessing because no one has telemetry on how many people run in this configuration. Distro versions of Firefox typically turn off telemetry anyway.
Flags: needinfo?(dveditz)
Thanks for the clarification Daniel.
Flags: sec-bounty? → sec-bounty+
Al, do you want to see this fixed for ESR38.2.0? Pending sign-offs from QE, ESR38.2.0 will release on 8/11. Is it ok to take this one in ESR 38.3.0? Thanks.
Flags: needinfo?(abillings)
Just wait for 38.3 unless we're waiting for 38.2 to build again? Do we even know if it applies cleanly on ESR38?
Flags: needinfo?(abillings)
Lee, I noticed that this bug is status-firefox41 "affected". Should we uplift the here fix (if safe) to Beta41 channel?
Flags: needinfo?(lsalzman)
(In reply to Ritu Kothari (:ritu) from comment #40)
> Lee, I noticed that this bug is status-firefox41 "affected". Should we
> uplift the here fix (if safe) to Beta41 channel?

If the patches will apply cleanly against 41 without modifications then it would be worth uplifting, though I'm swamped working on other stuff at the moment so someone else would need to investigate that.
Flags: needinfo?(lsalzman)
Jeff, Lee, since you are the last two devs with comments on this bug, would you be able to help me find someone who can provide a patch for uplift to Beta41? Or if you deem the patch to be unsafe then we have option of setting this to wontfix for FF41. Appreciate any help here!
Flags: needinfo?(lsalzman)
Flags: needinfo?(jmuizelaar)
(In reply to Ritu Kothari (:ritu) from comment #42)
> Jeff, Lee, since you are the last two devs with comments on this bug, would
> you be able to help me find someone who can provide a patch for uplift to
> Beta41? Or if you deem the patch to be unsafe then we have option of setting
> this to wontfix for FF41. Appreciate any help here!

The patch won't apply without modifications to 41.
Flags: needinfo?(lsalzman)
Modified version (cosmetic changes only) of part 1 patch to apply cleanly against 41.
Attachment #8651211 - Flags: review+
Modified version (cosmetic changes only) of part 2 patch to apply against 41.
Attachment #8651213 - Flags: review+
Comment on attachment 8651211 [details] [diff] [review]
upliftable to 41, part 1 - detect 16bpp cairo xlib surface format

Approval Request Comment
[Feature/regressing bug #]: Unknown, but problem likely introduced with Moz2D
[User impact if declined]: Memory overflow for Linux 16bpp users
[Describe test coverage new/current, TreeHerder]: mochitests/reftests
[Risks and why]: Patches have been in 42+ without issue/complaints
[String/UUID change made/needed]: None
Attachment #8651211 - Flags: approval-mozilla-beta?
Comment on attachment 8651213 [details] [diff] [review]
upliftable to 41, part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess

Approval Request Comment
[Feature/regressing bug #]: Unknown, but problem likely introduced with Moz2D
[User impact if declined]: Memory overflow for Linux 16bpp users
[Describe test coverage new/current, TreeHerder]: mochitests/reftests
[Risks and why]: Patches have been in 42+ without issue/complaints
[String/UUID change made/needed]: None
Attachment #8651213 - Flags: approval-mozilla-beta?
Comment on attachment 8630282 [details] [diff] [review]
part 3 - add gfxASurface::GetSurfaceFormat for retrieving precise surface format where necessary

Approval Request Comment
[Feature/regressing bug #]: Unknown, but problem likely introduced with Moz2D
[User impact if declined]: Memory overflow for Linux 16bpp users
[Describe test coverage new/current, TreeHerder]: mochitests/reftests
[Risks and why]: Patches have been in 42+ without issue/complaints
[String/UUID change made/needed]: None
Attachment #8630282 - Flags: approval-mozilla-beta?
Comment on attachment 8630283 [details] [diff] [review]
part 4 - use Optimal2DFormatForContent over OptimalFormatForContent to avoid enum conversions

Approval Request Comment
[Feature/regressing bug #]: Unknown, but problem likely introduced with Moz2D
[User impact if declined]: Memory overflow for Linux 16bpp users
[Describe test coverage new/current, TreeHerder]: mochitests/reftests
[Risks and why]: Patches have been in 42+ without issue/complaints
[String/UUID change made/needed]: None
Attachment #8630283 - Flags: approval-mozilla-beta?
(In reply to Lee Salzman [:eihrul] from comment #43)
> (In reply to Ritu Kothari (:ritu) from comment #42)
> > Jeff, Lee, since you are the last two devs with comments on this bug, would
> > you be able to help me find someone who can provide a patch for uplift to
> > Beta41? Or if you deem the patch to be unsafe then we have option of setting
> > this to wontfix for FF41. Appreciate any help here!
> 
> The patch won't apply without modifications to 41.

I modified the part 1 and part 2 patches to apply against 41. Parts 3 and 4 will apply against 41 unchanged.

That's the best I can do here. Anything before 41 will have to remain WONTFIX.
Thanks Lee, great work.
Comment on attachment 8630282 [details] [diff] [review]
part 3 - add gfxASurface::GetSurfaceFormat for retrieving precise surface format where necessary

These patches have been in Nightly for over a month. They seem safe to uplift to Beta41.
Attachment #8630282 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8630283 [details] [diff] [review]
part 4 - use Optimal2DFormatForContent over OptimalFormatForContent to avoid enum conversions

Beta41+
Attachment #8630283 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8651211 [details] [diff] [review]
upliftable to 41, part 1 - detect 16bpp cairo xlib surface format

Beta41+
Attachment #8651211 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8651213 [details] [diff] [review]
upliftable to 41, part 2 - make gfxPlatform::CreateOffscreenSurface use explicit format rather than guess

Beta41+
Attachment #8651213 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Lee Salzman [:eihrul] from comment #50)
> (In reply to Lee Salzman [:eihrul] from comment #43)
> > (In reply to Ritu Kothari (:ritu) from comment #42)
> > > Jeff, Lee, since you are the last two devs with comments on this bug, would
> > > you be able to help me find someone who can provide a patch for uplift to
> > > Beta41? Or if you deem the patch to be unsafe then we have option of setting
> > > this to wontfix for FF41. Appreciate any help here!
> > 
> > The patch won't apply without modifications to 41.
> 
> I modified the part 1 and part 2 patches to apply against 41. Parts 3 and 4
> will apply against 41 unchanged.
> 
> That's the best I can do here. Anything before 41 will have to remain
> WONTFIX.

Do you also mean wontfix for ESR38.3.0? Al recommended we fix it. See comment 39.
Flags: needinfo?(jmuizelaar) → needinfo?(lsalzman)
(In reply to Ritu Kothari (:ritu) from comment #57)
> (In reply to Lee Salzman [:eihrul] from comment #50)
> > (In reply to Lee Salzman [:eihrul] from comment #43)
> > > (In reply to Ritu Kothari (:ritu) from comment #42)
> > > > Jeff, Lee, since you are the last two devs with comments on this bug, would
> > > > you be able to help me find someone who can provide a patch for uplift to
> > > > Beta41? Or if you deem the patch to be unsafe then we have option of setting
> > > > this to wontfix for FF41. Appreciate any help here!
> > > 
> > > The patch won't apply without modifications to 41.
> > 
> > I modified the part 1 and part 2 patches to apply against 41. Parts 3 and 4
> > will apply against 41 unchanged.
> > 
> > That's the best I can do here. Anything before 41 will have to remain
> > WONTFIX.
> 
> Do you also mean wontfix for ESR38.3.0? Al recommended we fix it. See
> comment 39.

I meant wontfix for all of 38.

I could technically make a patch that would apply against 38. But I would not be willing to make any guarantees as to the safety of the change, since the differences between 38 and 41 internally are quite big, and I couldn't reasonably go back and make sure there were no changes between then and now that might have subtly or unpredictably interfering behavior.

So you'd have to be okay with incorporating a fix which, basically, I have no idea if it would cause problems or not, and the burden of testing would have to be on someone else.

I'm just not sure this particular issue warrants more focus than it has already received, given its limited user effect radius.

I'll leave that decisions up to you guys.
Flags: needinfo?(lsalzman) → needinfo?(rkothari)
I am ok with your decision. If porting the patch over to ESR38 creates more problem than fixing the existing one, setting it to wontfix seems like a reasonable option.

Al, Daniel, are you OK with that?
Flags: needinfo?(rkothari) → needinfo?(dveditz)
Hi Francisco, can you take a look at a recent build of Fx41 and comment here on how the fix looks for you? Thank you.
Flags: needinfo?(rs)
I'm OK with MoCo not fixing it, but we might want to let folks on security-group know because for some of them (e.g. Linux distros, Tor browser) this configuration might be a larger percentage of their users. They might be willing to put in the back-porting and testing time it would take.
Flags: needinfo?(dveditz)
Whiteboard: [gfx-noted] sec-high affecting small subset of users → [gfx-noted] sec-high when using Cairo xlib 16-bit
Group: core-security → core-security-release
Sorry, this should fix the Android failure. try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e0a48cb3441
Attachment #8654827 - Attachment is obsolete: true
Whiteboard: [gfx-noted] sec-high when using Cairo xlib 16-bit → [post-critsmash-triage][gfx-noted] sec-high when using Cairo xlib 16-bit
(In reply to Matt Wobensmith from comment #60)
> Hi Francisco, can you take a look at a recent build of Fx41 and comment here
> on how the fix looks for you? Thank you.

Fix looks ok for me, thanks!
Flags: needinfo?(rs)
Whiteboard: [post-critsmash-triage][gfx-noted] sec-high when using Cairo xlib 16-bit → [post-critsmash-triage][gfx-noted][adv-main41+] sec-high when using Cairo xlib 16-bit
Alias: CVE-2015-4512
The dev suggested we wontfix this for esr38 due to high risk to esr release quality owing to significant code changes made to fix this sec issue. Sec team agrees with this proposal (see comment 61). Updating the bug to reflect esr38 as wontfix.
status-firefox-esr38: affected → wontfix
tracking-firefox-esr38: ? → -
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: