838489 - Remaining dir=auto use after frees

Status: RESOLVED FIXED

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

Description

[Abhishek Arya]

Attachment: Testcase 1

Reproduces on trunk.

Stack 1
=================================================================
==1171== ERROR: AddressSanitizer: heap-use-after-free on address 0x7f2bf291894c at pc 0x7f2c0a8d8a7c bp 0x7fff46dbc300 sp 0x7fff46dbc2f8
READ of size 4 at 0x7f2bf291894c thread T0
    #0 0x7f2c0a8d8a7b in mozilla::WalkAncestorsResetAutoDirection(mozilla::dom::Element*, bool) ../../../dist/include/nsINode.h:1348
    #1 0x7f2c093814c2 in nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) content/html/content/src/nsGenericHTMLElement.cpp:603
    #2 0x7f2c0b8bad11 in nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) content/base/src/nsINode.cpp:1325
    #3 0x7f2c0b8c3066 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) content/base/src/nsINode.cpp:1929
    #4 0x7f2c0b8bfe65 in nsINode::ReplaceOrInsertBefore(bool, nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) content/base/src/nsINode.cpp:1957
    #5 0x7f2c0b9a9ead in nsEditor::SplitNodeImpl(nsIDOMNode*, int, nsIDOMNode*, nsIDOMNode*) editor/libeditor/base/nsEditor.cpp:2833
    #6 0x7f2c086e7944 in SplitElementTxn::DoTransaction() editor/libeditor/base/SplitElementTxn.cpp:99
    #7 0x7f2c0947b5e5 in nsTransactionManager::BeginTransaction(nsITransaction*, nsISupports*) editor/txmgr/src/nsTransactionManager.cpp:782
0x7f2bf291894c is located 44 bytes inside of 120-byte region [0x7f2bf2918920,0x7f2bf2918998)
freed by thread T0 here:
    #0 0x40f082 in __interceptor_free
    #1 0x7f2c07c3d3a1 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258
    #2 0x7f2c093801d3 in nsGenericDOMDataNode::Release() content/base/src/nsGenericDOMDataNode.cpp:115
previously allocated by thread T0 here:
    #0 0x40f162 in __interceptor_malloc
Shadow bytes around the buggy address:
  0x1fe57e5230d0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x1fe57e5230e0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1fe57e5230f0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x1fe57e523100: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1fe57e523110: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x1fe57e523120: fa fa fa fa fd fd fd fd fd[fd]fd fd fd fd fd fd
  0x1fe57e523130: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x1fe57e523140: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fe57e523150: 00 00 00 00 00 00 00 00 00 00 00 fb fa fa fa fa
  0x1fe57e523160: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fe57e523170: 00 00 fb fb 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 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
Stats: 229M malloced (201M for red zones) by 391376 calls
Stats: 34M realloced by 15150 calls
Stats: 188M freed by 158173 calls
Stats: 48M really freed by 90960 calls
Stats: 416M (416M-0M) mmaped; 104 maps, 0 unmaps
  mmaps   by size class: 6:196605; 7:163835; 8:32766; 9:24573; 10:12285; 11:12282; 12:3072; 13:1536; 14:1280; 15:384; 16:832; 17:1312; 18:48; 19:40; 20:24;
  mallocs by size class: 6:154639; 7:145299; 8:40409; 9:20729; 10:9654; 11:11439; 12:3117; 13:1761; 14:1393; 15:354; 16:1074; 17:1411; 18:37; 19:39; 20:21;
  frees   by size class: 6:65016; 7:31982; 8:24145; 9:13948; 10:6773; 11:9428; 12:1971; 13:1301; 14:1237; 15:240; 16:654; 17:1395; 18:28; 19:36; 20:19;
  rfrees  by size class: 6:46415; 7:17122; 8:14144; 9:3287; 10:1795; 11:6215; 12:489; 13:438; 14:332; 15:116; 16:436; 17:160; 18:6; 19:4; 20:1;
Stats: malloc large: 1508 small slow: 1738
Stats: StackDepot: 0 ids; 0M mapped
==1171== ABORTING

Stack2
=================================================================
==28337== ERROR: AddressSanitizer: heap-use-after-free on address 0x7fb58cfd344c at pc 0x7fb588734076 bp 0x7fffae5604e0 sp 0x7fffae5604d8
READ of size 4 at 0x7fb58cfd344c thread T0
    #0 0x7fb588734075 in mozilla::ResetDir(mozilla::dom::Element*) ../../../dist/include/nsINode.h:1348
    #1 0x7fb58628c77c in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:656
    #2 0x7fb58800394e in mozilla::dom::Element::UnbindFromTree(bool, bool) content/base/src/Element.cpp:1378
    #3 0x7fb58628c77c in nsGenericHTMLElement::UnbindFromTree(bool, bool) content/html/content/src/nsGenericHTMLElement.cpp:656
    #4 0x7fb5887be8f9 in nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) content/base/src/nsINode.cpp:1387
0x7fb58cfd344c is located 44 bytes inside of 120-byte region [0x7fb58cfd3420,0x7fb58cfd3498)
freed by thread T0 here:
    #0 0x40f082 in __interceptor_free
    #1 0x7fb584b403a1 in nsNodeUtils::LastRelease(nsINode*) content/base/src/nsNodeUtils.cpp:258
    #2 0x7fb5862831d3 in nsGenericDOMDataNode::Release() content/base/src/nsGenericDOMDataNode.cpp:115
previously allocated by thread T0 here:
    #0 0x40f162 in __interceptor_malloc
Shadow bytes around the buggy address:
  0x1ff6b19fa630: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x1ff6b19fa640: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1ff6b19fa650: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x1ff6b19fa660: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1ff6b19fa670: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x1ff6b19fa680: fa fa fa fa fd fd fd fd fd[fd]fd fd fd fd fd fd
  0x1ff6b19fa690: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x1ff6b19fa6a0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1ff6b19fa6b0: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x1ff6b19fa6c0: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x1ff6b19fa6d0: fd fd fd fd 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 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
Stats: 213M malloced (181M for red zones) by 269475 calls
Stats: 33M realloced by 14482 calls
Stats: 176M freed by 145087 calls
Stats: 37M really freed by 87553 calls
Stats: 396M (396M-0M) mmaped; 99 maps, 0 unmaps
  mmaps   by size class: 6:131070; 7:98301; 8:32766; 9:24573; 10:8190; 11:12282; 12:2048; 13:1536; 14:1280; 15:384; 16:832; 17:1312; 18:48; 19:40; 20:24;
  mallocs by size class: 6:105241; 7:81569; 8:36148; 9:20305; 10:7004; 11:11277; 12:2105; 13:1664; 14:1383; 15:344; 16:1029; 17:1310; 18:37; 19:38; 20:21;
  frees   by size class: 6:62260; 7:28397; 8:21294; 9:13681; 10:4218; 11:9327; 12:1213; 13:1220; 14:1231; 15:237; 16:631; 17:1295; 18:28; 19:36; 20:19;
  rfrees  by size class: 6:45643; 7:16612; 8:13697; 9:2506; 10:1607; 11:5918; 12:418; 13:350; 14:223; 15:105; 16:424; 17:40; 18:5; 19:4; 20:1;
Stats: malloc large: 1406 small slow: 1516
Stats: StackDepot: 0 ids; 0M mapped
==28337== ABORTING

Comment 1

[Abhishek Arya]

Attachment: Testcase 2

Comment 2

[Simon Montagu :smontagu]

Attachment: Patch for testcase 1

This is a simple fix to an error in bug 832644

Comment 3

[Simon Montagu :smontagu]

Attachment: Patch for testcase 2

... and this is a simple fix to an error in bug 831287

Comment 4

[Simon Montagu :smontagu]

Attachment: Testcases for checkin (after the bug is opened)

Comment 6

[(no longer active)]

Comment on attachment 714281 [details] [diff] [review]
Patch for testcase 1

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

sigh...  Sorry I did not catch this before.

Comment 7

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6d2ac238d95
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a1d89eaf2e5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3a1d89eaf2e5

Comment 9

[Abhishek Arya]

I don't think this is completely fixed. Here is another variant.

- crash stack -
mozilla::WalkAncestorsResetAutoDirection
mozilla::SetDirOnBind
- free stack -
nsNodeUtils::LastRelease
nsGenericDOMDataNode::Release

Testcase::
<body dir=auto> u	
<script>
function initCF() {
setTimeout("CFcrash()", 284);
}
document.addEventListener("DOMContentLoaded", initCF, false);
function CFcrash() {
try { document.designMode = 'on'; document.execCommand("InsertHTML", false, "world"); document.designMode = 'off'; } catch(e) {}
try { document.designMode = 'on'; document.execCommand("inserthtml", false, "<span><div>"); } catch(e) {}
}</script>>

Comment 10

[Abhishek Arya]

And

- crash stack -
mozilla::ResetDir
mozilla::dom::Element::UnbindFromTree
- free stack -
nsNodeUtils::LastRelease
nsGenericDOMDataNode::Release

Testcase2::
><bdi id=test1><refa id=test2>&#xf921;>></bdi>><h1 id=test3><script>
document.addEventListener("DOMContentLoaded", CFcrash, false);
function CFcrash() {
test1.setAttribute("dir", "invalid");
test2.textContent = "	&#xf851;S	";
test3.appendChild(test1);
}</script>

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

If this bug is fixed on 22 (not sure where it stands now with comment 9) please nominate for uplift to branches.

Comment 13

[chris hofmann]

does anyone know latest testing status?

Comment 14

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

(In reply to chris hofmann from comment #13)
> does anyone know latest testing status?

What "latest testing status?" The automated testcase can't go in until we ship the fix.

The fix was checked in on 2/21. 

I haven't seen an Aurora or Beta patch nomination, which would be nice since they are affected and it is a sec-critical. Ryan?

Comment 15

[Simon Montagu :smontagu]

I'll be nominating together with bug 845093 (the spin-out bug of the cases from comment 9)

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Simon Montagu from comment #15)
> I'll be nominating together with bug 845093 (the spin-out bug of the cases
> from comment 9)

OK, please do go ahead with nomination then - we should get both those bugs into beta before Tues Mar 12th go to build if low risk enough to take on beta branch.

Comment 17

[Simon Montagu :smontagu]

Comment on attachment 714281 [details] [diff] [review]
Patch for testcase 1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 548206 or one of its followups
User impact if declined: use-after-free vulnerability
Testing completed (on m-c, etc.): baked on m-c since 2013-2-20
Risk to taking this patch (and alternatives if risky): minimal
String or UUID changes made by this patch: none

Comment 18

[Simon Montagu :smontagu]

Comment on attachment 714282 [details] [diff] [review]
Patch for testcase 2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 548206 or one of its followups
User impact if declined: use-after-free vulnerability
Testing completed (on m-c, etc.): Baked on m-c since 2013-02-20
Risk to taking this patch (and alternatives if risky): minimal
String or UUID changes made by this patch: none

Comment 19

[Simon Montagu :smontagu]

https://hg.mozilla.org/releases/mozilla-aurora/rev/43f3eea0639f
https://hg.mozilla.org/releases/mozilla-aurora/rev/a391f996eac1
https://hg.mozilla.org/releases/mozilla-beta/rev/e9fc89ba0f15
https://hg.mozilla.org/releases/mozilla-beta/rev/93fd24dc1bfd

Comment 20

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad9666da2bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fd991121b6e

Comment 21

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

https://hg.mozilla.org/mozilla-central/rev/3ad9666da2bc
https://hg.mozilla.org/mozilla-central/rev/5fd991121b6e