819623 - Heap-use-after-free in mozilla::WalkDescendantsSetDirectionFromText

Reporter

Description

•

12 years ago

Attached file Testcase — Details

This is yet another dir=auto issue and it still reproduces even after applying the patches for bug 815500 and bug 815276. For reproducing this, you need to install the fuzzPriv extension [https://www.squarefree.com/extensions/domFuzzLite3.xpi]

==16967== ERROR: AddressSanitizer: heap-use-after-free on address 0x7f5dc8e9ac80 at pc 0x7f5de3083fc5 bp 0x7fffc7ba6990 sp 0x7fffc7ba6988
READ of size 8 at 0x7f5dc8e9ac80 thread T0
    #0 0x7f5de3083fc4 in nsINode::GetFirstChild() const src/../../dist/include/nsINode.h:1034
    #1 0x7f5de4a7364d in mozilla::WalkDescendantsSetDirectionFromText(mozilla::dom::Element*, bool, nsINode*) src/content/base/src/DirectionalityUtils.cpp:382
    #2 0x7f5de4a7cc73 in mozilla::nsTextNodeDirectionalityMap::ResetNodeDirection(nsPtrHashKey<mozilla::dom::Element>*, void*) src/content/base/src/DirectionalityUtils.cpp:493
    #3 0x7f5de4a7b48b in nsTHashtable<nsPtrHashKey<mozilla::dom::Element> >::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) src/../../../dist/include/nsTHashtable.h:484
    #4 0x7f5def943875 in PL_DHashTableEnumerate src/objdir-ff-asan-sym/xpcom/build/pldhash.cpp:717
    #5 0x7f5de4a7b162 in nsTHashtable<nsPtrHashKey<mozilla::dom::Element> >::EnumerateEntries(PLDHashOperator (*)(nsPtrHashKey<mozilla::dom::Element>*, void*), void*) src/../../../dist/include/nsTHashtable.h:237
    #6 0x7f5de4a7a7ac in nsCheapSet<nsPtrHashKey<mozilla::dom::Element> >::EnumerateEntries(PLDHashOperator (*)(nsPtrHashKey<mozilla::dom::Element>*, void*), void*) src/../../../dist/include/nsCheapSets.h:74
    #7 0x7f5de4a7c932 in mozilla::nsTextNodeDirectionalityMap::ResetAutoDirection(nsINode*, nsINode*) src/content/base/src/DirectionalityUtils.cpp:509
    #8 0x7f5de4a74a02 in mozilla::nsTextNodeDirectionalityMap::ResetTextNodeDirection(nsINode*, nsINode*) src/content/base/src/DirectionalityUtils.cpp:542
    #9 0x7f5de4a7927e in mozilla::ResetDirectionSetByTextNode(nsTextNode*) src/content/base/src/DirectionalityUtils.cpp:825
    #10 0x7f5de5332a39 in nsTextNode::UnbindFromTree(bool, bool) src/content/base/src/nsTextNode.cpp:183
    #11 0x7f5de502380f in mozilla::dom::Element::UnbindFromTree(bool, bool) src/content/base/src/Element.cpp:1384
    #12 0x7f5de5fcb47a in nsGenericHTMLElement::UnbindFromTree(bool, bool) src/content/html/content/src/nsGenericHTMLElement.cpp:1581
    #13 0x7f5de502380f in mozilla::dom::Element::UnbindFromTree(bool, bool) src/content/base/src/Element.cpp:1384
    #14 0x7f5de5fcb47a in nsGenericHTMLElement::UnbindFromTree(bool, bool) src/content/html/content/src/nsGenericHTMLElement.cpp:1581
    #15 0x7f5de614d0bc in nsHTMLBodyElement::UnbindFromTree(bool, bool) src/content/html/content/src/nsHTMLBodyElement.cpp:348
    #16 0x7f5de502380f in mozilla::dom::Element::UnbindFromTree(bool, bool) src/content/base/src/Element.cpp:1384
    #17 0x7f5de5fcb47a in nsGenericHTMLElement::UnbindFromTree(bool, bool) src/content/html/content/src/nsGenericHTMLElement.cpp:1581
    #18 0x7f5de6982999 in nsHTMLSharedElement::UnbindFromTree(bool, bool) src/content/html/content/src/nsHTMLSharedElement.cpp:434
    #19 0x7f5de50d74a2 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) src/content/base/src/nsINode.cpp:1383
    #20 0x7f5de4e0f1dd in nsDocument::RemoveChildAt(unsigned int, bool) src/content/base/src/nsDocument.cpp:3383
    #21 0x7f5de50be16e in nsINode::RemoveChild(nsINode&, mozilla::ErrorResult&) src/content/base/src/nsINode.cpp:458
    #22 0x7f5def23896c in mozilla::dom::NodeBinding::removeChild(JSContext*, JS::Handle<JSObject*>, nsINode*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:669
    #23 0x7f5def205177 in mozilla::dom::NodeBinding::genericMethod(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/dom/bindings/NodeBinding.cpp:1308
    #24 0x7f5df71a3239 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:364
    #25 0x7f5df7152c54 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2321
    #26 0x7f5df70b411e in js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) src/js/src/jsinterp.cpp:326
    #27 0x7f5df71b07ed in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) src/js/src/jsinterp.cpp:515
    #28 0x7f5df71b24ab in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) src/js/src/jsinterp.cpp:552
    #29 0x7f5df691040d in JS::Evaluate(JSContext*, JS::Handle<JSObject*>, JS::CompileOptions, unsigned short const*, unsigned long, JS::Value*) src/js/src/jsapi.cpp:5605
    #30 0x7f5de7624a93 in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) src/dom/base/nsJSEnvironment.cpp:1524
    #31 0x7f5de77e89b1 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) src/dom/base/nsGlobalWindow.cpp:9703
    #32 0x7f5de779d1ce in nsGlobalWindow::RunTimeout(nsTimeout*) src/dom/base/nsGlobalWindow.cpp:9962
    #33 0x7f5de77e6ac6 in nsGlobalWindow::TimerCallback(nsITimer*, void*) src/dom/base/nsGlobalWindow.cpp:10231
    #34 0x7f5defcf3ae8 in nsTimerImpl::Fire() src/xpcom/threads/nsTimerImpl.cpp:482
    #35 0x7f5defcf4f71 in nsTimerEvent::Run() src/xpcom/threads/nsTimerImpl.cpp:565
    #36 0x7f5defcb7c6e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
    #37 0x7f5def92de0f in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:221
    #38 0x7f5dede408fd in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:117
    #39 0x7f5deff9ff7e in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:215
    #40 0x7f5deff9fdc5 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:208
    #41 0x7f5deff9fcab in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:182
    #42 0x7f5ded240674 in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
    #43 0x7f5debd6d4d2 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:291
    #44 0x7f5de1266d14 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3824
    #45 0x7f5de126c9e9 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3891
    #46 0x7f5de126f760 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4089
    #47 0x40c1a6 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
    #48 0x4099e0 in main src/browser/app/nsBrowserApp.cpp:279
    #49 0x7f5e0165076c in ?? ??:0
0x7f5dc8e9ac80 is located 64 bytes inside of 120-byte region [0x7f5dc8e9ac40,0x7f5dc8e9acb8)
freed by thread T0 here:
    #0 0x4c3830 in __interceptor_free ??:?
    #1 0x7f5dff1de405 in moz_free src/memory/mozalloc/mozalloc.cpp:48
    #2 0x7f5de684f7dd in operator delete(void*) src/../../../../dist/include/mozilla/mozalloc.h:224
    #3 0x7f5de51fca72 in nsNodeUtils::LastRelease(nsINode*) src/content/base/src/nsNodeUtils.cpp:257
    #4 0x7f5de54d2bf7 in mozilla::dom::FragmentOrElement::Release() src/content/base/src/FragmentOrElement.cpp:1681
    #5 0x7f5de684fcc7 in nsHTMLParagraphElement::Release() src/content/html/content/src/nsHTMLParagraphElement.cpp:72
    #6 0x7f5deaae74ac in _ZL17DoDeferredReleaseIP11nsISupportsEvR8nsTArrayIT_24nsTArrayDefaultAllocatorE src/js/xpconnect/src/XPCJSRuntime.cpp:535
    #7 0x7f5deaae6879 in XPCJSRuntime::GCCallback(JSRuntime*, JSGCStatus) src/js/xpconnect/src/XPCJSRuntime.cpp:737
    #8 0x7f5df6ed414d in Collect(JSRuntime*, bool, long, js::JSGCInvocationKind, js::gcreason::Reason) src/js/src/jsgc.cpp:4130
    #9 0x7f5df6ebf869 in js::GC(JSRuntime*, js::JSGCInvocationKind, js::gcreason::Reason) src/js/src/jsgc.cpp:4148
    #10 0x7f5df6e27380 in js::GCForReason(JSRuntime*, js::gcreason::Reason) src/js/src/jsfriendapi.cpp:178
    #11 0x7f5dea9affe2 in nsXPCComponents_Utils::ForceGC() src/js/xpconnect/src/XPCComponents.cpp:4044
    #12 0x7f5defddcd53 in NS_InvokeByIndex_P src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
    #13 0x7f5deac570aa in CallMethodHelper::Invoke() src/js/xpconnect/src/XPCWrappedNative.cpp:3081
    #14 0x7f5deacb61ca in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1488
previously allocated by thread T0 here:
    #0 0x4c38f0 in malloc ??:?
    #1 0x7f5dff1de541 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7f5de684ee7f in operator new(unsigned long) src/../../../../dist/include/mozilla/mozalloc.h:200
    #3 0x7f5de6d7ac4a in CreateHTMLElement(unsigned int, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) src/content/html/document/src/nsHTMLContentSink.cpp:497
    #4 0x7f5de6d7b498 in NS_NewHTMLElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) src/content/html/document/src/nsHTMLContentSink.cpp:480
    #5 0x7f5de51c9364 in NS_NewElement(nsIContent**, already_AddRefed<nsINodeInfo>, mozilla::dom::FromParser) src/content/base/src/nsNameSpaceManager.cpp:192
    #6 0x7f5de4e2a868 in nsDocument::CreateElementNS(nsAString_internal const&, nsAString_internal const&, nsIContent**) src/content/base/src/nsDocument.cpp:4419
    #7 0x7f5dead53a49 in nsIDOMDocument_CreateElementNS(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:1250
    #8 0x7f5df71a3239 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:364
Shadow byte and word:
  0x1febb91d3590: fd
  0x1febb91d3590: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1febb91d3570: 00 00 00 00 00 00 00 00
  0x1febb91d3578: 00 00 00 00 00 fb fb fb
  0x1febb91d3580: fa fa fa fa fa fa fa fa
  0x1febb91d3588: fd fd fd fd fd fd fd fd
=>0x1febb91d3590: fd fd fd fd fd fd fd fd
  0x1febb91d3598: fa fa fa fa fa fa fa fa
  0x1febb91d35a0: fa fa fa fa fa fa fa fa
  0x1febb91d35a8: fd fd fd fd fd fd fd fd
  0x1febb91d35b0: fd fd fd fd fd fd fd fd
Stats: 253M malloced (239M for red zones) by 410732 calls
Stats: 47M realloced by 24498 calls
Stats: 228M freed by 287460 calls
Stats: 194M really freed by 238270 calls
Stats: 231M (59343 full pages) mmaped in 440 calls
  mmaps   by size class: 7:147420; 8:45034; 9:13299; 10:5110; 11:6120; 12:1280; 13:768; 14:576; 15:224; 16:736; 17:460; 18:36; 19:35; 20:21;
  mallocs by size class: 7:266840; 8:85637; 9:23728; 10:8763; 11:16492; 12:2444; 13:1779; 14:1618; 15:418; 16:1508; 17:1374; 18:69; 19:40; 20:22;
  frees   by size class: 7:184040; 8:57968; 9:17516; 10:5629; 11:14476; 12:1607; 13:1572; 14:1448; 15:293; 16:1441; 17:1356; 18:57; 19:38; 20:19;
  rfrees  by size class: 7:158177; 8:46664; 9:10864; 10:3883; 11:12401; 12:1234; 13:1026; 14:1321; 15:223; 16:1050; 17:1327; 18:55; 19:38; 20:7;
Stats: malloc large: 3431 small slow: 4987
==16967== ABORTING

Abhishek Arya

Reporter

Comment 1

•

12 years ago

Simon, since you are already diving this in code, can you fix a null pointer crash along the way. Basically I am seeing hundreds of null pointer crashes in my fuzzing caused by GetDirectionalityMap(aTextNode) returning null in various functions in DirectionalityUtils.cpp.

Daniel Veditz [:dveditz]

Comment 2

•

12 years ago

more fallout from bug 548206?

Component: General → Layout: Text

Keywords: csec-uaf, regressionwindow-wanted, sec-critical

Product: Firefox → Core

Jesse Ruderman

Comment 3

•

12 years ago

Assertion failure: aEntry->GetKey()->IsElement() (Must be an Element), at content/base/src/DirectionalityUtils.cpp:484

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

Comment 4

•

12 years ago

Hoping Simon can own this (?) and maybe comment on comment 2 and comment 3.

Assignee: nobody → smontagu

Simon Montagu :smontagu

Assignee

Comment 5

•

12 years ago

Attached patch Patch (obsolete) — Details — Splinter Review

Attachment #694559 - Flags: review?(peterv)

Simon Montagu :smontagu

Assignee

Updated

•

12 years ago

Blocks: 824719

Al Billings [:abillings - ex-MoCo]

Comment 6

•

12 years ago

Peter, any timeline on review here?

Whiteboard: [asan]

Peter Van der Beken [:peterv]

Comment 7

•

12 years ago

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

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

::: content/base/src/DirectionalityUtils.cpp
@@ +902,5 @@
>  }
>  
> +void ResetDir(mozilla::dom::Element* aElement)
> +{
> +  if (aElement->HasDirAuto() || aElement->HasDirAutoSet()) {

Why do we need both conditions? I thought the dirAutoSetBy property can only be set if HasDirAutoSet() returns true. Is that not the case?

Simon Montagu :smontagu

Assignee

Comment 8

•

12 years ago

Attached patch Patch v.2 — Details — Splinter Review

Yes, the double condition was an error.

Attachment #694559 - Attachment is obsolete: true

Attachment #694559 - Flags: review?(peterv)

Attachment #697601 - Flags: review?(peterv)

Peter Van der Beken [:peterv]

Comment 9

•

12 years ago

Comment on attachment 697601 [details] [diff] [review]
Patch v.2

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

::: content/base/src/DirectionalityUtils.cpp
@@ +920,5 @@
>  }
>  
> +void ResetDir(mozilla::dom::Element* aElement)
> +{
> +  if (aElement->HasDirAuto() || aElement->HasDirAutoSet()) {

With the check for HasDirAuto() actually removed :-).

@@ +923,5 @@
> +{
> +  if (aElement->HasDirAuto() || aElement->HasDirAutoSet()) {
> +    nsINode* setByNode =
> +      static_cast<nsINode*>(aElement->GetProperty(nsGkAtoms::dirAutoSetBy));
> +    if (setByNode) {

The null-check shouldn't be needed afaict.

Attachment #697601 - Flags: review?(peterv) → review+

Simon Montagu :smontagu

Assignee

Comment 10

•

12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/f80f9b1e217b

(no longer active)

Comment 11

•

12 years ago

https://hg.mozilla.org/mozilla-central/rev/f80f9b1e217b

Status: NEW → RESOLVED

Closed: 12 years ago

Resolution: --- → FIXED

Target Milestone: --- → mozilla20

Matt Wobensmith [:mwobensmith][:matt:]

Comment 12

•

11 years ago

Confirmed crash on central, ASan build 2012-12-07.
Confirmed fixed on central, ASan build 2012-01-10.
Confirmed fixed on central, release build 2012-01-10.

Status: RESOLVED → VERIFIED

Daniel Veditz [:dveditz]

Comment 13

•

11 years ago

If this is NOT a regression from the DirAuto landing then please clear the errant "unaffected" status flags.

Blocks: DirAuto

status-b2g18: --- → unaffected

status-firefox19: --- → unaffected

status-firefox20: --- → fixed

status-firefox21: --- → fixed

status-firefox-esr17: --- → unaffected

Keywords: regressionwindow-wanted → regression

Daniel Veditz [:dveditz]

Comment 14

•

11 years ago

in-testsuite+ since the test from bug 824719 was checked in for this.

Flags: sec-bounty?

Flags: in-testsuite+

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

Flags: sec-bounty? → sec-bounty+

Al Billings [:abillings - ex-MoCo]

Comment 15

•

11 years ago

Attached file Bounty Awarded $3000 [paid] — Details

Kyle Huey (Exited; not receiving bugmail, old account, do not use)

Updated

•

11 years ago

Depends on: 831287

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

Whiteboard: [asan] → [asan][adv-main20+]

Al Billings [:abillings - ex-MoCo]

Updated

•

11 years ago

Whiteboard: [asan][adv-main20+] → [asan][adv-main20-]

Daniel Veditz [:dveditz]

Updated

•

11 years ago

Group: core-security

Testcase 12 years ago Abhishek Arya 1.57 KB, text/html		Details
Patch 12 years ago Simon Montagu :smontagu 3.47 KB, patch		Details \| Diff \| Splinter Review
Patch v.2 12 years ago Simon Montagu :smontagu 3.52 KB, patch	peterv : review+	Details \| Diff \| Splinter Review