Closed Bug 1431232 Opened 2 years ago Closed 2 years ago

[stylo] heap-buffer-overflow in [@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit] with multicol, perspective, <dialog>, <label>, <li>

Categories

(Core :: Layout, defect, critical)

58 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- verified
firefox61 --- verified

People

(Reporter: tsmith, Assigned: mats)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [adv-main60-])

Attachments

(6 files, 1 obsolete file)

Attached file testcase.html
BuildID=20180117193713
SourceStamp=e7a33d4b7271602c41347a99be1512d74ce78d91

==30899==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6060009b57a8 at pc 0x7fe8627e73f5 bp 0x7ffe4aaaba30 sp 0x7ffe4aaaba28
READ of size 8 at 0x6060009b57a8 thread T0
    #0 0x7fe8627e73f4 in Equals /src/layout/base/FrameProperties.h:399:16
    #1 0x7fe8627e73f4 in IndexOf<const mozilla::FramePropertyDescriptorUntyped *, mozilla::FrameProperties::PropertyComparator> /src/obj-firefox/dist/include/nsTArray.h:1159
    #2 0x7fe8627e73f4 in GetInternal /src/layout/base/FrameProperties.h:413
    #3 0x7fe8627e73f4 in Get<nsContainerFrame> /src/layout/base/FrameProperties.h:235
    #4 0x7fe8627e73f4 in GetProperty<nsContainerFrame> /src/layout/generic/nsIFrame.h:3563
    #5 0x7fe8627e73f4 in nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit(mozilla::ServoRestyleState&) /src/layout/generic/nsInlineFrame.cpp:988
    #6 0x7fe8626f73fa in nsIFrame::DoUpdateStyleOfOwnedAnonBoxes(mozilla::ServoRestyleState&) /src/layout/generic/nsFrame.cpp:11115:42
    #7 0x7fe86241f2b7 in UpdateStyleOfOwnedAnonBoxes /src/layout/generic/nsIFrame.h:3376:7
    #8 0x7fe86241f2b7 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:980
    #9 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #10 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #11 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #12 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #13 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #14 0x7fe86241efd6 in mozilla::ServoRestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:956:32
    #15 0x7fe862421ca7 in mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1149:28
    #16 0x7fe8623dd14e in ProcessPendingRestyles /src/layout/base/ServoRestyleManager.cpp:1242:3
    #17 0x7fe8623dd14e in ProcessPendingRestyles /src/obj-firefox/dist/include/mozilla/RestyleManagerInlines.h:44
    #18 0x7fe8623dd14e in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4215
    #19 0x7fe85db4e0f5 in FlushPendingNotifications /src/obj-firefox/dist/include/nsIPresShell.h:575:5
    #20 0x7fe85db4e0f5 in nsDocument::FlushPendingNotifications(mozilla::FlushType, mozilla::FlushTarget) /src/dom/base/nsDocument.cpp:8135
    #21 0x7fe85d90d232 in GetPrimaryFrame /src/dom/base/Element.cpp:2373:10
    #22 0x7fe85d90d232 in mozilla::dom::Element::GetScrollFrame(nsIFrame**, mozilla::FlushType) /src/dom/base/Element.cpp:691
    #23 0x7fe85d90f115 in mozilla::dom::Element::SetScrollLeft(int) /src/dom/base/Element.cpp:955:28
    #24 0x7fe85f550fe1 in mozilla::dom::ElementBinding::set_scrollLeft(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitSetterCallArgs) /src/obj-firefox/dom/bindings/ElementBinding.cpp:2867:9
    #25 0x7fe85fb36c39 in mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:2997:8
    #26 0x7fe866572364 in CallJSNative /src/js/src/jscntxtinlines.h:291:15
    #27 0x7fe866572364 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #28 0x7fe866574ab6 in InternalCall /src/js/src/vm/Interpreter.cpp:522:12
    #29 0x7fe866574ab6 in Call /src/js/src/vm/Interpreter.cpp:541
    #30 0x7fe866574ab6 in js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) /src/js/src/vm/Interpreter.cpp:670
    #31 0x7fe86761bcac in SetExistingProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) /src/js/src/vm/NativeObject.cpp:2736:10
    #32 0x7fe8676120cc in bool js::NativeSetProperty<(js::QualifiedBool)1>(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) /src/js/src/vm/NativeObject.cpp:2764:20
    #33 0x7fe86655597a in SetProperty /src/js/src/vm/NativeObject.h:1637:12
    #34 0x7fe86655597a in SetPropertyOperation /src/js/src/vm/Interpreter.cpp:270
    #35 0x7fe86655597a in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:2893
    #36 0x7fe866543d40 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #37 0x7fe86657289c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #38 0x7fe8665733c2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10
    #39 0x7fe86705939c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3029:12
    #40 0x7fe85f029eaf in mozilla::dom::IdleRequestCallback::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::IdleDeadline&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/WindowBinding.cpp:827:8
    #41 0x7fe85d96c15a in Call /src/obj-firefox/dist/include/mozilla/dom/WindowBinding.h:633:12
    #42 0x7fe85d96c15a in mozilla::dom::IdleRequest::IdleRun(nsPIDOMWindowInner*, double, bool) /src/dom/base/IdleRequest.cpp:74
    #43 0x7fe85d7b4bc4 in nsGlobalWindowInner::RunIdleRequest(mozilla::dom::IdleRequest*, double, bool) /src/dom/base/nsGlobalWindowInner.cpp:712:19
    #44 0x7fe85d7fdbe2 in nsGlobalWindowInner::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) /src/dom/base/nsGlobalWindowInner.cpp:6637:19
    #45 0x7fe85da2a267 in mozilla::dom::TimeoutManager::RunTimeout(mozilla::TimeStamp const&, mozilla::TimeStamp const&) /src/dom/base/TimeoutManager.cpp:876:42
    #46 0x7fe85da1fa74 in mozilla::dom::TimeoutExecutor::MaybeExecute() /src/dom/base/TimeoutExecutor.cpp:171:11
    #47 0x7fe85da202d6 in Notify /src/dom/base/TimeoutExecutor.cpp:239:5
    #48 0x7fe85da202d6 in non-virtual thunk to mozilla::dom::TimeoutExecutor::Notify(nsITimer*) /src/dom/base/TimeoutExecutor.cpp
    #49 0x7fe85a9ed6dc in nsTimerImpl::Fire(int) /src/xpcom/threads/nsTimerImpl.cpp:704:40
    #50 0x7fe85a9bd859 in nsTimerEvent::Run() /src/xpcom/threads/TimerThread.cpp:286:11
    #51 0x7fe85a9dcf0f in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() /src/xpcom/threads/ThrottledEventQueue.cpp:193:22
    #52 0x7fe85a9dca3f in mozilla::ThrottledEventQueue::Inner::Executor::Run() /src/xpcom/threads/ThrottledEventQueue.cpp:79:15
    #53 0x7fe85a9cd85d in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1040:14
    #54 0x7fe85a9e8630 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:517:10
    #55 0x7fe85b87338a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
    #56 0x7fe85b7cac79 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #57 0x7fe85b7cac79 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #58 0x7fe85b7cac79 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299
    #59 0x7fe861bea3ca in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27
    #60 0x7fe8660868db in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #61 0x7fe86629225a in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4702:22
    #62 0x7fe86629523e in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4841:8
    #63 0x7fe8662966b4 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4933:21
    #64 0x4ee72b in do_main /src/browser/app/nsBrowserApp.cpp:231:22
    #65 0x4ee72b in main /src/browser/app/nsBrowserApp.cpp:304
    #66 0x7fe8793e882f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #67 0x41dfe8 in _start (firefox+0x41dfe8)

0x6060009b57a8 is located 8 bytes to the right of 64-byte region [0x6060009b5760,0x6060009b57a0)
allocated by thread T0 here:
    #0 0x4bf132 in realloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:107:3
    #1 0x4ef98d in moz_xrealloc /src/memory/mozalloc/mozalloc.cpp:93:20
    #2 0x7fe85a8239cc in Realloc /src/obj-firefox/dist/include/nsTArray.h:211:12
    #3 0x7fe85a8239cc in nsTArrayInfallibleAllocator::ResultTypeProxy nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long, unsigned long) /src/obj-firefox/dist/include/nsTArray-inl.h:183
    #4 0x7fe862553448 in AppendElement<mozilla::FrameProperties::PropertyValue, nsTArrayInfallibleAllocator> /src/obj-firefox/dist/include/nsTArray.h:2188:47
    #5 0x7fe862553448 in SetInternal /src/layout/base/FrameProperties.h:442
    #6 0x7fe862553448 in void mozilla::FrameProperties::Set<nsContainerFrame>(mozilla::FramePropertyDescriptor<nsContainerFrame> const*, mozilla::detail::FramePropertyTypeHelper<nsContainerFrame>::Type, nsIFrame const*) /src/layout/base/FrameProperties.h:179
    #7 0x7fe8624872d8 in SetProperty<nsContainerFrame> /src/layout/generic/nsIFrame.h:3577:17
    #8 0x7fe8624872d8 in SetFrameIsIBSplit /src/layout/base/nsCSSFrameConstructor.cpp:593
    #9 0x7fe8624872d8 in nsCSSFrameConstructor::CreateIBSiblings(nsFrameConstructorState&, nsContainerFrame*, bool, nsFrameItems&, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:12291
    #10 0x7fe86247deff in nsCSSFrameConstructor::ConstructInline(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:12221:3
    #11 0x7fe86247883c in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:4034:7
    #12 0x7fe8624842c7 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:6432:3
    #13 0x7fe862462996 in ConstructFramesFromItemList /src/layout/base/nsCSSFrameConstructor.cpp:10830:5
    #14 0x7fe862462996 in nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /src/layout/base/nsCSSFrameConstructor.cpp:11148
    #15 0x7fe86246cda9 in nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState&, nsIContent*, nsContainerFrame*, nsContainerFrame*, nsStyleContext*, nsContainerFrame**, nsFrameItems&, nsIFrame*, PendingBinding*) /src/layout/base/nsCSSFrameConstructor.cpp:12090:3
    #16 0x7fe8624755bb in nsCSSFrameConstructor::ConstructNonScrollableBlockWithConstructor(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&, nsBlockFrame* (*)(nsIPresShell*, nsStyleContext*)) /src/layout/base/nsCSSFrameConstructor.cpp:5160:3
    #17 0x7fe86247d367 in nsCSSFrameConstructor::ConstructNonScrollableBlock(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:5124:10
    #18 0x7fe86247883c in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:4034:7
    #19 0x7fe8624842c7 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:6432:3
    #20 0x7fe862461255 in nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:10830:5
    #21 0x7fe86248db31 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind, TreeMatchContext*) /src/layout/base/nsCSSFrameConstructor.cpp:8338:3
    #22 0x7fe86248529d in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /src/layout/base/nsCSSFrameConstructor.cpp:9876:9
    #23 0x7fe8623a7b1d in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /src/layout/base/RestyleManager.cpp:1514:25
    #24 0x7fe8624220c5 in mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1166:9
    #25 0x7fe8623dd14e in ProcessPendingRestyles /src/layout/base/ServoRestyleManager.cpp:1242:3
    #26 0x7fe8623dd14e in ProcessPendingRestyles /src/obj-firefox/dist/include/mozilla/RestyleManagerInlines.h:44
    #27 0x7fe8623dd14e in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4215
    #28 0x7fe85db4e0f5 in FlushPendingNotifications /src/obj-firefox/dist/include/nsIPresShell.h:575:5
    #29 0x7fe85db4e0f5 in nsDocument::FlushPendingNotifications(mozilla::FlushType, mozilla::FlushTarget) /src/dom/base/nsDocument.cpp:8135
    #30 0x7fe85d7e2c85 in FlushPendingNotifications /src/dom/base/nsGlobalWindowInner.cpp:6711:11
    #31 0x7fe85d7e2c85 in nsGlobalWindowInner::ScrollTo(mozilla::dom::ScrollToOptions const&) /src/dom/base/nsGlobalWindowInner.cpp:3773
    #32 0x7fe85f1acd80 in mozilla::dom::WindowBinding::scrollTo(JSContext*, JS::Handle<JSObject*>, nsGlobalWindowInner*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/WindowBinding.cpp:3351:13
    #33 0x7fe85f1a5460 in mozilla::dom::WindowBinding::genericMethod(JSContext*, unsigned int, JS::Value*) /src/obj-firefox/dom/bindings/WindowBinding.cpp:15196:13
    #34 0x7fe866572364 in CallJSNative /src/js/src/jscntxtinlines.h:291:15
    #35 0x7fe866572364 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #36 0x7fe86655d396 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12
    #37 0x7fe86655d396 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096
    #38 0x7fe866543d40 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #39 0x7fe86657289c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #40 0x7fe8665733c2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:541:10
    #41 0x7fe86705939c in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3029:12

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/layout/base/FrameProperties.h:399:16 in Equals
Shadow bytes around the buggy address:
  0x0c0c8012eaa0: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c0c8012eab0: 00 00 00 00 00 00 00 fa fa fa fa fa 00 00 00 00
  0x0c0c8012eac0: 00 00 00 00 fa fa fa fa fd fd fd fd fd fd fd fa
  0x0c0c8012ead0: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c8012eae0: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
=>0x0c0c8012eaf0: 00 00 00 00 fa[fa]fa fa 00 00 00 00 00 00 00 fa
  0x0c0c8012eb00: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c8012eb10: 00 00 00 00 00 00 06 fa fa fa fa fa 00 00 00 00
  0x0c0c8012eb20: 00 00 00 fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c8012eb30: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c0c8012eb40: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==30899==ABORTING
Flags: in-testsuite?
Attached file testcase 2
Here's a second testcase which is a tad simpler / more targeted than the first.

Still includes a bunch of bizarre stuff -- multicol, perspective, <dialog>, <label>, <li>.
Summary: heap-buffer-overflow in [@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit] → heap-buffer-overflow in [@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit] with multicol, perspective, <dialog>, <label>, <li>
This crashes a normal Nightly build, too, BTW -- no ASAN required.

If I disable stylo (layout.css.servo.enabled), neither testcase crashes (in either ASAN or Nightly builds).  So this superficially looks like a regression from shipping stylo. And indeed, mozregression traces it to this range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f721a664bf64fed99184a866b60c24a6afcb3a0&tochange=62cebea1d1578461a423a9ca7848706455db9ea5
...which includes "enable stylo by default".

Though -- while Firefox Nightly 57 *was* affected (as I can verify by running "mozregression --launch 7b3215e18443 -a /path/to/testcase.html"), I *cannot* reproduce in Firefox 57 release (latest version).  So there's some other variable involved which was enabled on Nightly57 & disabled on release-57.

But Firefox 58 Beta *is* affected (I can reproduce the crash there). So from a user's perspective, it looks like this will manifest as a regression in Firefox 58. Setting flags accordingly.

emilio, maybe you could take a look?
Flags: needinfo?(emilio)
Keywords: regression
Summary: heap-buffer-overflow in [@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit] with multicol, perspective, <dialog>, <label>, <li> → [stylo] heap-buffer-overflow in [@ nsInlineFrame::UpdateStyleOfOwnedAnonBoxesForIBSplit] with multicol, perspective, <dialog>, <label>, <li>
Note: in a debug m-c build with stylo enabled (i.e. the default), I hit the following assertions (the last one fatal, inside the same call to UpdateStyleOfOwnedAnonBoxesForIBSplit that crashes in the backtrace quoted in comment 0):

=======
###!!! ASSERTION: frame tree not empty, but caller reported complete status: 'aSubtreeRoot->GetPrevInFlow()', file $SRC/layout/base/nsLayoutUtils.cpp, line 7993

###!!! ASSERTION: why should we have flushed style again?: 'target == FlushTarget::ParentOnly || (mPresShell && currentGeneration == mPresShell->GetPresContext()->GetUndisplayedRestyleGeneration())', file $SRC/layout/style/nsComputedDOMStyle.cpp, line 1084

Assertion failure: nextInline (There is always a trailing inline in an IB split), at $SRC/layout/generic/nsInlineFrame.cpp:989
=======


If I disable stylo, then I get these nonfatal assertions instead (the first assertion is the same, the other one is different):

=======
###!!! ASSERTION: frame tree not empty, but caller reported complete status: 'aSubtreeRoot->GetPrevInFlow()', file $SRC/layout/base/nsLayoutUtils.cpp, line 7993

###!!! ASSERTION: How did that happen?: 'nextCont', file $SRC/layout/painting/nsCSSRendering.cpp, line 320
=======

In both cases, the final assertion seems to be about the same thing -- a null next-continuation pointer on a frame that's marked as being part of an IB split.  Looking for recent bugs with similar buzzwords -- it looks like this is related to bug 1405443, which starts with the same assertion ("frame tree not empty"), and which mentions page-break-inside:avoid, columns, and ib splits.  Or maybe this is bug 1402766 resurfacing in a different way? (which was also a servo-specific crash in UpdateStyleOfOwnedAnonBoxesForIBSplit, with multicol & page-break-inside:avoid)
(Also: strangely, in an --enable-optimize --enable-debug-symbols build, we crash one line *earlier* than we do in a debug build, as suggested by the line numbers in Comment 0's backtrace vs. the line number of the fatal assertion in comment 3. Specifically, we crash in a InvalidArrayIndex_CRASH(...) invocation, inside of nsTArray_Impl<mozilla::FrameProperties::PropertyValue, nsTArrayInfallibleAllocator>::ElementAt for the property-table lookup here just before the assertion:
https://searchfox.org/mozilla-central/rev/1f9b4f0f0653b129b4db1b9fc5e9068054afaac0/layout/generic/nsInlineFrame.cpp#988

InvalidArrayIndex_CRASH looks like an intentionally-caught safe crash (per its name), but I don't know enough about it to be sure whether we hit it reliably on user machines & release builds.
Hmm, this looks familiar indeed...

So this seems to be crashing on one of the scrollLeft setters of marquee in: https://searchfox.org/mozilla-central/source/layout/style/xbl-marquee/xbl-marquee.xml

Why is this a HBO though? Seems like nsTArray::IndexOf should never ever read out of bounds...
This is the restyle that causes the:

 "why should we have flushed style again?"

Assertion.

That looks pretty bogus.
That was with test-case 1, let's see the second.
This is the page-break: avoid check (I think) that makes us mess up, report a complete reflow and delete the inline frame incorrectly.

`kidFrame` in nsAbsoluteContainingBlock, is the dialog frame, nextFrame a continuation of it, that we will delete next, and where the inline holds from:

Block(li)(1)@7f1691c5abd8 parent=7f1691c5aa50 next-in-flow=7f1691c5b6c0 {0,0,23760,1140} vis-overflow=-840,0,24600,2240 scr-overflow=-840,0,24600,2240 [state=0001122041100201] perspective [content=7f168e742790] [sc=7f168e748208]<
  line 7f1691c5b2a8: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x500] {0,0,0,1140} vis-overflow=-840,427,840,473 scr-overflow=-840,427,840,473 <
    Placeholder(dialog)(1)@7f1691c5ad38 parent=7f1691c5abd8 {0,900,0,0} [state=0000000000200000] [content=7f168e742820] [sc=7f168e748808:-moz-oof-placeholder] outOfFlowFrame=Block(dialog)(1)@7f1691c5ac88
  >
  AbsoluteList 0x7f16a051a3e0 <
    Block(dialog)(1)@7f1691c5ac88 parent=7f1691c5abd8 next-in-flow=7f1691c5b600 {10800,0,2160,2240} [state=0000162000d00301] [content=7f168e742820] [sc=7f168e748308]<
      line 7f1691c5b1b8: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x300] {1080,1080,0,0} <
        Inline(label)(1)@7f1691c5ada8 parent=7f1691c5ac88 next=7f1691c5b028 IBSplitSibling=7f1691c5b028 {1080,180,0,1140} vis-overflow=0,0,0,0 scr-overflow=0,900,0,0 [state=0080100000008200] [content=7f168e7428b0] [sc=7f168e748408]<
          Text(0)"\n        "@7f1691c5ae38 parent=7f1691c5ada8 {0,900,0,0} [state=4000000028600000] [content=7f16b2869d80] [sc=7f168e748a08:-moz-text] [run=7f16942bfba0][0,9,T] 
        >
      >
      line 7f1691c5b208: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8] {1080,1080,0,1140} vis-overflow=240,1080,840,1140 scr-overflow=240,1080,840,1140 <
        Block(label)(1)@7f1691c5b028 parent=7f1691c5ac88 IBSplitSibling=7f1691c5b128 IBSplitPrevSibling=7f1691c5ada8 {1080,1080,0,1140} vis-overflow=-840,0,840,1140 scr-overflow=-840,0,840,1140 [state=0000020000108000] [content=7f168e7428b0] [sc=7f168e748d08:-moz-block-inside-inline-wrapper]<
          line 7f1691c5b0d8: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x108] {0,0,0,1140} vis-overflow=-840,0,840,1140 scr-overflow=-840,0,840,1140 <
            Block(li)(1)@7f1691c5aec0 parent=7f1691c5b028 {0,0,0,1140} vis-overflow=-840,0,840,1140 scr-overflow=-840,0,840,1140 [state=0000120040100200] [content=7f168e742940] [sc=7f168e748508]<
              BulletList 0x7f1691c5b018 <
                Bullet(li)(1)@7f1691c5af70 parent=7f1691c5aec0 {-840,427,840,473} [content=7f168e742940] [sc=7f168e748c08:-moz-list-bullet]
              >
            >
          >
        >
      >
    >
  >
  BulletList 0x7f1691c5b3a0 <
    Bullet(li)(1)@7f1691c5b2f8 parent=7f1691c5abd8 {-840,427,840,473} [content=7f168e742790] [sc=7f168e748e08:-moz-list-bullet]
  >
>
So the history here is that bug 1402766 landed a wallpaper null-check in the UpdateStyleOfOwnedAnonBoxesForIBSplit code.  This landed on nightly 58 and was backported to beta 57.

Bug 1405443 was filed to fix the real underlying issue.  Once that was fixed, the wallpaper was removed.  That happened in 58.

This testcase seems to trigger the issue bug 1405443 was filed on, in a slightly different way.  That is, the fix for bug 1405443 was not complete.  The above history is why it crashes in 57 nightly but not 57 release (which has the wallpaper), and in 58 beta (which does not have the wallpaper).

I think we should reland bug 1402766 on beta 58 (I filed bug 1431282 on this) and work on fixing the real underlying issue here: page-break-inside:avoid can trigger the "frame tree not empty, but caller reported complete status" assertion, after which we destroy random frames that other parts of the frame tree might be referencing.  We clean up the pointers, so it's not UAF, but we get nulls where we should never have nulls.
Flags: needinfo?(mats)
The check for completion that doesn't check for page-break-inside is:

  https://searchfox.org/mozilla-central/rev/1f9b4f0f0653b129b4db1b9fc5e9068054afaac0/layout/generic/nsAbsoluteContainingBlock.cpp#170

What's the right thing to do there where you get to that situation, I wish I knew :)
Flags: needinfo?(emilio)
Keywords: sec-high
See also the publicly-visible bug 1431781, which was just filed with a testcase that triggers some of the same assertions in debug build (but doesn't cause any obvious security problems / crashes in opt builds).

Perhaps it might be easier to fix the [shared?] root cause on that bug, in the open, rather than this one.
See Also: → 1402766
See Also: → 1431781
As expected, bug 1431282 fixed the crash and fatal assertion.
The non-fatal assertions still occurs though:
ASSERTION: frame tree not empty, but caller reported complete status: 'aSubtreeRoot->GetPrevInFlow()', layout/base/nsLayoutUtils.cpp, line 7995
ASSERTION: frame tree not empty, but caller reported complete status: 'start == end || IsInLetterFrame(aSubtreeRoot)', layout/base/nsLayoutUtils.cpp, line 8009
ASSERTION: How did that happen?: 'nextCont', layout/painting/nsCSSRendering.cpp, line 320
ASSERTION: why should we have flushed style again?: 'target == FlushTarget::ParentOnly || (mPresShell && currentGeneration == mPresShell->GetPresContext()->GetUndisplayedRestyleGeneration())', layout/style/nsComputedDOMStyle.cpp, line 1084
Depends on: 1431282
(In reply to Daniel Holbert [:dholbert] from comment #12)
> Perhaps it might be easier to fix the [shared?] root cause on that bug, in
> the open, rather than this one.

This bug is unrelated to bug 1431781.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #11)
> What's the right thing to do there where you get to that situation, I wish I
> knew :)

I think we could either introduce some sort of (empty) overflow container
frames *at the start* of the continuation chain, or allow abs.pos. frames
to be pushed.  Either one is probably non-trivial to implement.
Attached patch wip (obsolete) — Splinter Review
This might work as a wallpaper for now...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d6f60e6523b240a0d3ee61fecb8b765ffa8c2ba
Flags: needinfo?(mats)
Attached patch fixSplinter Review
I think this is a reasonable wallpaper for now.
Assignee: nobody → mats
Attachment #8944221 - Attachment is obsolete: true
Attachment #8944310 - Flags: review?(dholbert)
Comment on attachment 8944310 [details] [diff] [review]
fix

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

Commit message nit:
> Bug 1431232 - Treat break-before status as Incomplete for now. r=dholbert

Please add "for abspos frames" or something like that, to avoid giving the impression that we're suppressing break-before entirely.

::: layout/generic/nsAbsoluteContainingBlock.cpp
@@ +172,4 @@
>            aDelegatingFrame->IsFrameOfType(nsIFrame::eCanContainOverflowContainers)) {
> +        // XXX it's unclear how we should handle 'page-break-inside:avoid' on
> +        // abs.pos. boxes -- ignore it for now by setting the status to
> +        // Incomplete (which will fragment it).

s/will/will probably/

(Hypothetically, 'kidFrame' here might not be a fragmentable frame-type -- so stating that we "will fragment it" might be wrong.)
Attachment #8944310 - Flags: review?(dholbert) → review+
I believe this should have had sec-approval before landing since it's a sec-high affecting more than just trunk. Please do the request when you get a chance and also request Beta uplift.
Flags: needinfo?(mats)
Version: 59 Branch → 58 Branch
(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)
> I believe this should have had sec-approval before landing since it's a
> sec-high affecting more than just trunk. Please do the request when you get
> a chance and also request Beta uplift.

I think this is just a null deref fwiw, when I was looking at it at least.
I believe the sec-high part of this bug was fixed by bug 1431282 (see
comment 10 above).  With that fix, the testcases here don't crash, so
this bug is now only about the non-fatal assertions in comment 13.
I believe those could at most lead to stale frame pointers so any
potential crashes should be covered by frame-poisoning.

The conditions to trigger the assertions seems like an edge case to me,
so I don't think it needs to be uplifted.  It's fairly low-risk though
(for the same reason) so feel free to request it if you want it anyway.
Flags: needinfo?(mats)
OS: Unspecified → All
Hardware: Unspecified → All
Depends on: 1432853
We're early in the cycle, so I think uplifting to Beta is still reasonable.
Group: layout-core-security → core-security-release
Whiteboard: [adv-main60-]
I have managed to reproduce this issue using Firefox 59.0a1(BuildId:20180117220327).

This issue is verified fixed using Firefox 61.0a1(BuildId:20180503100146) and Firefox 60.0(BuildId:20180430165945) on Windows 10 64bit, macOS 10.13.3 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.