Closed Bug 898871 (CVE-2013-1735) Opened 11 years ago Closed 11 years ago

ASAN heap-use-after-free in mozilla::layout::ScrollbarActivity

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

Product:
Core
Component:
Layout: Images, Video, and HTML Frames
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla26
Tracking Flags:
Tracking Status
firefox23 --- wontfix
firefox24 --- verified
firefox25 --- verified
firefox26 + verified
firefox-esr17 24+ verified
b2g18 24+ fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: nils, Assigned: MatsPalmgren_bugz)

References

Details

(4 keywords, Whiteboard: [asan][adv-main24+][adv-esr1709+])

Attachments

(5 files, 1 obsolete file)

testcase.zip (crash.html crashes firefox)
15.74 KB, application/java-archive
Details
a few stacks of interest
36.69 KB, text/html
Details
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.
1.50 KB, patch
roc
: review+
abillings
: approval-mozilla-aurora+
abillings
: approval-mozilla-beta+
akeybl
: approval-mozilla-esr17+
bajaj
: approval-mozilla-b2g18+
abillings
: sec-approval+
Details | Diff | Splinter Review
Assert if the shell was deleted
3.27 KB, patch
roc
: review+
Details | Diff | Splinter Review
More nsWeakFrame checks
46.49 KB, patch
roc
: review+
Details | Diff | Splinter Review 
The attached testcase crashes the firefox ASAN build with following ASAN output. The testcase is attached as a zip archive, as it requires several files.

================================================================
==45947==ERROR: AddressSanitizer: heap-use-after-free on address 0x625001110a98 at pc 0x7feca0144595 bp 0x7fffdd1e1ff0 sp 0x7fffdd1e1fe8
READ of size 8 at 0x625001110a98 thread T0
    #0 0x7feca0144594 in nsCOMPtr<mozilla::layout::ScrollbarActivity>::get() const /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/layout/generic/../../dist/include/nsCOMPtr.h:800:0
    #1 0x7feca0144594 in nsCOMPtr<mozilla::layout::ScrollbarActivity>::operator mozilla::layout::ScrollbarActivity*() const /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/layout/generic/../../dist/include/nsCOMPtr.h:813:0
    #2 0x7feca0144594 in nsGfxScrollFrameInner::SetCoordAttribute(nsIContent*, nsIAtom*, int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:3861:0
    #3 0x7feca013bbb4 in nsGfxScrollFrameInner::UpdateScrollbarPosition() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:2941:0
    #4 0x7feca0136fbe in nsGfxScrollFrameInner::ScrollToImpl(nsPoint, nsRect const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:2012:0
    #5 0x7feca013886c in nsGfxScrollFrameInner::ScrollToWithOrigin(nsPoint, nsIScrollableFrame::ScrollMode, nsIAtom*, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:1651:0
    #6 0x7feca0028e0c in nsGfxScrollFrameInner::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.h:172:0
    #7 0x7feca0028e0c in nsHTMLScrollFrame::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.h:524:0
    #8 0x7feca0028e0c in non-virtual thunk to nsHTMLScrollFrame::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:4099:0
    #9 0x7feca0ef0f66 in mozilla::dom::ImageDocument::ScrollImageTo(int, int, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:379:0
    #10 0x7feca0ef07d1 in mozilla::dom::ImageDocument::ShrinkToFit() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:336:0
    #11 0x7feca0ef532b in mozilla::dom::ImageDocument::CheckOverflowing(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:636:0
    #12 0x7feca0ef4857 in mozilla::dom::ImageDocument::HandleEvent(nsIDOMEvent*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:537:0
    #13 0x7feca0ef545c in non-virtual thunk to mozilla::dom::ImageDocument::HandleEvent(nsIDOMEvent*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:565:0
    #14 0x7feca0a9d6a2 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, mozilla::dom::CallbackObjectHolder<mozilla::dom::EventListener, nsIDOMEventListener> const&, nsIDOMEvent*, mozilla::dom::EventTarget*, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventListenerManager.cpp:941:0
    #15 0x7feca0a9e3e8 in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventListenerManager.cpp:1012:0
    #16 0x7feca0a90577 in nsEventTargetChainItem::CurrentTarget() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventListenerManager.h:328:0
    #17 0x7feca0a90577 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventDispatcher.cpp:221:0
    #18 0x7feca0a8f4f1 in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventDispatcher.cpp:342:0
    #19 0x7feca0a93753 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventDispatcher.cpp:644:0
    #20 0x7fec9ff969ad in PresShell::FireResizeEvent() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:1956:0
    #21 0x7fec9ffa9fbc in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:3807:0
    #22 0x7fec9ff9a635 in PresShell::HandlePostedReflowCallbacks(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:3711:0
    #23 0x7fec9ff9a635 in PresShell::DidDoReflow(bool, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:7663:0
    #24 0x7fec9ff97768 in PresShell::ResizeReflowIgnoreOverride(int, int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:1885:0
    #25 0x7feca1229c1f in nsViewManager::DoSetWindowDimensions(int, int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/view/src/nsViewManager.cpp:197:0
    #26 0x7feca1229c1f in nsViewManager::SetWindowDimensions(int, int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/view/src/nsViewManager.cpp:217:0
    #27 0x7fec9ff1dbd7 in nsDocumentViewer::SetBounds(nsIntRect const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsDocumentViewer.cpp:1874:39
    #28 0x7fec9ff1dc67 in nsDocumentViewer::SetBounds(nsIntRect const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsDocumentViewer.cpp:1886:0
    #29 0x7feca1fd913a in nsDocShell::SetPositionAndSize(int, int, int, int, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/docshell/base/nsDocShell.cpp:5101:0
    #30 0x7feca1fd913a in non-virtual thunk to nsDocShell::SetPositionAndSize(int, int, int, int, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/docshell/base/nsDocShell.cpp:5105:0
    #31 0x7feca0822249 in nsFrameLoader::UpdateBaseWindowPositionAndSize(nsSubDocumentFrame*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsFrameLoader.cpp:1884:0
    #32 0x7feca0821d97 in nsFrameLoader::UpdatePositionAndSize(nsSubDocumentFrame*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsFrameLoader.cpp:1858:0
    #33 0x7feca01eb7da in nsSubDocumentFrame::ReflowFinished() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsSubDocumentFrame.cpp:692:0
    #34 0x7feca01eb7da in non-virtual thunk to nsSubDocumentFrame::ReflowFinished() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsSubDocumentFrame.cpp:701:0
    #35 0x7fec9ff9a5cf in PresShell::HandlePostedReflowCallbacks(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:3702:0
    #36 0x7fec9ff9a5cf in PresShell::DidDoReflow(bool, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:7663:0
    #37 0x7fec9ffaaf6e in PresShell::ProcessReflowCommands(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:7996:0
    #38 0x7fec9ffaa708 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:3896:0
    #39 0x7feca07bcb88 in nsDocument::FlushPendingNotifications(mozFlushType) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsDocument.cpp:7087:0
    #40 0x7feca07bc98e in nsDocument::FlushPendingNotifications(mozFlushType) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsDocument.cpp:7065:0
    #41 0x7feca202fba5 in nsDocLoader::DocLoaderIsEmpty(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/uriloader/base/nsDocLoader.cpp:709:0
    #42 0x7feca203159a in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/uriloader/base/nsDocLoader.cpp:639:0
    #43 0x7feca2031e49 in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/uriloader/base/nsDocLoader.cpp:642:0
    #44 0x7fec9f6b4176 in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, tag_nsresult) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/netwerk/base/src/nsLoadGroup.cpp:684:0
    #45 0x7feca07c33aa in nsDocument::DoUnblockOnload() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsDocument.cpp:7967:0
    #46 0x7feca07c2eaf in nsDocument::UnblockOnload(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsDocument.cpp:7895:0
    #47 0x7feca07a1492 in nsDocument::DispatchContentLoadedEvents() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/nsDocument.cpp:4685:0
    #48 0x7feca07ea2ac in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/content/base/src/../../../dist/include/nsThreadUtils.h:350:0
    #49 0x7fec9f47e2ea in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/xpcom/threads/nsThread.cpp:621:0
    #50 0x7fec9f3a74f6 in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/xpcom/build/nsThreadUtils.cpp:238:0
    #51 0x7fec9e294011 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/ipc/glue/MessagePump.cpp:81:0
    #52 0x7fec9f5a6533 in MessageLoop::RunInternal() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/ipc/chromium/src/base/message_loop.cc:219:0
    #53 0x7fec9f5a6533 in MessageLoop::RunHandler() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/ipc/chromium/src/base/message_loop.cc:212:0
    #54 0x7fec9f5a6533 in MessageLoop::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/ipc/chromium/src/base/message_loop.cc:186:0
    #55 0x7feca27a8bec in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/widget/xpwidgets/nsBaseAppShell.cpp:163:0
    #56 0x7feca21c9abe in nsAppStartup::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/toolkit/components/startup/nsAppStartup.cpp:269:0
    #57 0x7fec9df31730 in XREMain::XRE_mainRun() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/toolkit/xre/nsAppRunner.cpp:3858:0
    #58 0x7fec9df32695 in XREMain::XRE_main(int, char**, nsXREAppData const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/toolkit/xre/nsAppRunner.cpp:3926:0
    #59 0x7fec9df335cb in XRE_main /builds/slave/m-cen-l64-asan-ntly-0000000000/build/toolkit/xre/nsAppRunner.cpp:4128:0
    #60 0x459add in do_main(int, char**, nsIFile*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/browser/app/nsBrowserApp.cpp:272:0
    #61 0x459add in main /builds/slave/m-cen-l64-asan-ntly-0000000000/build/browser/app/nsBrowserApp.cpp:632:0
    #62 0x7fecacadaea4 in ?? ??:0
    #63 0x458f5c in _start ??:?
0x625001110a98 is located 4504 bytes inside of 8192-byte region [0x62500110f900,0x625001111900)
freed by thread T0 here:
    #0 0x445e65 in __interceptor_free _asan_rtl_:0
    #1 0x7fecaa10f7e3 in FreeArenaList /builds/slave/m-cen-l64-asan-ntly-0000000000/build/nsprpub/lib/ds/plarena.c:273:0
    #2 0x7fecaa10f7e3 in PL_FinishArenaPool /builds/slave/m-cen-l64-asan-ntly-0000000000/build/nsprpub/lib/ds/plarena.c:313:0
    #3 0x7fec9ff8cdd8 in PresShell::~PresShell() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/base/nsPresShell.cpp:747:0
    #4 0x7feca23bf03e in nsEditingSession::SetupEditorOnWindow(nsIDOMWindow*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/editor/composer/src/nsEditingSession.cpp:332:0
    #5 0x7feca23bd390 in nsEditingSession::MakeWindowEditable(nsIDOMWindow*, char const*, bool, bool, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/editor/composer/src/nsEditingSession.cpp:168:0
    #6 0x7feca0f1acc9 in nsHTMLDocument::EditingStateChanged() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/nsHTMLDocument.cpp:2871:0
    #7 0x7feca0f36a7c in nsRunnableMethodImpl<void (nsHTMLDocument::*)(), true>::Run() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/content/html/document/src/../../../../dist/include/nsThreadUtils.h:350:0
    #8 0x7feca063dc6c in ~nsAutoScriptBlocker /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/content/base/src/../../../dist/include/nsContentUtils.h:2248:0
    #9 0x7feca063dc6c in ~nsAutoScriptBlocker /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/content/base/src/../../../dist/include/nsContentUtils.h:2247:0
    #10 0x7feca063dc6c in mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/base/src/Element.cpp:1629:0
    #11 0x7feca01444f1 in nsIContent::SetAttr(int, nsIAtom*, nsAString_internal const&, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/obj-firefox/layout/generic/../../dist/include/nsIContent.h:350:0
    #12 0x7feca01444f1 in nsGfxScrollFrameInner::SetCoordAttribute(nsIContent*, nsIAtom*, int) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:3859:0
    #13 0x7feca013bbb4 in nsGfxScrollFrameInner::UpdateScrollbarPosition() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:2941:0
    #14 0x7feca0136fbe in nsGfxScrollFrameInner::ScrollToImpl(nsPoint, nsRect const&) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:2012:0
    #15 0x7feca013886c in nsGfxScrollFrameInner::ScrollToWithOrigin(nsPoint, nsIScrollableFrame::ScrollMode, nsIAtom*, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:1651:0
    #16 0x7feca0028e0c in nsGfxScrollFrameInner::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.h:172:0
    #17 0x7feca0028e0c in nsHTMLScrollFrame::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.h:524:0
    #18 0x7feca0028e0c in non-virtual thunk to nsHTMLScrollFrame::ScrollTo(nsPoint, nsIScrollableFrame::ScrollMode, nsRect const*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/layout/generic/nsGfxScrollFrame.cpp:4099:0
    #19 0x7feca0ef0f66 in mozilla::dom::ImageDocument::ScrollImageTo(int, int, bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:379:0
    #20 0x7feca0ef07d1 in mozilla::dom::ImageDocument::ShrinkToFit() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:336:0
    #21 0x7feca0ef532b in mozilla::dom::ImageDocument::CheckOverflowing(bool) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:636:0
    #22 0x7feca0ef545c in non-virtual thunk to mozilla::dom::ImageDocument::HandleEvent(nsIDOMEvent*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/html/document/src/ImageDocument.cpp:565:0
    #23 0x7feca0a9e3e8 in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventListenerManager.cpp:1012:0
    #24 0x7feca0a90577 in nsEventTargetChainItem::CurrentTarget() /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventListenerManager.h:328:0
    #25 0x7feca0a90577 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&, nsCxPusher*) /builds/slave/m-cen-l64-asan-ntly-0000000000/build/content/events/src/nsEventDispatcher.cpp:221:0
previously allocated by thread T0 here:
    #0 0x445fa5 in malloc _asan_rtl_:0
    #1 0x7fecaa10edca in PL_ArenaAllocate /builds/slave/m-cen-l64-asan-ntly-0000000000/build/nsprpub/lib/ds/plarena.c:200:0
Shadow bytes around the buggy address:
  0x0c4a8021a100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a120: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a130: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c4a8021a150: fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a160: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4a8021a1a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
==45947==ABORTING
This is similar to bug 842166, but worse.  In this case the shell is deallocated
so frame poisoning doesn't help, it's a use-after-free.
Assignee: nobody → matspal
Severity: normal → critical
Flags: sec-bounty?
Keywords: crash, csec-uaf, sec-critical, testcase
OS: Linux → All
Hardware: x86 → All
Whiteboard: [asan]
ImageDocument::ScrollImageTo requests a synchronous scroll.
SetCoordAttribute executes a script runner which Destroy's
the shell.  FlushPendingNotifications had the last ref to it
so it's deallocated.  Back in SetCoordAttribute, 'this' is
now pointing to freed memory, it crashes when calling
this->mScrollbarActivity->ActivityOccurred().
Attached patch wip (obsolete) — — Splinter Review 
This actually fixes the crash but not as thoroughly as I would like.

It holds the shell alive, although it'll still be Destroy'ed.
So the scroll frame still points to allocated memory, but the
scroll frame destructor has run (but it's not poisoned since
we teared down the whole shell), so this->mScrollbarActivity is
a valid access and it's null so we skip the ActivityOccurred() call.

Still, continuing to run scroll frame methods for instances who's
dtor has run (but are still allocated) seems ... undesirable ;-)
Stephen, I guess this bug may also have caused some crash reports with
a ScrollbarActivity method signature, although none of that code is to blame.

        Attachment #785284 -
        Flags: review?(roc)

    
    

    
  
I've also audited all code involved in scrolling one way or another (phew!)
to make sure they hold a strong ref on the pres shell for the duration of
the scroll operation.

    
    

    
  
Comment on attachment 785285 [details] [diff] [review]
More nsWeakFrame checks

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

I really hate this, but OK.

        Attachment #785285 -
        Flags: review?(roc) → review+

    
    

    
  
Comment on attachment 785283 [details] [diff] [review]
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.

[Security approval request comment]

Request and answers below is for all three patches...

How easily could an exploit be constructed based on the patch?
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Not really.  You can probably work out from the patch/comments that
scrolling an image document is part of the problem, but I think
it's probably hard to construct a crash test from that.

Which older supported branches are affected by this flaw?

All.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Should be trivial to backport.

How likely is this patch to cause regressions; how much testing does it need?

Low risk.  No specific testing required.

        Attachment #785283 -
        Flags: sec-approval?

    
    

    
  
Comment on attachment 785283 [details] [diff] [review]
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.

sec-approval+ for trunk.

Once it is in, you should nominate this for branch and create branch patches. I expect that we'll want to take this everywhere.

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

    
  
Flags: in-testsuite?

    
    

    
  
Comment on attachment 785283 [details] [diff] [review]
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: likely exploitable crash
Testing completed (on m-c, etc.): none
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

The patches applies with minor changes to aurora/beta/esr17 branches.

        Attachment #785283 -
        Flags: approval-mozilla-esr17?

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

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

    
    

    
  
Comment on attachment 785283 [details] [diff] [review]
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.

Approving for Aurora and Beta. Will want Release Management input for ESR.

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

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

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

        Attachment #785283 -
        Flags: approval-mozilla-aurora+
Flags: needinfo?(release-mgmt)
Flags: sec-bounty?

    
    

    
  
Mats, can we get this into ESR please?
Flags: needinfo?(release-mgmt) → needinfo?(matspal)
Whiteboard: [asan] → [asan][adv-main24+]

    
    

    
  
Was someone going to formally approve this for esr17 at some point? Or are bug comments replacing patch flags now?

    
  

        Attachment #785283 -
        Flags: approval-mozilla-esr17? → approval-mozilla-esr17+

    
    

    
  
I've built local ASan builds for ESR17, 24, 25 and 26.

This fix looks good in ESR17 and 26.

However, I get the same crash in 24 and 25 as originally reported in comment 0. 

Mats, would you mind taking a look?

    
    

    
  
Actually, scratch that, Mats. Looks like this could be a local build issue for me, where my codebase was not updated properly.

Assume all is good for now, please. :)
Whiteboard: [asan][adv-main24+] → [asan][adv-main24+][adv-esr1709+]

    
    

    
  
Verified no longer crashes in ASan FF24 and FF25, 2013-08-26.

I can see the asserts now, post-fix. Based on comment 6, I assume this is to be expected.

Question for anyone: 
Does this bug affect b2g or Fennec? If not, we can safely mark it verified and close it out.

          status-firefox24:
          fixed → verified

          status-firefox25:
          fixed → verified

          status-firefox26:
          fixed → verified

          status-firefox-esr17:
          fixed → verified

    
    

    
  
(In reply to Matt Wobensmith from comment #23)
> I can see the asserts now, post-fix. Based on comment 6, I assume this is to
> be expected.

No, you should NOT see the assertion I added ("pres shell was destroyed by scrolling").  There may be other assertions though.

> Does this bug affect b2g or Fennec? If not, we can safely mark it verified
> and close it out.

I would assume the mozilla-b2g18 branch is affected.

    
    

    
  
I do not see your assertion, so this sounds good.

The bug files attached to this bug do cause these two assertions, however:

[Parent 46755] ###!!! ASSERTION: No Document Request!: 'mDocumentRequest', file /Users/mwobensmith/asan_moz_central/uriloader/base/nsDocLoader.cpp, line 726
[Parent 46755] ###!!! ASSERTION: Firing OnStateChange(...) notification with a NULL request!: 'aRequest', file /Users/mwobensmith/asan_moz_central/uriloader/base/nsDocLoader.cpp, line 1303

Unrelated?

    
    

    
  
Looks unrelated to the code I touched, yes.
That's probably bug 479160 -- I added a note there just in case that test
doesn't reproduce it anymore.
Alias: CVE-2013-1735

        Attachment #782265 -
        Attachment mime type: application/octet-stream → application/java-archive

    
    

    
  
Comment on attachment 785283 [details] [diff] [review]
Hold a strong ref on the pres shell while scrolling, check for damage afterwards.

[Approval Request Comment]
sec-critical.  See above.

        Attachment #785283 -
        Flags: approval-mozilla-b2g18?

        Attachment #785283 -
        Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+

    
    

    
  
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0740553e48d
Group: core-security
Flags: in-testsuite? → in-testsuite+

    
  
Product: Core → Core Graveyard

    
  
Product: Core Graveyard → Core
See Also:  → 1602368

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: