849727 - Heap-use-after-free in mozilla::ResetDir

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Still reproduces after fix in https://bugzilla.mozilla.org/show_bug.cgi?id=845093.

>==22902== ERROR: AddressSanitizer: heap-use-after-free on address 0x601800298dac at pc 0x7fbef7fbfd9c bp 0x7fff0bf4c750 sp 0x7fff0bf4c748
>READ of size 4 at 0x601800298dac thread T0
>    #0 0x7fbef7fbfd9b in nsINode::GetBoolFlag(nsINode::BooleanFlag) const content/base/public/nsINode.h:1357
>    #1 0x7fbef9c96533 in nsINode::HasTextNodeDirectionalityMap() const ../../../dist/include/nsINode.h:1440
>    #2 0x7fbef9c95149 in mozilla::nsTextNodeDirectionalityMap::RemoveElementFromMap(nsINode*, mozilla::dom::Element*) content/base/src/DirectionalityUtils.cpp:524
>    #3 0x7fbef9c9c40f in mozilla::ResetDir(mozilla::dom::Element*) content/base/src/DirectionalityUtils.cpp:955
>    #4 0x7fbefa2655d7 in mozilla::dom::Element::UnbindFromTree(bool, bool) content/base/src/Element.cpp:1364
>    #5 0x7fbefb10ed21 in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:655
>    #6 0x7fbefb1353de in nsGenericHTMLFormElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:2332
>    #7 0x7fbefa2658ae in mozilla::dom::Element::UnbindFromTree(bool, bool) content/base/src/Element.cpp:1377
>    #8 0x7fbefb10ed21 in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:655
>    #9 0x7fbefb275ce6 in mozilla::dom::HTMLBodyElement::UnbindFromTree(bool, bool) content/html/content/src/HTMLBodyElement.cpp:357
>    #10 0x7fbefa2658ae in mozilla::dom::Element::UnbindFromTree(bool, bool) content/base/src/Element.cpp:1377
>    #11 0x7fbefb10ed21 in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:655
>    #12 0x7fbefba7e815 in mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) content/html/content/src/HTMLSharedElement.cpp:305
>    #13 0x7fbefa3340c9 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) content/base/src/nsINode.cpp:1398
>    #14 0x7fbefa03c90f in nsDocument::RemoveChildAt(unsigned int, bool) content/base/src/nsDocument.cpp:3610
>    #15 0x7fbefa31c785 in nsINode::RemoveChild(nsINode&, mozilla::ErrorResult&) content/base/src/nsINode.cpp:461
>    #16 0x7fbf049b7b3f in mozilla::dom::NodeBinding::removeChild(JSContext*, JS::Handle<JSObject*>, nsINode*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:736
>    #17 0x7fbf0497f90f in mozilla::dom::NodeBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:1433
>    #18 0x7fbf0dcb1c9f in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:327
>    #19 0x7fbf0dc7b6c9 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2361
>    #20 0x7fbf0dbf2ced in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:340
>    #21 0x7fbf0dcbe981 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/jsinterp.cpp:530
>    #22 0x7fbf0dcc0604 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/jsinterp.cpp:569
>    #23 0x7fbf0d4bfb79 in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:5529
>    #24 0x7fbefc7574ee in nsJSContext::EvaluateString(nsAString_internal const&, JSObject&, JS::CompileOptions&, bool, JS::Value*) dom/base/nsJSEnvironment.cpp:1290
>    #25 0x7fbefc923771 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:9956
>    #26 0x7fbefc8d8c82 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:10208
>    #27 0x7fbefc921591 in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10477
>    #28 0x7fbf060f3434 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:498
>    #29 0x7fbf060f48ad in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:589
>    #30 0x7fbf060ba057 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:627
>    #31 0x7fbf05d67572 in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
>    #32 0x7fbf02885369 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
>    #33 0x7fbf063804eb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:216
>    #34 0x7fbf0638033e in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:209
>    #35 0x7fbf06380229 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:183
>    #36 0x7fbf01d0af4e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
>    #37 0x7fbf00949fa0 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:288
>    #38 0x7fbef662811c in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3880
>    #39 0x7fbef662d7c9 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3947
>    #40 0x7fbef6630285 in XRE_main toolkit/xre/nsAppRunner.cpp:4150
>    #41 0x427f37 in do_main(int, char**, nsIFile*) browser/app/nsBrowserApp.cpp:228
>    #42 0x4250f1 in main browser/app/nsBrowserApp.cpp:529
>    #43 0x7fbf174ff76c in
>    #44 0x424864 in
>0x601800298dac is located 44 bytes inside of 120-byte region [0x601800298d80,0x601800298df8)
>freed by thread T0 here:
>    #0 0x4186d2 in __interceptor_free
>    #1 0x7fbf1851b77e in moz_free memory/mozalloc/mozalloc.cpp:48
>    #2 0x7fbefa57d2b9 in nsTextNode::~nsTextNode() ../../../dist/include/mozilla/mozalloc.h:225
>    #3 0x7fbefa44cac5 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258
>    #4 0x7fbefa2daf3a in nsGenericDOMDataNode::Release() content/base/src/nsGenericDOMDataNode.cpp:116
>    #5 0x7fbefa57e055 in nsTextNode::Release() content/base/src/nsTextNode.cpp:120
>    #6 0x7fbef65f4680 in nsCOMPtr_base::~nsCOMPtr_base() objdir-ff-asan-sym/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsCOMPtr.h:410
>    #7 0x7fbef8255261 in nsCOMPtr<nsIContent>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:449
>    #8 0x7fbef8254f5e in nsCOMPtr<nsIContent>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:449
>    #9 0x7fbefa6dd8bf in mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) content/base/src/FragmentOrElement.cpp:927
>    #10 0x7fbefb3da7da in mozilla::dom::HTMLFieldSetElement::RemoveChildAt(unsigned int, bool) content/html/content/src/HTMLFieldSetElement.cpp:218
>    #11 0x7fbefa2848bb in mozilla::dom::Element::SetInnerHTML(nsAString_internal const&, mozilla::ErrorResult&) content/base/src/Element.cpp:3357
>    #12 0x7fbf03ddf956 in mozilla::dom::ElementBinding::set_innerHTML(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:1760
>    #13 0x7fbf03dc6ad4 in mozilla::dom::ElementBinding::genericSetter(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2115
>    #14 0x7fbf0dcb1c9f in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:327
>    #15 0x7fbf0d5c5a66 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:135
>    #16 0x7fbf0dcb7489 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:430
>    #17 0x7fbf0dcbd58f in js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:503
>    #18 0x7fbf0df5072b in js::Shape::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, bool, JS::MutableHandle<JS::Value>) js/src/vm/Shape-inl.h:309
>    #19 0x7fbf0df9702a in js::baseops::SetPropertyHelper(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<long>, unsigned int, JS::MutableHandle<JS::Value>, int) js/src/jsobj.cpp:4122
>    #20 0x7fbf0dcf200b in js::SetPropertyOperation(JSContext*, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>) js/src/jsinterpinlines.h:370
>    #21 0x7fbf0dc6cb98 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2252
>    #22 0x7fbf0dbf2ced in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:340
>    #23 0x7fbf0dcb2348 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jsinterp.cpp:397
>    #24 0x7fbf0d5c5a66 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:135
>    #25 0x7fbf0dcb7489 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:430
>    #26 0x7fbf0d4cc1d2 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) js/src/jsapi.cpp:5713
>    #27 0x7fbeffbd93e8 in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJSClass.cpp:1433
>    #28 0x7fbeffb7de6c in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) js/xpconnect/src/XPCWrappedJS.cpp:579
>    #29 0x7fbf061d99e4 in PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
>previously allocated by thread T0 here:
>    #0 0x4187b2 in __interceptor_malloc
>    #1 0x7fbf1851b8c5 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
>    #2 0x7fbefa57cb78 in NS_NewTextNode(nsIContent**, nsNodeInfoManager*) ../../../dist/include/mozilla/mozalloc.h:201
>    #3 0x7fbefdb70066 in nsHtml5TreeOperation::AppendText(unsigned short const*, unsigned int, nsIContent*, nsHtml5TreeOpExecutor*) parser/html/nsHtml5TreeOperation.cpp:164
>    #4 0x7fbefdb7a500 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) parser/html/nsHtml5TreeOperation.cpp:457
>    #5 0x7fbefdb9662d in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:557
>    #6 0x7fbefdbd0bb2 in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:125
>    #7 0x7fbf060ba057 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:627
>    #8 0x7fbf05d67572 in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
>    #9 0x7fbf02885369 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
>    #10 0x7fbf063804eb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:216
>    #11 0x7fbf0638033e in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:209
>    #12 0x7fbf06380229 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:183
>    #13 0x7fbf01d0af4e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
>    #14 0x7fbf00949fa0 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:288
>    #15 0x7fbef662811c in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3880
>    #16 0x7fbef662d7c9 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3947
>    #17 0x7fbef6630285 in XRE_main toolkit/xre/nsAppRunner.cpp:4150
>    #18 0x427f37 in do_main(int, char**, nsIFile*) browser/app/nsBrowserApp.cpp:228
>    #19 0x4250f1 in main browser/app/nsBrowserApp.cpp:529
>    #20 0x7fbf174ff76c in
>Shadow bytes around the buggy address:
>  0x0c038004b160: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>  0x0c038004b170: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
>  0x0c038004b180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
>  0x0c038004b190: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>  0x0c038004b1a0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
>=>0x0c038004b1b0: fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fa
>  0x0c038004b1c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>  0x0c038004b1d0: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
>  0x0c038004b1e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
>  0x0c038004b1f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>  0x0c038004b200: fd fd fd fd fd fd fd fd 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 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
>==22902== ABORTING
>
>

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

It's bad that this issue didn't surface in testing already: I'll open a separate bug to add some unit tests that catch it and aren't associated with this security bug.

Comment 2

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

Matt agreed to help find affected branches but Simon feel free to beat him with that info :)

Comment 3

[(no longer active)]

Comment on attachment 724820 [details] [diff] [review]
Patch

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

::: content/base/src/DirectionalityUtils.cpp
@@ +500,5 @@
> +    nsINode* newTextNode = nullptr;
> +    if (rootNode->HasDirAuto()) {
> +      newTextNode = WalkDescendantsSetDirectionFromText(rootNode, true,
> +                                                        oldTextNode);
> +    }

Does it make sense for us to add a MOZ_ASSERT in WalkDescendantsSetDirectionFromText to guarantee that HasDirAutio() will be true for the argument that they receive?

Comment 4

[Daniel Veditz [:dveditz]]

Assuming this bug was introduced by DirAuto we'll need this fix in Firefox 20 as well, but not earlier.

Comment 5

[Mats Palmgren (inactive)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> Assuming this bug was introduced by DirAuto we'll need this fix in Firefox
> 20 as well, but not earlier.

I think DirAuto was backed out for FF20 in bug 850069.

Comment 6

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/74847b983bc8

Comment 7

[Mats Palmgren (inactive)]

Looks like this was merged to m-c a few days ago:
http://hg.mozilla.org/mozilla-central/rev/74847b983bc8

Comment 8

[bhavana bajaj [:bajaj]]

(In reply to Simon Montagu from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/74847b983bc8

Can you request approval for Aurora as Fx21 is affected ?

Comment 9

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

I'm assuming that we have all the info we need on affected branches. If not, reset needinfo flag to me.

Comment 11

[Simon Montagu :smontagu]

Comment on attachment 724820 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 548206 or one of its followups
User impact if declined: sec-critical vulnerability plus incorrect behaviour when resetting direction dynamically
Testing completed (on m-c, etc.): baked on m-c since 2013-03-24
Risk to taking this patch (and alternatives if risky): believed minimal
String or IDL/UUID changes made by this patch: none

Comment 12

[(no longer active)]

This should be on Aurora too, IINM.

Comment 13

[Simon Montagu :smontagu]

Yeah, my mistake

Comment 14

[Alex Keybl [:akeybl]]

(In reply to Mats Palmgren [:mats] from comment #5)
> (In reply to Daniel Veditz [:dveditz] from comment #4)
> > Assuming this bug was introduced by DirAuto we'll need this fix in Firefox
> > 20 as well, but not earlier.
> 
> I think DirAuto was backed out for FF20 in bug 850069.

Marking FF20 as unaffected.

Comment 15

[Simon Montagu :smontagu]

https://hg.mozilla.org/releases/mozilla-beta/rev/681bd5e6a343

Comment 16

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7db0065a2143

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7db0065a2143