860971 - (CVE-2013-1674) UAF with video and onresize event

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nils]

Attachment: testcase (crashes firefox)

The attached testcase crashes central and stables. skeleton.ogv needs to be in the same directory. The stack on windows looks as follows:

Comment 1

[Nils]

WARNING: Frame IP not in any known module. Following frames may be wrong.
0x8005c95e
xul!nsDocumentViewer::Show+0x187
xul!nsDocShell::SetVisibility+0x25
xul!nsFrameLoader::Show+0x144
xul!nsSubDocumentFrame::ShowViewer+0x92
xul!AsyncFrameInit::Run+0x13
xul!nsContentUtils::RemoveScriptBlocker+0x654381
xul!PresShell::FlushPendingNotifications+0x267
xul!PresShell::FlushPendingNotifications+0x1a
xul!nsEventStateManager::FlushPendingEvents+0x14
xul!nsEventStateManager::PreHandleEvent+0x7d2c77
xul!PresShell::HandleEventInternal+0x114
xul!PresShell::HandlePositionedEvent+0xa5
xul!PresShell::HandleEvent+0x1f0
xul!nsViewManager::DispatchEvent+0xa9
xul!nsView::HandleEvent+0x42
xul!nsWindow::DispatchEvent+0x28
xul!nsWindow::DispatchWindowEvent+0x13
xul!nsWindow::DispatchMouseEvent+0x4b3
xul!nsWindow::ProcessMessage+0x5c0779

Comment 2

[Nils]

Attachment: required video file

Comment 3

[Nils]

ASAN output:

==21559== ERROR: AddressSanitizer: heap-use-after-free on address 0x7fe0dc7b40c0 at pc 0x7fe10df87c57 bp 0x7fffb3770a60 sp 0x7fffb3770a58
READ of size 8 at 0x7fe0dc7b40c0 thread T0
    #0 0x7fe10df87c56 in _ZNK8nsCOMPtrI20nsISelectionListenerE3getEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/../../dist/include/nsCOMPtr.h:764
    #1 0x7fe10df68d95 in _ZN16nsDocumentViewer21InitPresentationStuffEb /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsDocumentViewer.cpp:747
    #2 0x7fe10df71cec in _ZN16nsDocumentViewer4ShowEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsDocumentViewer.cpp:2012
    #3 0x7fe10f7258b4 in _ZN10nsDocShell13SetVisibilityEb /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:5477
    #4 0x7fe10f7258e3 in _ZThn312_N10nsDocShell13SetVisibilityEb /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:5481
    #5 0x7fe10e5d754f in _ZN13nsFrameLoader4ShowEiiiiP18nsSubDocumentFrame /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsFrameLoader.cpp:821
    #6 0x7fe10e197ecf in _ZN18nsSubDocumentFrame10ShowViewerEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/generic/nsSubDocumentFrame.cpp:209
    #7 0x7fe10e19e778 in _ZN14AsyncFrameInit3RunEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/generic/nsSubDocumentFrame.cpp:103
    #8 0x7fe10e510c9b in _ZN14nsContentUtils19RemoveScriptBlockerEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsContentUtils.cpp:5082
    #9 0x7fe10dee41cd in ~nsAutoScriptBlocker /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/../../../../dist/include/nsContentUtils.h:2296
    #10 0x7fe10dfe3b8b in _ZN9PresShell25FlushPendingNotificationsEN7mozilla14ChangesToFlushE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsPresShell.cpp:3877
    #11 0x7fe10dfe34aa in _ZN9PresShell25FlushPendingNotificationsE12mozFlushType /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsPresShell.cpp:3761
    #12 0x7fe10e7b4659 in _ZN19nsEventStateManager14PreHandleEventEP13nsPresContextP7nsEventP8nsIFrameP13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/events/src/nsEventStateManager.cpp:1023
    #13 0x7fe10dff4976 in _ZN9PresShell19HandleEventInternalEP7nsEventP13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsPresShell.cpp:6810
    #14 0x7fe10dff3a73 in _ZN9PresShell21HandlePositionedEventEP8nsIFrameP10nsGUIEventP13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsPresShell.cpp:6564
    #15 0x7fe10dff2845 in _ZN9PresShell11HandleEventEP8nsIFrameP10nsGUIEventbP13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsPresShell.cpp:6363
    #16 0x7fe10eba1f08 in _ZN13nsViewManager13DispatchEventEP10nsGUIEventP6nsViewP13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/view/src/nsViewManager.cpp:712
    #17 0x7fe10eb9e4da in _ZN6nsView11HandleEventEP10nsGUIEventb /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/view/src/nsView.cpp:1015
    #18 0x7fe10fc9b3fc in _ZN8nsWindow13DispatchEventEP10nsGUIEventR13nsEventStatus /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/widget/gtk2/nsWindow.cpp:475
    #19 0x7fe10fca810f in _ZN8nsWindow19OnMotionNotifyEventEP15_GdkEventMotion /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/widget/gtk2/nsWindow.cpp:2557
    #20 0x7fe10fcafaa7 in _ZL22motion_notify_event_cbP10_GtkWidgetP15_GdkEventMotion /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/widget/gtk2/nsWindow.cpp:5205
    #21 0x7fe109806dd7 in ?? ??:0
0x7fe0dc7b40c0 is located 128 bytes inside of 336-byte region [0x7fe0dc7b4040,0x7fe0dc7b4190)
freed by thread T0 here:
    #0 0x433c80 in free ??:0
    #1 0x7fe10df663f7 in _ZN16nsDocumentViewer7ReleaseEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsDocumentViewer.cpp:555
    #2 0x7fe10dde893d in _ZN8nsCOMPtrI16nsIContentViewerEaSEPS0_ /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/../../../dist/include/nsCOMPtr.h:624
    #3 0x7fe10f722864 in _ZN10nsDocShell7DestroyEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:5028
    #4 0x7fe10f722c6f in _ZThn312_N10nsDocShell7DestroyEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:5070
    #5 0x7fe10e5d6742 in _ZN13nsFrameLoader8FinalizeEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsFrameLoader.cpp:564
    #6 0x7fe10e57d119 in _ZN10nsDocument35MaybeInitializeFinalizeFrameLoadersEv /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsDocument.cpp:6197
    #7 0x7fe10e57cea5 in _ZN10nsDocument9EndUpdateEj /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsDocument.cpp:4323
    #8 0x7fe10ea2dbbf in _ZN14nsHTMLDocument9EndUpdateEj /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/html/document/src/nsHTMLDocument.cpp:2577
    #9 0x7fe10e10530f in ~mozAutoDocUpdate /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/xul/templates/src/../../../base/src/mozAutoDocUpdate.h:38
    #10 0x7fe10e6199e1 in _ZN7nsINode21ReplaceOrInsertBeforeEbPS_S0_RN7mozilla11ErrorResultE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/base/src/nsINode.cpp:1950
    #11 0x7fe10ea25b77 in _ZN14nsHTMLDocument7SetBodyEP20nsGenericHTMLElementRN7mozilla11ErrorResultE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/content/html/document/src/nsHTMLDocument.cpp:1135
    #12 0x7fe1103c7433 in _ZN7mozilla3dom19HTMLDocumentBindingL8set_bodyEP9JSContextN2JS6HandleIP8JSObjectEEP14nsHTMLDocumentPNS4_5ValueE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:207
    #13 0x7fe1103c1a68 in _ZN7mozilla3dom19HTMLDocumentBindingL13genericSetterEP9JSContextjPN2JS5ValueE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:1629
    #14 0x7fe111c93ee2 in _ZN2js12CallJSNativeEP9JSContextPFiS1_jPN2JS5ValueEERKNS2_8CallArgsE /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/js/src/jscntxtinlines.h:338
previously allocated by thread T0 here:
    #0 0x433d40 in __interceptor_malloc ??:0
    #1 0x7fe1183e2417 in moz_xmalloc /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/memory/mozalloc/mozalloc.cpp:54
    #2 0x7fe10df6588e in _Znwm /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/../../dist/include/mozilla/mozalloc.h:201
    #3 0x7fe10df657f3 in _Z19NS_NewContentViewerPP16nsIContentViewer /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/base/nsDocumentViewer.cpp:504
    #4 0x7fe10de8349e in _ZN12nsContentDLF14CreateDocumentEPKcP10nsIChannelP12nsILoadGroupP11nsISupportsRK4nsIDPP17nsIStreamListenerPP16nsIContentViewer /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/build/nsContentDLF.cpp:403
    #5 0x7fe10de8309a in _ZN12nsContentDLF14CreateInstanceEPKcP10nsIChannelP12nsILoadGroupS1_P11nsISupportsS7_PP17nsIStreamListenerPP16nsIContentViewer /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/layout/build/nsContentDLF.cpp:205
    #6 0x7fe10f7399ea in _ZN10nsDocShell19NewContentViewerObjEPKcP10nsIRequestP12nsILoadGroupPP17nsIStreamListenerPP16nsIContentViewer /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:8132
    #7 0x7fe10f736e99 in _ZN10nsDocShell19CreateContentViewerEPKcP10nsIRequestPP17nsIStreamListener /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDocShell.cpp:7938
    #8 0x7fe10f76237c in _ZN22nsDSURIContentListener9DoContentEPKcbP10nsIRequestPP17nsIStreamListenerPb /builds/slave/m-cen-l64-dbg-asan-ntly-000000/build/docshell/base/nsDSURIContentListener.cpp:123
Shadow byte and word:
  0x1ffc1b8f6818: fd
  0x1ffc1b8f6818: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1ffc1b8f67f8: fa fa fa fa fa fa fa fa
  0x1ffc1b8f6800: fa fa fa fa fa fa fa fa
  0x1ffc1b8f6808: fd fd fd fd fd fd fd fd
  0x1ffc1b8f6810: fd fd fd fd fd fd fd fd
=>0x1ffc1b8f6818: fd fd fd fd fd fd fd fd
  0x1ffc1b8f6820: fd fd fd fd fd fd fd fd
  0x1ffc1b8f6828: fd fd fd fd fd fd fd fd
  0x1ffc1b8f6830: fd fd fd fd fd fd fd fd
  0x1ffc1b8f6838: fa fa fa fa fa fa fa fa

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This is the safest possible patch I can imagine.
I went through other callers of SetVisibility, and they looked safe.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Old code from year 2000
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/docshell/base&command=DIFF_FRAMESET&file=nsDocShell.cpp&rev2=1.55&rev1=1.54
User impact if declined: crashes
Testing completed: Not yet, but this should be super safe
Risk to taking this patch (and alternatives if risky): Should be super safe
String or UUID changes made by this patch: NA

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 736792 [details] [diff] [review]
patch

In SetVisibility, document that Show/Hide can change mContentViewer?

r=me

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Olli Pettay [:smaug] from comment #4)
> [Approval Request Comment]

Would you mind going through the sec-approval process?

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 736792 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch? Probably not extremely easily, but somebody could look at all the existing callers of this function as Olli did, and may be able to figure something out.

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

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? they should be the same

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

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +comment

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9022b73d427a

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9022b73d427a

Comment 11

[bhavana bajaj [:bajaj]]

Comment on attachment 736792 [details] [diff] [review]
patch

low risk patch for a sec-crit crash.Approving on aurora in preparation for a beta uplift by tomorrow.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/db999ea82b97

Comment 13

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

Good to uplift.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 736792 [details] [diff] [review]
patch

Approving for beta/esr17 landing, no need to approve for b2g18 since this is tef+.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0373b241072e
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e89233880b40
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/112c874cb203
https://hg.mozilla.org/releases/mozilla-esr17/rev/5aa9a5849c3d

Comment 17

[Matt Wobensmith [:mwobensmith][:matt:]]

Reproduced on FF17.0.5esr
Confirmed fixed on FF17.0.6esr candidate build 1
Confirmed fixed on FF21 candidate build 2
Confirmed fixed on FF22 2013-05-09
Confirmed fixed on FF23 2013-05-09