916404 - (CVE-2013-5603) Heap-use-after-free in nsContentUtils::ContentIsHostIncludingDescendantOf

Status: RESOLVED FIXED

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

Description

[Abhishek Arya]

Attachment: Testcase

Need to install Jesse's fuzzPriv extension.

>==19928==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160020b5520 at pc 0x7f18a8bb0b38 bp 0x7fff1ca5cc20 sp 0x7fff1ca5cc18
>READ of size 8 at 0x6160020b5520 thread T0
>    #0 0x7f18a8bb0b37 in get objdir-ff-asan/content/base/src/../../../dist/include/nsCOMPtr.h:800
>    #1 0x7f18a8bb0b37 in operator-> objdir-ff-asan/content/base/src/../../../dist/include/nsCOMPtr.h:820
>    #2 0x7f18a8bb0b37 in NodeType objdir-ff-asan/content/base/src/../../../dist/include/nsINode.h:506
>    #3 0x7f18a8bb0b37 in nsContentUtils::ContentIsHostIncludingDescendantOf(nsINode const*, nsINode const*) content/base/src/nsContentUtils.cpp:1862
>    #4 0x7f18a8d15a72 in IsAllowedAsChild(nsIContent*, nsINode*, bool, nsINode*) content/base/src/nsINode.cpp:1542
>    #5 0x7f18a8d12720 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) content/base/src/nsINode.cpp:1747
>    #6 0x7f18ab855e25 in InsertBefore objdir-ff-asan/dom/bindings/../../dist/include/nsINode.h:1515
>    #7 0x7f18ab855e25 in mozilla::dom::NodeBinding::insertBefore(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) objdir-ff-asan/dom/bindings/NodeBinding.cpp:477
>    #8 0x7f18ab84d7fe in mozilla::dom::NodeBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan/dom/bindings/NodeBinding.cpp:1230
>    #9 0x7f18ad83247a in native js/src/jscntxtinlines.h:218
>    #10 0x7f18ad83247a in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:478
>    #11 0x7f18ad8257f7 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2454
>    #12 0x7f18ad813ff1 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:435
>    #13 0x7f18ad8346fb in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:619
>    #14 0x7f18ad834a56 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:655
>    #15 0x7f18ada25f6d in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:4897
>    #16 0x7f18a961f8eb in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions&, JS::Value*, void**) dom/base/nsJSUtils.cpp:280
>    #17 0x7f18a960d71f in nsJSContext::EvaluateString(nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, bool, JS::Value*, void**) dom/base/nsJSEnvironment.cpp:994
>    #18 0x7f18a95cb7a4 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:10489
>    #19 0x7f18a95aff22 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:10733
>    #20 0x7f18a95ca9bc in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10980
>    #21 0x7f18abff59d1 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:546
>    #22 0x7f18abff6076 in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:630
>    #23 0x7f18abfebb29 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
>    #24 0x7f18abf13601 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:238
>    #25 0x7f18aacaaf30 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:116
>    #26 0x7f18ac105363 in RunInternal ipc/chromium/src/base/message_loop.cc:220
>    #27 0x7f18ac105363 in RunHandler ipc/chromium/src/base/message_loop.cc:213
>    #28 0x7f18ac105363 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:187
>    #29 0x7f18aaa94e2c in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:161
>    #30 0x7f18aa48318e in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:269
>    #31 0x7f18a78842f0 in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3869
>    #32 0x7f18a788522a in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3937
>    #33 0x7f18a788615b in XRE_main toolkit/xre/nsAppRunner.cpp:4139
>    #34 0x45524d in do_main browser/app/nsBrowserApp.cpp:275
>    #35 0x45524d in main browser/app/nsBrowserApp.cpp:635
>    #36 0x7f18b6b4976c in
>    #37 0x45483c in _start
>0x6160020b5520 is located 32 bytes inside of 144-byte region [0x6160020b5500,0x6160020b5590)
>freed by thread T0 here:
>    #0 0x441785 in free _asan_rtl_
>    #1 0x7f18ac012144 in SnowWhiteKiller::~SnowWhiteKiller() xpcom/base/nsCycleCollector.cpp:1993
>    #2 0x7f18ac00967e in nsCycleCollector_doDeferredDeletion() xpcom/base/nsCycleCollector.cpp:3153
>    #3 0x7f18abfebb29 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:622
>    #4 0x7f18abf13601 in NS_ProcessNextEvent(nsIThread*, bool) objdir-ff-asan/xpcom/build/nsThreadUtils.cpp:238
>    #5 0x7f18aacaaf51 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:81
>    #6 0x7f18ac105363 in RunInternal ipc/chromium/src/base/message_loop.cc:220
>    #7 0x7f18ac105363 in RunHandler ipc/chromium/src/base/message_loop.cc:213
>    #8 0x7f18ac105363 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:187
>    #9 0x7f18aaa94e2c in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:161
>    #10 0x7f18a788522a in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3937
>    #11 0x7f18a788615b in XRE_main toolkit/xre/nsAppRunner.cpp:4139
>    #12 0x45524d in do_main browser/app/nsBrowserApp.cpp:275
>    #13 0x45524d in main browser/app/nsBrowserApp.cpp:635
>    #14 0x7f18b6b4976c in
>previously allocated by thread T0 here:
>    #0 0x4418c5 in malloc _asan_rtl_
>    #1 0x7f18b7f044e8 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
>Shadow bytes around the buggy address:
>  0x0c2c8040ea50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  0x0c2c8040ea60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040ea70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040ea80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040ea90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>=>0x0c2c8040eaa0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
>  0x0c2c8040eab0: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040eac0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040ead0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040eae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>  0x0c2c8040eaf0: 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
>==19928==ABORTING
>
>

Comment 1

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

Couldn't reproduce in a non-ASAN build. But based on the stack trace this might be 
regression from bug 818976.

Comment 2

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

Olli how would you rate this one?

Comment 3

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

I am also unable to reproduce on m-c ASan build from 2013-09-11.

Comment 4

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

Stack trace hints sg-critical

Comment 5

[Abhishek Arya]

(In reply to Matt Wobensmith from comment #3)
> I am also unable to reproduce on m-c ASan build from 2013-09-11.

I just retested using trunk build. I can reproduce it very easily by loading testcase from command line. Did you forget to install the fuzzPriv extension https://www.squarefree.com/extensions/domFuzzLite3.xpi ?

Comment 6

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

Hi Matt just making sure comment 5 is on your radar.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Possible fix

HTMLTemplateElement needs to make sure that it calls SetHost(nullptr) on its mContent member, but if we get unlinked, then it's possible for us to null out mContent without calling SetHost first.

Comment 8

[Andrew McCreight [:mccr8]]

Comment on attachment 808605 [details] [diff] [review]
Possible fix

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

Fun.  Thanks for fixing this.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 808605 [details] [diff] [review]
Possible fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Moderate difficulty: it's clear that this has to do with template elements and their host. A little searching will show that the host will be the document fragment. The hardest part will be to make the jump to causing the CC to unlink (and collect) the template and then using the document fragment.

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

Which older supported branches are affected by this flaw?

The code was checked in for Firefox 22.

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

This patch applies to all branches.

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

This patch is very safe, I don't see how it could cause any regressions off the top of my head (I manually expanded a macro and added a single, safe function call).

Comment 10

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

Comment on attachment 808605 [details] [diff] [review]
Possible fix

sec-approval+ for trunk. This should be safe enough to take on Aurora and Beta, I expect.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/331a8e2fa56b

Comment 12

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

Comment on attachment 808605 [details] [diff] [review]
Possible fix

Marking this for Aurora and Beta approval since it is in and we plussed it for branch tracking, which generally indicates release management approval. :-)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/331a8e2fa56b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad676365b814
https://hg.mozilla.org/releases/mozilla-beta/rev/39c1e363005c

Al, you marked firefox24 as wontfix, but should this be considered for esr24?

Comment 15

[Andrew McCreight [:mccr8]]

Yeah, good catch, this should go on ESR-17, pending approval or whatever.

Comment 16

[Andrew McCreight [:mccr8]]

ESR-24, rather.  My fingers are too trained to type "-17" once I type ESR, it would seem.

Comment 17

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

Yes, the "wontfix" for Firefox 24 doesn't apply to ESR24. That flag simply means "we shipped 24 and this wasn't fixed." 

This should be considered for 24.

Comment 18

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

I never was able to reproduce the crash. I definitely have fuzzPriv installed with ASan builds from around and before 2013-09-13.

Abhishek, can you verify this crash has been fixed, on any branch? Doing so would give us more confidence. Thank you.

Comment 19

[Abhishek Arya]

tested on trunk, it is fixed.

Comment 20

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

Thanks Abhishek!

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/c19709803a23

Comment 22

[Ryan VanderMeulen [:RyanVM]]

And a bustage fix with smaug's blessing.
https://hg.mozilla.org/releases/mozilla-esr24/rev/4fef25ef9988