871099 - (CVE-2013-1685) Heap-use-after-free in nsIDocument::GetRootElement

Status: VERIFIED FIXED

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

Description

[Abhishek Arya]

Attachment: Testcase

==8886== ERROR: AddressSanitizer: heap-use-after-free on address 0x601c0009a3e0 at pc 0x7ffc01ebdca5 bp 0x7fff2ec8f5f0 sp 0x7fff2ec8f5e8
READ of size 8 at 0x601c0009a3e0 thread T0
    #0 0x7ffc01ebdca4 in nsINode::GetParentNode() const ../../dist/include/nsINode.h:762
    #1 0x7ffc03b3f9ec in nsIDocument::GetRootElement() const content/base/src/nsDocument.cpp:3610
    #2 0x7ffc03b948d5 in nsIDocument::GetHtmlElement() const content/base/src/nsDocument.cpp:5819
    #3 0x7ffc03b94bee in nsIDocument::GetHtmlChildElement(nsIAtom*) content/base/src/nsDocument.cpp:5828
    #4 0x7ffc01f37dd5 in nsIDocument::GetBodyElement() ../../dist/include/nsIDocumentInlines.h:15
    #5 0x7ffc05a4a9b6 in nsHTMLDocument::GetBody() content/html/document/src/nsHTMLDocument.cpp:1085
    #6 0x7ffc05a4af0d in nsHTMLDocument::GetBody(nsIDOMHTMLElement**) content/html/document/src/nsHTMLDocument.cpp:1106
    #7 0x7ffc05a4b382 in non-virtual thunk to nsHTMLDocument::GetBody(nsIDOMHTMLElement**) content/html/document/src/nsHTMLDocument.cpp:1109
    #8 0x7ffc07b29165 in nsHTMLEditor::GetBodyElement(nsIDOMHTMLElement**) editor/libeditor/html/nsHTMLEditor.cpp:5288
    #9 0x7ffc07b281bf in nsHTMLEditor::GetRootElement(nsIDOMElement**) editor/libeditor/html/nsHTMLEditor.cpp:336
    #10 0x7ffc07b856bc in nsHTMLEditor::ResetRootElementAndEventTarget() editor/libeditor/html/nsHTMLEditor.cpp:5259
    #11 0x7ffc07bc544a in nsRunnableMethodImpl<void (nsHTMLEditor::*)(), true>::Run() ../../../dist/include/nsThreadUtils.h:350
    #12 0x7ffc0397bd4a in nsContentUtils::RemoveScriptBlocker() content/base/src/nsContentUtils.cpp:4971
    #13 0x7ffc03b631f3 in nsDocument::EndUpdate(unsigned int) content/base/src/nsDocument.cpp:4342
    #14 0x7ffc05a6b7c1 in nsHTMLDocument::EndUpdate(unsigned int) content/html/document/src/nsHTMLDocument.cpp:2580
    #15 0x7ffc02793151 in mozAutoDocUpdate::~mozAutoDocUpdate() content/xul/templates/src/../../../base/src/mozAutoDocUpdate.h:38
    #16 0x7ffc027657ee in mozAutoDocUpdate::~mozAutoDocUpdate() content/xul/templates/src/../../../base/src/mozAutoDocUpdate.h:36
    #17 0x7ffc03b2d819 in nsDocument::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*) content/base/src/nsDocument.cpp:2074
    #18 0x7ffc05a34b3e in nsHTMLDocument::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*) content/html/document/src/nsHTMLDocument.cpp:294
    #19 0x7ffc03b2c47d in nsDocument::Reset(nsIChannel*, nsILoadGroup*) content/base/src/nsDocument.cpp:2012
    #20 0x7ffc05a34692 in nsHTMLDocument::Reset(nsIChannel*, nsILoadGroup*) content/html/document/src/nsHTMLDocument.cpp:281
    #21 0x7ffc05a558fd in nsHTMLDocument::Open(JSContext*, nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) content/html/document/src/nsHTMLDocument.cpp:1687
    #22 0x7ffc0d6a2ff6 in mozilla::dom::HTMLDocumentBinding::open(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLDocumentBinding.cpp:524
    #23 0x7ffc0d68eba5 in mozilla::dom::HTMLDocumentBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLDocumentBinding.cpp:1545
    #24 0x7ffc172428ef in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337
    #25 0x7ffc172428ef in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:426
    #26 0x7ffc1721befe in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2393
    #27 0x7ffc171cb0be in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:383
    #28 0x7ffc17242f98 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:440
    #29 0x7ffc17246f5b in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:473
    #30 0x7ffc16b72040 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5851
    #31 0x7ffc0913a469 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1435
    #32 0x7ffc090ec13c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:578
    #33 0x7ffc0f776394 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #34 0x7ffc0f77344a in SharedStub
0x601c0009a3e0 is located 32 bytes inside of 160-byte region [0x601c0009a3c0,0x601c0009a460)
freed by thread T0 here:
    #0 0x41a9e2 in __interceptor_free
    #1 0x7ffc1fff36de in moz_free memory/mozalloc/mozalloc.cpp:48
    #2 0x7ffc055f9ef9 in operator delete(void*) ../../../../dist/include/mozilla/mozalloc.h:225
    #3 0x7ffc055f9ef9 in mozilla::dom::HTMLSharedElement::~HTMLSharedElement() content/html/content/src/HTMLSharedElement.cpp:28
    #4 0x7ffc03f12bb5 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258
    #5 0x7ffc041bfe9a in mozilla::dom::FragmentOrElement::Release() content/base/src/FragmentOrElement.cpp:1713
    #6 0x7ffc055fa925 in mozilla::dom::HTMLSharedElement::Release() content/html/content/src/HTMLSharedElement.cpp:32
    #7 0x7ffc0021450a in nsCOMPtr_base::assign_assuming_AddRef(nsISupports*) objdir-ff-asan-sym/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsCOMPtr.h:470
    #8 0x7ffc0f2a8f72 in nsCOMPtr_base::assign_with_AddRef(nsISupports*) objdir-ff-asan-sym/xpcom/build/nsCOMPtr.cpp:49
    #9 0x7ffc03a2bfc4 in nsCOMPtr<mozilla::dom::Element>::operator=(mozilla::dom::Element*) ../../../../dist/include/nsCOMPtr.h:664
    #10 0x7ffc07b8544a in nsHTMLEditor::ResetRootElementAndEventTarget() editor/libeditor/html/nsHTMLEditor.cpp:5251
    #11 0x7ffc07bc544a in nsRunnableMethodImpl<void (nsHTMLEditor::*)(), true>::Run() ../../../dist/include/nsThreadUtils.h:350
    #12 0x7ffc0397bd4a in nsContentUtils::RemoveScriptBlocker() content/base/src/nsContentUtils.cpp:4971
    #13 0x7ffc03b631f3 in nsDocument::EndUpdate(unsigned int) content/base/src/nsDocument.cpp:4342
    #14 0x7ffc05a6b7c1 in nsHTMLDocument::EndUpdate(unsigned int) content/html/document/src/nsHTMLDocument.cpp:2580
    #15 0x7ffc02793151 in mozAutoDocUpdate::~mozAutoDocUpdate() content/xul/templates/src/../../../base/src/mozAutoDocUpdate.h:38
    #16 0x7ffc027657ee in mozAutoDocUpdate::~mozAutoDocUpdate() content/xul/templates/src/../../../base/src/mozAutoDocUpdate.h:36
    #17 0x7ffc03b2d819 in nsDocument::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*) content/base/src/nsDocument.cpp:2074
    #18 0x7ffc05a34b3e in nsHTMLDocument::ResetToURI(nsIURI*, nsILoadGroup*, nsIPrincipal*) content/html/document/src/nsHTMLDocument.cpp:294
    #19 0x7ffc03b2c47d in nsDocument::Reset(nsIChannel*, nsILoadGroup*) content/base/src/nsDocument.cpp:2012
    #20 0x7ffc05a34692 in nsHTMLDocument::Reset(nsIChannel*, nsILoadGroup*) content/html/document/src/nsHTMLDocument.cpp:281
    #21 0x7ffc05a558fd in nsHTMLDocument::Open(JSContext*, nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) content/html/document/src/nsHTMLDocument.cpp:1687
    #22 0x7ffc0d6a2ff6 in mozilla::dom::HTMLDocumentBinding::open(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLDocumentBinding.cpp:524
    #23 0x7ffc0d68eba5 in mozilla::dom::HTMLDocumentBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/HTMLDocumentBinding.cpp:1545
    #24 0x7ffc172428ef in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337
    #25 0x7ffc172428ef in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:426
    #26 0x7ffc1721befe in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2393
    #27 0x7ffc171cb0be in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:383
    #28 0x7ffc17242f98 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:440
    #29 0x7ffc17246f5b in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:473
    #30 0x7ffc16b72040 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5851
    #31 0x7ffc0913a469 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1435
previously allocated by thread T0 here:
    #0 0x41aac2 in malloc
    #1 0x7ffc1fff3825 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
    #2 0x7ffc055f9b9e in operator new(unsigned long) ../../../../dist/include/mozilla/mozalloc.h:201
    #3 0x7ffc055f9b9e in nsGenericHTMLElement* mozilla::dom::NewHTMLElementHelper::Create<nsHTMLSharedElement, mozilla::dom::HTMLSharedElement>(already_AddRefed<nsINodeInfo>, mozilla::dom::NewHTMLElementHelper::SFINAE<bool (*)(nsIDocument*), mozilla::dom::HTMLSharedElement::InNavQuirksMode>*) content/html/content/src/nsGenericHTMLElement.h:1869
    #4 0x7ffc055f99ab in NS_NewHTMLSharedElement(already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/html/content/src/HTMLSharedElement.cpp:20
    #5 0x7ffc05a01d8e in CreateHTMLElement(unsigned int, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/html/document/src/nsHTMLContentSink.cpp:498
    #6 0x7ffc05a025b5 in NS_NewHTMLElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/html/document/src/nsHTMLContentSink.cpp:481
    #7 0x7ffc03ee0d9a in NS_NewElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) content/base/src/nsNameSpaceManager.cpp:192
    #8 0x7ffc079ac890 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) parser/html/nsHtml5TreeOperation.cpp:349
    #9 0x7ffc079cb355 in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:557
    #10 0x7ffc07a05962 in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:125
    #11 0x7ffc0f64ede7 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:627
    #12 0x7ffc0f2ebf82 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
    #13 0x7ffc09c2d27b in nsXULWindow::ShowModal() xpfe/appshell/src/nsXULWindow.cpp:362
    #14 0x7ffc09c0c667 in nsContentTreeOwner::ShowAsModal() xpfe/appshell/src/nsContentTreeOwner.cpp:522
    #15 0x7ffc09c0c818 in non-virtual thunk to nsContentTreeOwner::ShowAsModal() xpfe/appshell/src/nsContentTreeOwner.cpp:523
    #16 0x7ffc09a297e0 in nsWindowWatcher::OpenWindowInternal(nsIDOMWindow*, char const*, char const*, char const*, bool, bool, bool, nsIArray*, nsIDOMWindow**) embedding/components/windowwatcher/src/nsWindowWatcher.cpp:1007
    #17 0x7ffc09a1d0a4 in nsWindowWatcher::OpenWindow(nsIDOMWindow*, char const*, char const*, char const*, nsISupports*, nsIDOMWindow**) embedding/components/windowwatcher/src/nsWindowWatcher.cpp:343
    #18 0x7ffc0f770ceb in NS_InvokeByIndex xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
    #19 0x7ffc0917c44f in CallMethodHelper::Invoke() js/xpconnect/src/XPCWrappedNative.cpp:2948
    #20 0x7ffc0917c44f in CallMethodHelper::Call() js/xpconnect/src/XPCWrappedNative.cpp:2283
    #21 0x7ffc0917c44f in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:2249
    #22 0x7ffc091d5f41 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1485
    #23 0x7ffc172428ef in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:337
    #24 0x7ffc172428ef in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:426
    #25 0x7ffc1721befe in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) js/src/jsinterp.cpp:2393
    #26 0x7ffc171cb0be in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:383
    #27 0x7ffc17242f98 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:440
    #28 0x7ffc17246f5b in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:473
    #29 0x7ffc16b72040 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5851
    #30 0x7ffc0913a469 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1435
    #31 0x7ffc090ec13c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:578
    #32 0x7ffc0f776394 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
    #33 0x7ffc0f77344a in SharedStub
Shadow bytes around the buggy address:
  0x0c040000b420: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c040000b430: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c040000b440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c040000b450: 00 00 00 fa fa fa fa fa fa fa fa fa 00 00 00 00
  0x0c040000b460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c040000b470: fa fa fa fa fa fa fa fa fd fd fd fd[fd]fd fd fd
  0x0c040000b480: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c040000b490: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c040000b4a0: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c040000b4b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c040000b4c0: fd fd fd fd fa fa fa fa fa fa fa fa 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 righ 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
==8886== ABORTING

Comment 1

[Mats Palmgren (inactive)]

The problem is 'mCachedRootElement' is being used although it was removed
and has been destroyed.  ASan detects this at GetParentNode():

nsIDocument::GetRootElement() const
{
  return (mCachedRootElement && mCachedRootElement->GetParentNode() == this) ?
         mCachedRootElement : GetRootElementInternal();
}

but I suspect that in a release build we'll unwind to nsHTMLDocument::GetBody
which calls CallQueryInterface which I suspect use the vtbl which would make
this exploitable.

nsHTMLDocument::GetBody(nsIDOMHTMLElement** aBody)
{
  *aBody = nullptr;

  nsIContent *body = GetBody();

  return body ? CallQueryInterface(body, aBody) : NS_OK;
}

(the other methods at the top of the stack are inlines)

Comment 2

[Mats Palmgren (inactive)]

Attachment: fix

This should fix it.  I checked the other places that do
mChildren.RemoveChildAt and they look safe wrt mCachedRootElement.

Alternatively we could change nsIDocument.h:
  mozilla::dom::Element* mCachedRootElement;
to use a nsRefPtr, then GetParentNode() would be safe to use and return
null if the root was removed so GetRootElement() would do the non-cached
lookup.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 749100 [details] [diff] [review]
fix

Please add a comment about how we want to null it out before the update goes out of scope.

r=me

Comment 4

[Mats Palmgren (inactive)]

Attachment: crashtest and code comment - LAND WHEN BUG IS PUBLIC

> Please add a comment about how we want to null it out before the update goes
> out of scope.

I suggest we delay adding the code comment until the bug is public.
(on second thought - I'll remove the commit message too)

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 749100 [details] [diff] [review]
fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
The code change suggests that the crash involves removing/modifying
the root element, in combination with changing the URL,
which I guess helps finding a way to trigger the crash.
Should be relatively easy to exploit once you have the crash.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
I'll remove the commit message before landing

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?
same patch should work

How likely is this patch to cause regressions; how much testing does it need?
zero risk of regressions, no testing needed

Comment 6

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 749100 [details] [diff] [review]
fix

sec-approval+ for checking in on m-c on 5/28. Please make branch patches and nominate after m-c checkin.

Comment 7

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

This is ready to land?

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Any time next week.

Comment 9

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6444d16bfb57

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 749100 [details] [diff] [review]
fix

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

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6444d16bfb57

Comment 12

[Mats Palmgren (inactive)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/12e6914af648
https://hg.mozilla.org/releases/mozilla-beta/rev/6b4643b6b64b
https://hg.mozilla.org/releases/mozilla-esr17/rev/c3e507fc131c
https://hg.mozilla.org/releases/mozilla-b2g18/rev/8f5274ec2ec3

Comment 13

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

Confirmed crash on ASan FF23, 2013-04-18
Confirmed fixed on ASan FF17, 2013-06-18
Confirmed fixed on ASan FF22/FF23, 2013-06-14
Confirmed fixed on ASan FF24, 2013-06-06