Closed Bug 1406395 Opened 7 years ago Closed 7 years ago

DocumentType heap-use-after-free

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Version:
57 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 + verified
firefox58 + verified
firefox59 --- verified

People

(Reporter: lipe, Assigned: ben.tian)

References

Details

(4 keywords, Whiteboard: [post-critsmash-triage])

Attachments

(3 files, 3 obsolete files)

P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)
3.10 KB, patch
smaug
: review+
abillings
: approval-mozilla-beta+
abillings
: sec-approval+
Details | Diff | Splinter Review
P2: Backout changeset 8f77d260780d (bug 1384661 part 2)
4.29 KB, patch
smaug
: review+
abillings
: approval-mozilla-beta+
abillings
: sec-approval+
Details | Diff | Splinter Review
P3: Backout changeset b679806ce7e3 (bug 1384661 part 1)
5.81 KB, patch
smaug
: review+
abillings
: approval-mozilla-beta+
abillings
: sec-approval+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171005171938

Steps to reproduce:

Opening the following html file causes the free and reuse of the object right after the timeout seconds.

Tested on a Nightly ASAN BuildID: 20171005171938
Linux x64.


<!DOCTYPE html>
<html>
<head>
<script type="text/javascript">

function trigger() {
 
  var range = new Range();
  var p = document.createElement("p");
  range.insertNode(p);

}

function test() {
  for (var p in document) {
    typeof(document[p]);
  }

  var doc = document.implementation.createDocument(null, 'xml', null);
  doc.load("");
  var str = new XMLSerializer().serializeToString(doc);
  document.write(str);
  
  setTimeout(trigger, 10000);
}

</script>
</head>
<body onload="test()">
</body>
</html>


Actual results:

=================================================================
==30496==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f0003bde80 at pc 0x7effd6f8f8e2 bp 0x7fff39e80b20 sp 0x7fff39e80b18
READ of size 8 at 0x60f0003bde80 thread T0
    #0 0x7effd6f8f8e1 in CallQueryInterface<nsINode, nsIDOMNode> /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsUtils.h:134:19
    #1 0x7effd6f8f8e1 in nsParentNodeChildContentList::Item(unsigned int, nsIDOMNode**) /home/worker/workspace/build/src/dom/base/FragmentOrElement.cpp:624
    #2 0x7effd72eaca6 in nsRange::InsertNode(nsINode&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/nsRange.cpp:2815:23
    #3 0x7effd7c3b6fe in mozilla::dom::RangeBinding::insertNode(JSContext*, JS::Handle<JSObject*>, nsRange*, JSJitMethodCallArgs const&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/RangeBinding.cpp:1048:9
    #4 0x7effd8c338d0 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3053:13
    #5 0x7effdf2618c4 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15
    #6 0x7effdf2618c4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:469
    #7 0x7effdf24b368 in CallFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:520:12
    #8 0x7effdf24b368 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3065
    #9 0x7effdf232a9b in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:409:12
    #10 0x7effdf261a5c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:487:15
    #11 0x7effdf2623b2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:533:10
    #12 0x7effdfcae19b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2951:12
    #13 0x7effd87bf389 in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:36:8
    #14 0x7effd6e98b5d in Call<nsCOMPtr<nsISupports> > /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:72:12
    #15 0x7effd6e98b5d in nsGlobalWindow::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:13196
    #16 0x7effd7068644 in mozilla::dom::TimeoutManager::RunTimeout(mozilla::TimeStamp const&, mozilla::TimeStamp const&) /home/worker/workspace/build/src/dom/base/TimeoutManager.cpp:868:42
    #17 0x7effd705d320 in mozilla::dom::TimeoutExecutor::MaybeExecute() /home/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp:167:11
    #18 0x7effd705db76 in Notify /home/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp:235:5
    #19 0x7effd705db76 in non-virtual thunk to mozilla::dom::TimeoutExecutor::Notify(nsITimer*) /home/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp:230
    #20 0x7effd4645760 in nsTimerImpl::Fire(int) /home/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:517:40
    #21 0x7effd4644e74 in nsTimerEvent::Run() /home/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:286:11
    #22 0x7effd46689bc in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:193:22
    #23 0x7effd466844f in mozilla::ThrottledEventQueue::Inner::Executor::Run() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:79:15
    #24 0x7effd465539d in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #25 0x7effd465a728 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:521:10
    #26 0x7effd53e8e96 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:125:5
    #27 0x7effd534a29b in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #28 0x7effd534a29b in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #29 0x7effd534a29b in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #30 0x7effdaa8976f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:158:27
    #31 0x7effdeb91791 in nsAppStartup::Run() /home/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #32 0x7effded72dc4 in XREMain::XRE_mainRun() /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4646:22
    #33 0x7effded749c8 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4810:8
    #34 0x7effded75dfb in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4905:21
    #35 0x4eb643 in do_main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:236:22
    #36 0x4eb643 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:309
    #37 0x7efff17d77cf in __libc_start_main (/lib64/libc.so.6+0x207cf)
    #38 0x41d198 in _start (/home/smbshare/sftp/firelux/firefox/nightly/firefox-bin+0x41d198)

0x60f0003bde80 is located 0 bytes inside of 176-byte region [0x60f0003bde80,0x60f0003bdf30)
freed by thread T0 here:
    #0 0x4bb6cb in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
    #1 0x7effd44f6657 in SnowWhiteKiller::~SnowWhiteKiller() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2695:25
    #2 0x7effd4500d54 in FreeSnowWhite /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:2883:3
    #3 0x7effd4500d54 in nsCycleCollector_doDeferredDeletion() /home/worker/workspace/build/src/xpcom/base/nsCycleCollector.cpp:4267
    #4 0x7effd5dca223 in AsyncFreeSnowWhite::Run() /home/worker/workspace/build/src/js/xpconnect/src/XPCJSRuntime.cpp:126:34
    #5 0x7effd466a2bf in IdleRunnableWrapper::Run() /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:345:22
    #6 0x7effd465539d in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #7 0x7effd465a728 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:521:10
    #8 0x7effd53e8ea1 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #9 0x7effd534a29b in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #10 0x7effd534a29b in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #11 0x7effd534a29b in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #12 0x7effdaa8976f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:158:27
    #13 0x7effdeb91791 in nsAppStartup::Run() /home/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #14 0x7effded72dc4 in XREMain::XRE_mainRun() /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4646:22
    #15 0x7effded749c8 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4810:8
    #16 0x7effded75dfb in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4905:21
    #17 0x4eb643 in do_main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:236:22
    #18 0x4eb643 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:309
    #19 0x7efff17d77cf in __libc_start_main (/lib64/libc.so.6+0x207cf)

previously allocated by thread T0 here:
    #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
    #1 0x4ecf3d in moz_xmalloc /home/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7effd6f4ec81 in operator new /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:200:12
    #3 0x7effd6f4ec81 in NS_NewDOMDocumentType(nsNodeInfoManager*, nsIAtom*, nsAString const&, nsAString const&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/base/DocumentType.cpp:56
    #4 0x7effd6f4ea59 in NS_NewDOMDocumentType(nsIDOMDocumentType**, nsNodeInfoManager*, nsIAtom*, nsAString const&, nsAString const&, nsAString const&) /home/worker/workspace/build/src/dom/base/DocumentType.cpp:31:15
    #5 0x7effd61c0c16 in nsHtml5TreeOperation::AppendDoctypeToDocument(nsIAtom*, nsAString const&, nsAString const&, nsHtml5DocumentBuilder*) /home/worker/workspace/build/src/parser/html/nsHtml5TreeOperation.cpp:680:3
    #6 0x7effd61c8534 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*) /home/worker/workspace/build/src/parser/html/nsHtml5TreeOperation.cpp:896:14
    #7 0x7effd61c4892 in nsHtml5TreeOpExecutor::RunFlushLoop() /home/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:465:27
    #8 0x7effd61cebfb in nsHtml5ExecutorFlusher::Run() /home/worker/workspace/build/src/parser/html/nsHtml5StreamParser.cpp:128:20
    #9 0x7effd465539d in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1040:14
    #10 0x7effd465a728 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:521:10
    #11 0x7effd53e8ea1 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #12 0x7effd534a29b in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #13 0x7effd534a29b in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #14 0x7effd534a29b in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #15 0x7effdaa8976f in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:158:27
    #16 0x7effdeb91791 in nsAppStartup::Run() /home/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #17 0x7effded72dc4 in XREMain::XRE_mainRun() /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4646:22
    #18 0x7effded749c8 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4810:8
    #19 0x7effded75dfb in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4905:21
    #20 0x4eb643 in do_main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:236:22
    #21 0x4eb643 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:309
    #22 0x7efff17d77cf in __libc_start_main (/lib64/libc.so.6+0x207cf)

SUMMARY: AddressSanitizer: heap-use-after-free /home/worker/workspace/build/src/obj-firefox/dist/include/nsISupportsUtils.h:134:19 in CallQueryInterface<nsINode, nsIDOMNode>
Shadow bytes around the buggy address:
  0x0c1e8006fb80: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c1e8006fb90: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e8006fba0: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa
  0x0c1e8006fbb0: fa fa fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e8006fbc0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x0c1e8006fbd0:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e8006fbe0: fd fd fd fd fd fd fa fa fa fa fa fa fa fa 00 00
  0x0c1e8006fbf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e8006fc00: 00 00 03 fa fa fa fa fa fa fa fa fa 00 00 00 00
  0x0c1e8006fc10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e8006fc20: 00 00 fa fa fa fa fa fa fa fa fd fd fd fd fd fd
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
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==30496==ABORTING
Possible regression from bug 1384661.
Group: firefox-core-security → dom-core-security
Component: Untriaged → DOM
Keywords: csectype-uaf, sec-critical
Product: Firefox → Core
Ben, can you investigate this? Thanks.
Flags: needinfo?(btian)
Assignee: nobody → btian
Flags: needinfo?(btian)
P1, as this is a likely new regression in 57.
Priority: -- → P1
Using strong reference in [1]:

  -  AutoTArray<nsIContent*, 8> mCachedChildArray;
  +  AutoTArray<nsCOMPtr<nsIContent>, 8> mCachedChildArray;

removes ASan error but the NS_ERROR_FAILURE returned from

  range.insertNode(p);

remains.

[1] http://searchfox.org/mozilla-central/rev/1033bfa26f6d42c1ef48621909f04e734a7ed8a3/dom/base/nsChildContentList.h#104
The child is removed by [1]. Need to invalidate cached array when nsAttrAndChildArray::TakeChildAt/RemoveChildAt is called, in addition to nsINode::doInsertChildAt/doRemoveChildAt.

[1] http://searchfox.org/mozilla-central/rev/1a4a26905f923458679a59a4be1e455ebc53c333/dom/base/nsDocument.cpp#2373
Change:
- Do not check childCount > 0 for nsDocument, as its childNodes must be of type nsParentNodeChildContentList.
Attachment #8917285 - Attachment is obsolete: true
Changes:
- do not invalidate cached child array in CC unlink method since prior nsINode::Unlink has done it.
- make helper method nsINode::InvalidateChildNodes protected.
Attachment #8917291 - Attachment is obsolete: true
Comment on attachment 8917653 [details] [diff] [review]
Patch 1 (v3): Invalidate cached child array when child is removed but not via nsINode::doRemoveChildAt

Olli, can you review my patch that invalidates cached child array when child is removed but not via |nsINode::doRemoveChildAt|?

The patch invalidates cached child array (bug 1384661) in
|nsDocument::ResetToURI| and |nsDocument::~nsDocument| for they remove child from |nsAttrAndChildArray| directly, not via |nsINode::doRemoveChildAt|. Also the patch adds protected helper method |nsINode::InvalidateChildNodes|.
Attachment #8917653 - Flags: review?(bugs)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8917653 [details] [diff] [review]
Patch 1 (v3): Invalidate cached child array when child is removed but not via nsINode::doRemoveChildAt

This doesn't look quite right, and feels error prone, if one needs to sprinkle InvalidateChildNodes to many places.
We'd need this also in 
http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/FragmentOrElement.cpp#1560,1567,1571 and probably elsewhere too, I think

We may consider backing out bug 1384661. That might be safer for FF57, and since bug 651120 didn't get to FF57, it should be fine, assume Speedometer doesn't regress.
Attachment #8917653 - Flags: review?(bugs) → review-
And sorry that I missed this stuff when looking at bug 1384661.
(In reply to Olli Pettay [:smaug] from comment #10)
> This doesn't look quite right, and feels error prone, if one needs to
> sprinkle InvalidateChildNodes to many places.
> We'd need this also in 
> http://searchfox.org/mozilla-central/rev/
> 31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/FragmentOrElement.cpp#1560,
> 1567,1571 and probably elsewhere too, I think

I did in comment 7 but removed in comment 8 since prior |nsINode::Unlink| [1] invalidates [2][3]. My idea was to invalidate for all callers on nsAttrAndChildArray::InsertChildAt, RemoveChildAt, and TakeChildAt.

[1] http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/FragmentOrElement.cpp#1528
[2] http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/nsINode.cpp#1501
[3] http://searchfox.org/mozilla-central/rev/31606bbabc50b08895d843b9f5f3da938ccdfbbf/dom/base/nsINode.cpp#148

> We may consider backing out bug 1384661. That might be safer for FF57, and
> since bug 651120 didn't get to FF57, it should be fine, assume Speedometer
> doesn't regress.

Agree. For 57 backout is safer. I'll prepare backout patches.
This backout patch causes conflict so I revert manually.
Attachment #8917653 - Attachment is obsolete: true
Attachment #8918161 - Flags: review?(bugs)
Attachment #8918162 - Flags: review?(bugs)
(In reply to Ben Tian [:btian] from comment #16)
> Comment on attachment 8918160 [details] [diff] [review]
> P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)
> 
> Olli, can you review my backout patches?
> 
> I verified on try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=180d32dadb4906a459c5a08e2759fd8509d68c9c
> 
> And speedometer diff with m-c:
> https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-
> central&newProject=try&newRevision=180d32dadb4906a459c5a08e2759fd8509d68c9c&f
> ramework=1&filter=speedometer&selectedTimeRange=172800

Also verified with comment 0 test on asan build.
Attachment #8918160 - Flags: review?(bugs) → review+
Attachment #8918161 - Flags: review?(bugs) → review+
Attachment #8918162 - Flags: review?(bugs) → review+
Comment on attachment 8918160 [details] [diff] [review]
P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
- As comment 0 sample code.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- No. Just backout of bug 1384661.

Which older supported branches are affected by this flaw?
- 57

If not all supported branches, which bug introduced the flaw?
- bug 1384661

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
- No. The same difficulty to create and equal risky as on m-c.

How likely is this patch to cause regressions; how much testing does it need?
- Unlikely since bug 651120 has not landed as comment 10. Bug 1384661 is substitute for removed child array (bug 651120). Verified as in comment 16 and comment 17.
Attachment #8918160 - Flags: sec-approval?
Comment on attachment 8918161 [details] [diff] [review]
P2: Backout changeset 8f77d260780d (bug 1384661 part 2)

See comment 18.
Attachment #8918161 - Flags: sec-approval?
Comment on attachment 8918162 [details] [diff] [review]
P3: Backout changeset b679806ce7e3 (bug 1384661 part 1)

See comment 18.
Attachment #8918162 - Flags: sec-approval?
Comment on attachment 8918160 [details] [diff] [review]
P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)

sec-approval+ for trunk.
We'll want this nominated for Beta (57) as well.
Attachment #8918160 - Flags: sec-approval? → sec-approval+
Attachment #8918161 - Flags: sec-approval? → sec-approval+
Attachment #8918162 - Flags: sec-approval? → sec-approval+
(In reply to Al Billings [:abillings] from comment #21)
> Comment on attachment 8918160 [details] [diff] [review]
> P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)
> 
> sec-approval+ for trunk.
> We'll want this nominated for Beta (57) as well.

Sure, will do.
Keywords: checkin-needed
Comment on attachment 8918160 [details] [diff] [review]
P1: Backout changeset 7df868e0e356 (bug 1384661 part 3)

[Triage Comment]
Attachment #8918160 - Flags: approval-mozilla-beta+
Comment on attachment 8918161 [details] [diff] [review]
P2: Backout changeset 8f77d260780d (bug 1384661 part 2)

[Triage Comment]
Attachment #8918161 - Flags: approval-mozilla-beta+
Comment on attachment 8918162 [details] [diff] [review]
P3: Backout changeset b679806ce7e3 (bug 1384661 part 1)

[Triage Comment]
Attachment #8918162 - Flags: approval-mozilla-beta+
Clear my ni? as beta uplift is approved.
Flags: needinfo?(btian)
Group: dom-core-security → core-security-release
Confirmed issue in Fx57.0b3.
Verified fixed in Fx57.0b14.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
I have managed to reproduce the issue mentioned in comment 0 using Firefox ASAN build 58.0a1 (BuildId:20171005171938).

This issue is no longer reproducible on Firefox 59.0a1(BuildId:20171214095211), 58.0b11(BuildId:20171212143601) and 57.0.2(BuildId:20171207111528) ASAN builds on Ubuntu 16.04 64bit.
status-firefox57: fixed → verified
status-firefox58: fixed → verified
status-firefox59: --- → verified
Flags: qe-verify+
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: