914966 - Heap-buffer-overflow in nsPNGEncoder::ConvertHostARGBRow

Reporter

Description

•

11 years ago

Attached file Testcase — Details

==15451==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x629000284d00 at pc 0x7ff18e8d9b57 bp 0x7ff151b414d0 sp 0x7ff151b414c8
READ of size 4 at 0x629000284d00 thread T53
    #0 0x7ff18e8d9b56 in nsPNGEncoder::ConvertHostARGBRow(unsigned char const*, unsigned char*, unsigned int, bool) image/encoders/png/nsPNGEncoder.cpp:599
    #1 0x7ff18e8d33df in nsPNGEncoder::AddImageFrame(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) image/encoders/png/nsPNGEncoder.cpp:267
    #2 0x7ff18e8cf9b2 in nsPNGEncoder::InitFromData(unsigned char const*, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, nsAString_internal const&) image/encoders/png/nsPNGEncoder.cpp:62
    #3 0x7ff1914b3e28 in mozilla::dom::ImageEncoder::GetInputStream(int, int, unsigned char*, int, imgIEncoder*, unsigned short const*, nsIInputStream**) content/canvas/src/ImageEncoder.cpp:208
    #4 0x7ff1914b1230 in mozilla::dom::ImageEncoder::ExtractDataInternal(nsAString_internal const&, nsAString_internal const&, unsigned char*, int, nsIntSize, nsICanvasRenderingContextInternal*, nsIInputStream**, imgIEncoder*) content/canvas/src/ImageEncoder.cpp:240
    #5 0x7ff1914b6da4 in mozilla::dom::EncodingRunnable::Run() content/canvas/src/ImageEncoder.cpp:99
    #6 0x7ff19e63f95c in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
    #7 0x7ff19e276643 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
    #8 0x7ff19e639dfc in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:250
    #9 0x7ff1b1ad2d52 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:204
    #10 0x447693 in __asan::AsanThread::ThreadStart(unsigned long) _asan_rtl_
    #11 0x7ff1b5016e99 in start_thread
    #12 0x7ff1b4125ccc in
0x629000284d00 is located 0 bytes to the right of 19200-byte region [0x629000280200,0x629000284d00)
allocated by thread T0 here:
    #0 0x440b65 in malloc _asan_rtl_
    #1 0x7ff1b540ae4a in moz_malloc memory/mozalloc/mozalloc.cpp:64
    #2 0x7ff1913d5a69 in operator new[](unsigned long, mozilla::fallible_t const&) objdir-ff-asan-sym/content/canvas/src/../../../dist/include/mozilla/mozalloc.h:275
    #3 0x7ff1913d5a69 in mozilla::dom::CanvasRenderingContext2D::GetImageBuffer(unsigned char**, int*) content/canvas/src/CanvasRenderingContext2D.cpp:1063
    #4 0x7ff191e23e57 in mozilla::dom::HTMLCanvasElement::ToBlob(nsIFileCallback*, nsAString_internal const&, JS::Value const&, JSContext*) content/html/content/src/HTMLCanvasElement.cpp:525
    #5 0x7ff19b3a0026 in mozilla::dom::HTMLCanvasElement::ToBlob(JSContext*, nsIFileCallback*, nsAString_internal const&, mozilla::dom::Optional<JS::Handle<JS::Value> > const&, mozilla::ErrorResult&) objdir-ff-asan-sym/dom/bindings/../../dist/include/mozilla/dom/HTMLCanvasElement.h:103
    #6 0x7ff19b38fd18 in mozilla::dom::HTMLCanvasElementBinding::toBlob(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) objdir-ff-asan-sym/dom/bindings/HTMLCanvasElementBinding.cpp:256
    #7 0x7ff19b38b795 in mozilla::dom::HTMLCanvasElementBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLCanvasElementBinding.cpp:556
    #8 0x7ff1a6f3e0c3 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:219
    #9 0x7ff1a6f3e0c3 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:489
    #10 0x7ff1a6f1cab1 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2459
    #11 0x7ff1a6ec59e3 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:446
    #12 0x7ff1a6f3e742 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:508
    #13 0x7ff1a6f41b10 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:539
    #14 0x7ff1a79cf8c7 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5327
    #15 0x7ff19647b243 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1445
    #16 0x7ff196443d48 in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:587
    #17 0x7ff19e796927 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #18 0x7ff19e79399a in SharedStub
    #19 0x7ff191c85c64 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:962
    #20 0x7ff191c877e6 in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:1033
    #21 0x7ff191c71eea in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.h:325
    #22 0x7ff191c63a86 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:189
    #23 0x7ff191c5420a in nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:286
    #24 0x7ff191c5aa7e in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) content/events/src/nsEventDispatcher.cpp:599
    #25 0x7ff191c5c33d in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) content/events/src/nsEventDispatcher.cpp:666
    #26 0x7ff191035d84 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) content/base/src/nsINode.cpp:1137
    #27 0x7ff190ae344c in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3366
    #28 0x7ff190ae27a8 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3336
    #29 0x7ff190d28ec4 in nsDocument::DispatchContentLoadedEvents() content/base/src/nsDocument.cpp:4605:40
    #30 0x7ff190e3ff0b in nsRunnableMethodImpl<void (nsDocument::*)(), void, true>::Run() objdir-ff-asan-sym/content/base/src/../../../dist/include/nsThreadUtils.h:418
    #31 0x7ff19e63f95c in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
Thread T53 created by T0 here:
    #0 0x431fd1 in __interceptor_pthread_create _asan_rtl_
    #1 0x7ff1b1ac4644 in _PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:444
    #2 0x7ff1b1ac2a1a in PR_CreateThread nsprpub/pr/src/pthreads/ptthread.c:527
    #3 0x7ff19e63c2de in nsThread::Init() xpcom/threads/nsThread.cpp:316
    #4 0x7ff19e65765d in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) xpcom/threads/nsThreadManager.cpp:214
    #5 0x7ff19e274201 in NS_NewThread(nsIThread**, nsIRunnable*, unsigned int) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:67
    #6 0x7ff1914b2b65 in mozilla::dom::ImageEncoder::ExtractDataAsync(nsAString_internal&, nsAString_internal const&, bool, unsigned char*, int, nsIntSize, nsICanvasRenderingContextInternal*, JSContext*, nsIFileCallback*) content/canvas/src/ImageEncoder.cpp:180
    #7 0x7ff191e240b2 in mozilla::dom::HTMLCanvasElement::ToBlob(nsIFileCallback*, nsAString_internal const&, JS::Value const&, JSContext*) content/html/content/src/HTMLCanvasElement.cpp:534
    #8 0x7ff19b3a0026 in mozilla::dom::HTMLCanvasElement::ToBlob(JSContext*, nsIFileCallback*, nsAString_internal const&, mozilla::dom::Optional<JS::Handle<JS::Value> > const&, mozilla::ErrorResult&) objdir-ff-asan-sym/dom/bindings/../../dist/include/mozilla/dom/HTMLCanvasElement.h:103
    #9 0x7ff19b38fd18 in mozilla::dom::HTMLCanvasElementBinding::toBlob(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLCanvasElement*, JSJitMethodCallArgs const&) objdir-ff-asan-sym/dom/bindings/HTMLCanvasElementBinding.cpp:256
    #10 0x7ff19b38b795 in mozilla::dom::HTMLCanvasElementBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLCanvasElementBinding.cpp:556
    #11 0x7ff1a6f3e0c3 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:219
    #12 0x7ff1a6f3e0c3 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:489
    #13 0x7ff1a6f1cab1 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2459
    #14 0x7ff1a6ec59e3 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:446
    #15 0x7ff1a6f3e742 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:508
    #16 0x7ff1a6f41b10 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:539
    #17 0x7ff1a79cf8c7 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5327
    #18 0x7ff19647b243 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1445
    #19 0x7ff196443d48 in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:587
    #20 0x7ff19e796927 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #21 0x7ff19e79399a in SharedStub
    #22 0x7ff191c85c64 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:962
    #23 0x7ff191c877e6 in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.cpp:1033
    #24 0x7ff191c71eea in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) content/events/src/nsEventListenerManager.h:325
    #25 0x7ff191c63a86 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:189
    #26 0x7ff191c5420a in nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*) content/events/src/nsEventDispatcher.cpp:286
    #27 0x7ff191c5aa7e in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) content/events/src/nsEventDispatcher.cpp:599
    #28 0x7ff191c5c33d in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) content/events/src/nsEventDispatcher.cpp:666
    #29 0x7ff191035d84 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) content/base/src/nsINode.cpp:1137
    #30 0x7ff190ae344c in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3366
    #31 0x7ff190ae27a8 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) content/base/src/nsContentUtils.cpp:3336
    #32 0x7ff190d28ec4 in nsDocument::DispatchContentLoadedEvents() content/base/src/nsDocument.cpp:4605:40
    #33 0x7ff190e3ff0b in nsRunnableMethodImpl<void (nsDocument::*)(), void, true>::Run() objdir-ff-asan-sym/content/base/src/../../../dist/include/nsThreadUtils.h:418
    #34 0x7ff19e63f95c in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
    #35 0x7ff19e276643 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
    #36 0x7ff198eaf9b8 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:81
    #37 0x7ff19eb8b7a7 in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:220
    #38 0x7ff19eb8b3fa in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:213
    #39 0x7ff19eb8b2d5 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:187
    #40 0x7ff1985e6d6f in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:161
    #41 0x7ff196f5163f in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:269
    #42 0x7ff18c0a6791 in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3869
    #43 0x7ff18c0a960a in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3937
    #44 0x7ff18c0aba94 in XRE_main toolkit/xre/nsAppRunner.cpp:4139
    #45 0x457257 in do_main(int, char**, nsIFile*) browser/app/nsBrowserApp.cpp:275
    #46 0x454385 in main browser/app/nsBrowserApp.cpp:635
    #47 0x7ff1b405376c in
Shadow bytes around the buggy address:
  0x0c5280048950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c5280048960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c5280048970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c5280048980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c5280048990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c52800489a0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c52800489b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c52800489c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c52800489d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c52800489e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c52800489f0: 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
  ASan internal:         fe
==15451==ABORTING

Olli Pettay [:smaug][bugs@pettay.fi]

Updated

•

11 years ago

Component: General → ImageLib

Product: Firefox → Core

Christoph Diehl [:posidron]

Updated

•

11 years ago

Severity: normal → critical

Keywords: crash, csec-bounds, testcase

Christoph Diehl [:posidron]

Comment 1

•

11 years ago

Abhishek, can you still reproduce this crash?
Have tested it against mozilla-inbound and mozilla-central on Linux and MacOS but no crash appeared.

Abhishek Arya

Reporter

Comment 2

•

11 years ago

Can't test, asan clang build seems broken on trunk [my build used to file report was like 3-4 days back]

./UnionTypes.h:43:17: error: use of undeclared identifier 'NonNull'
    UnionMember<NonNull<nsDOMEvent> > mEvent;
                ^
./UnionTypes.h:43:25: error: 'nsDOMEvent' does not refer to a value
    UnionMember<NonNull<nsDOMEvent> > mEvent;

Btw, i am using clang r186322. I will try to update clang to see if it fixes this. Which clang revision are you using on Linux ?

Christoph Diehl [:posidron]

Comment 3

•

11 years ago

Personally am still using r188899 but try with: 

https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1378984056/firefox-26.0a1.en-US.linux-x86_64-asan.tar.bz2

https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64-asan/1378984839/firefox-26.0a1.en-US.linux-x86_64-asan.tar.bz2

I think those are built with r185949.

Abhishek Arya

Reporter

Comment 4

•

11 years ago

Clang build works now. Funny part is it didn't reproduce on trunk. Then i checked the stack and couldn't find ImageEncoder.cpp. Looks like this change was rolled out (on Monday; just 1-2 days after my build), but it just went in again. See bug https://bugzilla.mozilla.org/show_bug.cgi?id=817700. I think the bug should reproduce pretty soon on mc. You probably want to cc spohl.mozilla.bugs@

Andrew McCreight [:mccr8]

Comment 5

•

11 years ago

Done, thanks.

Christoph Diehl [:posidron]

Updated

•

11 years ago

Keywords: sec-critical

Christoph Diehl [:posidron]

Updated

•

11 years ago

Blocks: 817700

Updated

•

11 years ago

Assignee: nobody → spohl.mozilla.bugs

Stephen A Pohl [:spohl]

Comment 6

•

11 years ago

It seems right to back out bug 817700 until this issue (and bug 916128) is resolved. If I don't hear otherwise, I'll go ahead with the backout today.

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

Assignee

Comment 7

•

11 years ago

The problem here is that HTMLCanvasElement changes size between sizing the context in (HTMLCanvasElement::UpdateContext and where it creates the async request to encode the image in HTMLCanvasElement::ToBlob, and we don't find out.  So, by the time the encoding happens, the canvas and the image are different sizes.  We make assumptions they are in sync.  Removing the attribute appears not to get the HTMLCanvasElement.
Backing out bug 817700 may fix this (did somebody try?) but the underlying problem is probably going to remain there.  I'll take a quick look, but let us know if the backout happens.

Stephen A Pohl [:spohl]

Comment 8

•

11 years ago

Backout of bug 817700 happened with https://hg.mozilla.org/integration/mozilla-inbound/rev/c36560cef148.

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

Assignee

Comment 9

•

11 years ago

Attached patch Listen for the unsetting of the attribute so that it can be reset to the default — Details — Splinter Review

This fixes the async version - I will try with the backout and see if it's a problem and if this fixes it.

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

Assignee

Comment 10

•

11 years ago

Right - this particular problem goes away with the backout because it does not matter that HTMLCanvasElement and the context it holds are the wrong size. So, there is no attack, but it's still wrong. I'll spin a separate bug for "shouldn't HTMLCanvasElement::UnsetAttr exist".

Stephen A Pohl [:spohl]

Comment 11

•

11 years ago

Comment on attachment 804704 [details] [diff] [review]
Listen for the unsetting of the attribute so that it can be reset to the default

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

Thanks Milan for jumping on this. Do you think this is ready for a review? I would land this at the same time as the patches for bug 817700. Let me know if anything else needs to be done.

::: content/html/content/src/HTMLCanvasElement.cpp
@@ +172,5 @@
> +HTMLCanvasElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName,
> +                             bool aNotify)
> +{
> +  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify);
> +  if (NS_SUCCEEDED(rv) && mCurrentContext && 

white space at the end of the line

@@ +495,5 @@
>    nsAutoString type;
>    aRv = nsContentUtils::ASCIIToLower(aType, type);
>    if (aRv.Failed()) {
>      return;
>    }

probably want to keep the new line here

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

Assignee

Comment 12

•

11 years ago

Because this doesn't reproduce anymore, I created bug 916322 to track this without the "it fixes this problem" part.  Perhaps it makes sense to have bug 817700 depend on 916322? I just did that, will apply your changes and some other comments and track it there.

Stephen A Pohl [:spohl]

Comment 13

•

11 years ago

Great, thanks Milan! I'll wait for bug 916322 to land and will then land bug 817700 and bug 916128 at the same time. Should this bug be closed because it doesn't reproduce at the moment?

Assignee: spohl.mozilla.bugs → nobody

David Bolter [:davidb] (NeedInfo me for attention)

Comment 14

•

11 years ago

We should probably leave this open for any related explicit security discussion. Will there need to be branch landings?

Stephen A Pohl [:spohl]

Comment 15

•

11 years ago

(In reply to David Bolter [:davidb] from comment #14)
> Will there need to be branch landings?

I wouldn't think so. Bug 817700, which exposed this issue, is needed for history swipe animations (bug 678392) and once turned on (bug 860493), we'll probably want to let it ride the train to fix any fallout from the feature.

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

status-firefox25: --- → ?

status-firefox26: --- → affected

status-firefox27: --- → affected

Whiteboard: [asan]

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

Assignee

Comment 16

•

11 years ago

I'd say status-firefox25 and 26 should be "not affected" - see comment 15.  This only becomes an issue once Bug 817700 lands, and it is not going to land until we fix this.

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

status-firefox25: ? → unaffected

status-firefox26: affected → unaffected

status-firefox-esr17: --- → unaffected

status-firefox-esr24: --- → unaffected

tracking-firefox27: --- → +

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

Assignee

Updated

•

11 years ago

Assignee: nobody → milan

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

Assignee

Comment 17

•

11 years ago

This problem cannot be seen until bug 817700 lands; in the meantime bug 916322 landed, and contains the fix for this once it's revealed.  I'm not completely sure what to do with the status for this bug though? Resolve fixed, or something else?

Status: NEW → ASSIGNED

Stephen A Pohl [:spohl]

Comment 18

•

11 years ago

FYI: I'll update the patches for bug 817700 and hope to have them landed today.

Andrew McCreight [:mccr8]

Comment 19

•

11 years ago

Yeah, that sounds reasonable.

Status: ASSIGNED → RESOLVED

Closed: 11 years ago

Flags: sec-bounty?

Resolution: --- → FIXED

Whiteboard: [asan] → [asan][fixed by bug 916322]

Ryan VanderMeulen [:RyanVM]

Updated

•

11 years ago

status-firefox27: affected → fixed

Target Milestone: --- → mozilla27

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

Flags: sec-bounty? → sec-bounty+

David Chan [:dchan]

Comment 21

•

11 years ago

Cleaning up list of security bugs for b2g18. This bug doesn't need to be backported either due to it affecting a later version of Fx or another reason.

status-b2g18: --- → unaffected

Christoph Diehl [:posidron]

Updated

•

11 years ago

Comment 22

•

11 years ago

I can't reproduce the original issue using an ASan build of FF27 from 2013-09-25 - tried both Mac/Linux.

Comment 1 seems to confirm that. And comments 8 and 12 indicate that it was likely fixed by other check-ins.

Abhishek, it would be great if you could confirm that this was fixed, since we're not able to do so. Thank you.

Abhishek Arya

Reporter

Comment 23

•

11 years ago

(In reply to Matt Wobensmith from comment #22)
> I can't reproduce the original issue using an ASan build of FF27 from
> 2013-09-25 - tried both Mac/Linux.
> 
> Comment 1 seems to confirm that. And comments 8 and 12 indicate that it was
> likely fixed by other check-ins.
> 
> Abhishek, it would be great if you could confirm that this was fixed, since
> we're not able to do so. Thank you.

Yes this is fixed. See also https://bugzilla.mozilla.org/show_bug.cgi?id=936795#c4

Matt Wobensmith [:mwobensmith][:matt:]

Comment 24

•

11 years ago

Thank you, Abhishek.

Status: RESOLVED → VERIFIED

status-firefox27: fixed → verified

Daniel Veditz [:dveditz]

Updated

•

9 years ago

Group: core-security

Testcase 11 years ago Abhishek Arya 211 bytes, text/html		Details
Listen for the unsetting of the attribute so that it can be reset to the default 11 years ago Milan Sreckovic [:milan] (needinfo for best results) 3.33 KB, patch		Details \| Diff \| Splinter Review