849732 - Heap-use-after-free in mozilla::WalkAncestorsResetAutoDirection

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

>==23146== ERROR: AddressSanitizer: heap-use-after-free on address 0x60180029916c at pc 0x7fad34170d9c bp 0x7fff27b864f0 sp 0x7fff27b864e8
>READ of size 4 at 0x60180029916c thread T0
>    #0 0x7fad34170d9b in nsINode::GetBoolFlag(nsINode::BooleanFlag) const content/base/public/nsINode.h:1357
>    #1 0x7fad35e47533 in nsINode::HasTextNodeDirectionalityMap() const ../../../dist/include/nsINode.h:1440
>    #2 0x7fad35e46149 in mozilla::nsTextNodeDirectionalityMap::RemoveElementFromMap(nsINode*, mozilla::dom::Element*) content/base/src/DirectionalityUtils.cpp:524
>    #3 0x7fad35e45b7e in mozilla::WalkAncestorsResetAutoDirection(mozilla::dom::Element*, bool) content/base/src/DirectionalityUtils.cpp:629
>    #4 0x7fad35e4d1a2 in mozilla::SetDirOnBind(mozilla::dom::Element*, nsIContent*) content/base/src/DirectionalityUtils.cpp:939
>    #5 0x7fad36411272 in mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) content/base/src/Element.cpp:1172
>    #6 0x7fad372bed84 in nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) content/html/content/src/nsGenericHTMLElement.cpp:602
>    #7 0x7fad36412531 in mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) content/base/src/Element.cpp:1216
>    #8 0x7fad372bed84 in nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) content/html/content/src/nsGenericHTMLElement.cpp:602
>    #9 0x7fad364e2e2d in nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) content/base/src/nsINode.cpp:1336
>    #10 0x7fad3688e4e3 in mozilla::dom::FragmentOrElement::InsertChildAt(nsIContent*, unsigned int, bool) content/base/src/FragmentOrElement.cpp:915
>    #11 0x7fad364eab64 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) content/base/src/nsINode.cpp:1940
>    #12 0x7fad3643a3fc in nsINode::InsertBefore(nsINode&, nsINode*, mozilla::ErrorResult&) ../../dist/include/nsINode.h:1550
>    #13 0x7fad36436edb in nsINode::AppendChild(nsINode&, mozilla::ErrorResult&) ../../dist/include/nsINode.h:1554
>    #14 0x7fad40b867ef in mozilla::dom::NodeBinding::appendChild(JSContext*, JS::Handle<JSObject*>, nsINode*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:588
>    #15 0x7fad40b3090f in mozilla::dom::NodeBinding::genericMethod(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:1433
>    #16 0x7fad49e62c9f in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:327
>    #17 0x7fad49e2c6c9 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2361
>    #18 0x7fad49da3ced in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:340
>    #19 0x7fad49e6f981 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/jsinterp.cpp:530
>    #20 0x7fad49e71604 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/jsinterp.cpp:569
>    #21 0x7fad49670b79 in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:5529
>    #22 0x7fad389084ee in nsJSContext::EvaluateString(nsAString_internal const&, JSObject&, JS::CompileOptions&, bool, JS::Value*) dom/base/nsJSEnvironment.cpp:1290
>    #23 0x7fad38ad4771 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:9956
>    #24 0x7fad38a89c82 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:10208
>    #25 0x7fad38ad2591 in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10477
>    #26 0x7fad422a4434 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:498
>    #27 0x7fad422a58ad in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:589
>    #28 0x7fad4226b057 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:627
>    #29 0x7fad41f18572 in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
>    #30 0x7fad3ea36934 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:117
>    #31 0x7fad425314eb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:216
>    #32 0x7fad4253133e in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:209
>    #33 0x7fad42531229 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:183
>    #34 0x7fad3debbf4e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
>    #35 0x7fad3cafafa0 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:288
>    #36 0x7fad327d911c in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3880
>    #37 0x7fad327de7c9 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3947
>    #38 0x7fad327e1285 in XRE_main toolkit/xre/nsAppRunner.cpp:4150
>    #39 0x427f37 in do_main(int, char**, nsIFile*) browser/app/nsBrowserApp.cpp:228
>    #40 0x4250f1 in main browser/app/nsBrowserApp.cpp:529
>    #41 0x7fad536b076c in
>    #42 0x424864 in
>0x60180029916c is located 44 bytes inside of 120-byte region [0x601800299140,0x6018002991b8)
>freed by thread T0 here:
>    #0 0x4186d2 in __interceptor_free
>    #1 0x7fad546cc77e in moz_free memory/mozalloc/mozalloc.cpp:48
>    #2 0x7fad3672e2b9 in nsTextNode::~nsTextNode() ../../../dist/include/mozilla/mozalloc.h:225
>    #3 0x7fad365fdac5 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258
>    #4 0x7fad3648bf3a in nsGenericDOMDataNode::Release() content/base/src/nsGenericDOMDataNode.cpp:116
>    #5 0x7fad3672f055 in nsTextNode::Release() content/base/src/nsTextNode.cpp:120
>    #6 0x7fad327a5680 in nsCOMPtr_base::~nsCOMPtr_base() objdir-ff-asan-sym/media/webrtc/signaling/signaling_ecc/../../../../dist/include/nsCOMPtr.h:410
>    #7 0x7fad34406261 in nsCOMPtr<nsIContent>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:449
>    #8 0x7fad34405f5e in nsCOMPtr<nsIContent>::~nsCOMPtr() ../../dist/include/nsCOMPtr.h:449
>    #9 0x7fad3688e8bf in mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) content/base/src/FragmentOrElement.cpp:927
>    #10 0x7fad364358bb in mozilla::dom::Element::SetInnerHTML(nsAString_internal const&, mozilla::ErrorResult&) content/base/src/Element.cpp:3357
>    #11 0x7fad3ff90956 in mozilla::dom::ElementBinding::set_innerHTML(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:1760
>    #12 0x7fad3ff77ad4 in mozilla::dom::ElementBinding::genericSetter(JSContext*, unsigned int, JS::Value*) objdir-ff-asan-sym/dom/bindings/ElementBinding.cpp:2115
>    #13 0x7fad49e62c9f in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:327
>    #14 0x7fad49776a66 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) js/src/jsinterp.h:135
>    #15 0x7fad49e68489 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:430
>    #16 0x7fad49e6e58f in js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value const&, unsigned int, JS::Value*, JS::Value*) js/src/jsinterp.cpp:503
>    #17 0x7fad4a10172b in js::Shape::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, bool, JS::MutableHandle<JS::Value>) js/src/vm/Shape-inl.h:309
>    #18 0x7fad4a14802a 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
>    #19 0x7fad49ea300b in js::SetPropertyOperation(JSContext*, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>) js/src/jsinterpinlines.h:370
>    #20 0x7fad49e1db98 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) js/src/jsinterp.cpp:2252
>    #21 0x7fad49da3ced in js::RunScript(JSContext*, js::StackFrame*) js/src/jsinterp.cpp:340
>    #22 0x7fad49e6f981 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/jsinterp.cpp:530
>    #23 0x7fad49e71604 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/jsinterp.cpp:569
>    #24 0x7fad49670b79 in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) js/src/jsapi.cpp:5529
>    #25 0x7fad389084ee in nsJSContext::EvaluateString(nsAString_internal const&, JSObject&, JS::CompileOptions&, bool, JS::Value*) dom/base/nsJSEnvironment.cpp:1290
>    #26 0x7fad38ad4771 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) dom/base/nsGlobalWindow.cpp:9956
>    #27 0x7fad38a89c82 in nsGlobalWindow::RunTimeout(nsTimeout*) dom/base/nsGlobalWindow.cpp:10208
>    #28 0x7fad38ad2591 in nsGlobalWindow::TimerCallback(nsITimer*, void*) dom/base/nsGlobalWindow.cpp:10477
>    #29 0x7fad422a4434 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:498
>previously allocated by thread T0 here:
>    #0 0x4187b2 in __interceptor_malloc
>    #1 0x7fad546cc8c5 in moz_xmalloc memory/mozalloc/mozalloc.cpp:54
>    #2 0x7fad3672db78 in NS_NewTextNode(nsIContent**, nsNodeInfoManager*) ../../../dist/include/mozilla/mozalloc.h:201
>    #3 0x7fad39d21066 in nsHtml5TreeOperation::AppendText(unsigned short const*, unsigned int, nsIContent*, nsHtml5TreeOpExecutor*) parser/html/nsHtml5TreeOperation.cpp:164
>    #4 0x7fad39d2b500 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) parser/html/nsHtml5TreeOperation.cpp:457
>    #5 0x7fad39d4762d in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:557
>    #6 0x7fad39d81bb2 in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:125
>    #7 0x7fad4226b057 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:627
>    #8 0x7fad41f18572 in NS_ProcessNextEvent_P(nsIThread*, bool) objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
>    #9 0x7fad3ea36369 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:82
>    #10 0x7fad425314eb in MessageLoop::RunInternal() ipc/chromium/src/base/message_loop.cc:216
>    #11 0x7fad4253133e in MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:209
>    #12 0x7fad42531229 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:183
>    #13 0x7fad3debbf4e in nsBaseAppShell::Run() widget/xpwidgets/nsBaseAppShell.cpp:163
>    #14 0x7fad3cafafa0 in nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:288
>    #15 0x7fad327d911c in XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:3880
>    #16 0x7fad327de7c9 in XREMain::XRE_main(int, char**, nsXREAppData const*) toolkit/xre/nsAppRunner.cpp:3947
>    #17 0x7fad327e1285 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 0x7fad536b076c in
>Shadow bytes around the buggy address:
>  0x0c038004b1d0: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
>  0x0c038004b1e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>  0x0c038004b1f0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>  0x0c038004b200: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa
>  0x0c038004b210: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>=>0x0c038004b220: fa fa fa fa fa fa fa fa fd fd fd fd fd[fd]fd fd
>  0x0c038004b230: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
>  0x0c038004b240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
>  0x0c038004b250: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>  0x0c038004b260: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
>  0x0c038004b270: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 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
>==23146== ABORTING
>
>

Comment 1

[Mats Palmgren (inactive)]

Possibly the same root cause as bug 849727.

Comment 2

[Simon Montagu :smontagu]

Attachment: Patch

Bug 829428 "made WalkDescendantsSetDirAuto consistent with SetDirOnBind", but it should be less consistent, as the added comment explains.

The second hunk is surprising -- have we really been setting the wrong flag here since forever? As in bug 849727, I'll try to come up with some tests that pinpoint an error in behaviour caused by this issue.

Comment 3

[(no longer active)]

(In reply to Simon Montagu from comment #2)
> The second hunk is surprising -- have we really been setting the wrong flag
> here since forever?

I guess so :(

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

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/426bf346fe1d

Comment 6

[Mats Palmgren (inactive)]

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

Comment 7

[bhavana bajaj [:bajaj]]

(In reply to Simon Montagu from comment #5)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/426bf346fe1d

Looks like this needs to land on Fx21 branch as well & needs aurora approval here.Can you please nominate the patch if ready?

Comment 9

[Simon Montagu :smontagu]

Comment on attachment 724844 [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 10

[(no longer active)]

Shouldn't this already be on Aurora?

Comment 11

[Simon Montagu :smontagu]

https://hg.mozilla.org/releases/mozilla-beta/rev/ed84088d095a

Fx20 is unaffected because of bug 850069

Comment 12

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8c38ed9f5533

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/8c38ed9f5533