1075336 - Heap-use-after-free in mozilla::CustomCounterStyle::IsBullet

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Abhishek Arya]

Attachment: Testcase

==29679==ERROR: AddressSanitizer: heap-use-after-free on address 0x6100002adb40 at pc 0x7fba7541c77a bp 0x7fff88675440 sp 0x7fff88675438
READ of size 8 at 0x6100002adb40 thread T0
    #0 0x7fba7541c779 in mozilla::CustomCounterStyle::IsBullet()
    #1 0x7fba75948ea6 in nsBlockFrame::SetInitialChildList(mozilla::layout::FrameChildListID, nsFrameList&) /build/firefox/src/layout/generic/nsBlockFrame.cpp:6540:46
    #2 0x7fba757a006c in nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState&, nsStyleDisplay const*, nsIContent*, nsContainerFrame*, nsContainerFrame*, nsStyleContext*, nsContainerFrame**, nsFrameItems&, nsIFrame*, PendingBinding*) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:10777:3
    #3 0x7fba757ad8d5 in nsCSSFrameConstructor::ConstructNonScrollableBlock(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:4720:3
    #4 0x7fba757a8fdf in nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:3738:7
    #5 0x7fba757b3548 in nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:5801:3
    #6 0x7fba757bbe12 in nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:9563:5
    #7 0x7fba757b418d in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, bool) /build/firefox/src/layout/base/nsCSSFrameConstructor.cpp:7090:10
    #8 0x7fba75741d94 in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /build/firefox/src/layout/base/RestyleManager.cpp:743:7
    #9 0x7fba7574415f in mozilla::RestyleManager::ComputeAndProcessStyleChange(nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint) /build/firefox/src/layout/base/RestyleManager.cpp:3568:3
    #10 0x7fba7574390c in mozilla::RestyleManager::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint) /build/firefox/src/layout/base/RestyleManager.cpp:926:5
    #11 0x7fba75755e16 in mozilla::RestyleTracker::DoProcessRestyles() /build/firefox/src/layout/base/RestyleTracker.cpp:166:5
    #12 0x7fba75747ec0 in mozilla::RestyleManager::ProcessPendingRestyles() /build/firefox/src/layout/base/RestyleTracker.h:277:7
    #13 0x7fba756b2cfd in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /build/firefox/src/layout/base/nsPresShell.cpp:4231:9
    #14 0x7fba746bcd91 in nsDocument::FlushPendingNotifications(mozFlushType) /build/firefox/src/content/base/src/nsDocument.cpp:7927:7
    #15 0x7fba7477ba82 in mozilla::dom::Element::GetStyledFrame() /build/firefox/src/content/base/src/Element.cpp:1802:5
    #16 0x7fba74b9dd77 in nsGenericHTMLElement::GetOffsetRect(mozilla::gfx::IntRectTyped<mozilla::CSSPixel>&) /build/firefox/src/content/html/content/src/nsGenericHTMLElement.cpp:335:21
    #17 0x7fba72ea3b53 in mozilla::dom::HTMLElementBinding::get_offsetTop(JSContext*, JS::Handle<JSObject*>, nsGenericHTMLElement*, JSJitGetterCallArgs) /build/firefox/src/content/html/content/src/nsGenericHTMLElement.h:268:5
    #18 0x7fba73b73d9b in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /build/firefox/src/dom/bindings/BindingUtils.cpp:2344:13
    #19 0x7fba789315c7 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /build/firefox/src/js/src/jscntxtinlines.h:231:15
    #20 0x7fba788b3622 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Interpreter.cpp:540:10
    #21 0x7fba7893385d in js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Interpreter.cpp:613:12
    #22 0x7fba786c9d1a in js::baseops::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Shape-inl.h:46:16
    #23 0x7fba789238d2 in Interpret(JSContext*, js::RunState&) /build/firefox/src/js/src/jsobj.h:1031:18
    #24 0x7fba78907f04 in js::RunScript(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:434:12
    #25 0x7fba788de6bb in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /build/firefox/src/js/src/vm/Interpreter.cpp:640:15
    #26 0x7fba78933beb in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /build/firefox/src/js/src/vm/Interpreter.cpp:676:12
    #27 0x7fba78567c82 in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::Value*) /build/firefox/src/js/src/jsapi.cpp:4813:19
    #28 0x7fba726a7b33 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:246:14
    #29 0x7fba726a71dd in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:176:10
    #30 0x7fba726a87e2 in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:298:10
    #31 0x7fba725b1770 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12234:5
    #32 0x7fba7258ff6b in nsGlobalWindow::RunTimeout(nsTimeout*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12465:32
    #33 0x7fba725b0811 in nsGlobalWindow::TimerCallback(nsITimer*, void*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12711:3
    #34 0x7fba708da2a9 in nsTimerImpl::Fire() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:618:7
    #35 0x7fba708daa73 in nsTimerEvent::Run() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:711:3
    #36 0x7fba708d1a9f in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:830:7
    #37 0x7fba70926d96 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:265:10
    #38 0x7fba7112b9be in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /build/firefox/src/ipc/glue/MessagePump.cpp:140:5
    #39 0x7fba710da721 in MessageLoop::Run() /build/firefox/src/ipc/chromium/src/base/message_loop.cc:230:3
    #40 0x7fba745752cf in nsBaseAppShell::Run()
    #41 0x7fba769bd761 in nsAppStartup::Run() /build/firefox/src/toolkit/components/startup/nsAppStartup.cpp:280:19
    #42 0x7fba76a9bd5e in XREMain::XRE_mainRun() /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4155:10
    #43 0x7fba76a9cd46 in XREMain::XRE_main(int, char**, nsXREAppData const*) /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4228:8
    #44 0x7fba76a9db86 in XRE_main /build/firefox/src/toolkit/xre/nsAppRunner.cpp:4442:16
    #45 0x7fba81b36062 in main /build/firefox/src/browser/app/nsBrowserApp.cpp:290:12
    #46 0x7fba8067276c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #47 0x7fba81a94c58 in _start

0x6100002adb40 is located 0 bytes inside of 192-byte region [0x6100002adb40,0x6100002adc00)
freed by thread T0 here:
    #0 0x7fba81b13c01 in __interceptor_free _asan_rtl_
    #1 0x7fba7546ecbf in mozilla::CustomCounterStyle::Release() /build/firefox/src/objdir-ff-asan/layout/style/../../dist/include/mozilla/mozalloc.h:232:12
    #2 0x7fba7547502c in nsTArray_Impl<nsRefPtr<mozilla::CounterStyle>, nsTArrayInfallibleAllocator>::~nsTArray_Impl() /build/firefox/src/objdir-ff-asan/layout/style/../../dist/include/nsRefPtr.h:57:7
    #3 0x7fba75422d5f in mozilla::CounterStyleManager::NotifyRuleChanged() /build/firefox/src/layout/style/CounterStyleManager.cpp:2006:8
    #4 0x7fba758ba5eb in nsPresContext::FlushCounterStyles() /build/firefox/src/layout/base/nsPresContext.cpp:2207:20
    #5 0x7fba756b2b2e in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) /build/firefox/src/layout/base/nsPresShell.cpp:4208:7
    #6 0x7fba746bcd91 in nsDocument::FlushPendingNotifications(mozFlushType) /build/firefox/src/content/base/src/nsDocument.cpp:7927:7
    #7 0x7fba7477ba82 in mozilla::dom::Element::GetStyledFrame() /build/firefox/src/content/base/src/Element.cpp:1802:5
    #8 0x7fba74b9dd77 in nsGenericHTMLElement::GetOffsetRect(mozilla::gfx::IntRectTyped<mozilla::CSSPixel>&) /build/firefox/src/content/html/content/src/nsGenericHTMLElement.cpp:335:21
    #9 0x7fba72ea3b53 in mozilla::dom::HTMLElementBinding::get_offsetTop(JSContext*, JS::Handle<JSObject*>, nsGenericHTMLElement*, JSJitGetterCallArgs) /build/firefox/src/content/html/content/src/nsGenericHTMLElement.h:268:5
    #10 0x7fba73b73d9b in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /build/firefox/src/dom/bindings/BindingUtils.cpp:2344:13
    #11 0x7fba789315c7 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) /build/firefox/src/js/src/jscntxtinlines.h:231:15
    #12 0x7fba788b3622 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Interpreter.cpp:540:10
    #13 0x7fba7893385d in js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Interpreter.cpp:613:12
    #14 0x7fba786c9d1a in js::baseops::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /build/firefox/src/js/src/vm/Shape-inl.h:46:16
    #15 0x7fba789238d2 in Interpret(JSContext*, js::RunState&) /build/firefox/src/js/src/jsobj.h:1031:18
    #16 0x7fba78907f04 in js::RunScript(JSContext*, js::RunState&) /build/firefox/src/js/src/vm/Interpreter.cpp:434:12
    #17 0x7fba788de6bb in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /build/firefox/src/js/src/vm/Interpreter.cpp:640:15
    #18 0x7fba78933beb in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /build/firefox/src/js/src/vm/Interpreter.cpp:676:12
    #19 0x7fba78567c82 in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::Value*) /build/firefox/src/js/src/jsapi.cpp:4813:19
    #20 0x7fba726a7b33 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:246:14
    #21 0x7fba726a71dd in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:176:10
    #22 0x7fba726a87e2 in nsJSUtils::EvaluateString(JSContext*, nsAString_internal const&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /build/firefox/src/dom/base/nsJSUtils.cpp:298:10
    #23 0x7fba725b1770 in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12234:5
    #24 0x7fba7258ff6b in nsGlobalWindow::RunTimeout(nsTimeout*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12465:32
    #25 0x7fba725b0811 in nsGlobalWindow::TimerCallback(nsITimer*, void*) /build/firefox/src/dom/base/nsGlobalWindow.cpp:12711:3
    #26 0x7fba708da2a9 in nsTimerImpl::Fire() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:618:7
    #27 0x7fba708daa73 in nsTimerEvent::Run() /build/firefox/src/xpcom/threads/nsTimerImpl.cpp:711:3
    #28 0x7fba708d1a9f in nsThread::ProcessNextEvent(bool, bool*) /build/firefox/src/xpcom/threads/nsThread.cpp:830:7
    #29 0x7fba70926d96 in NS_ProcessNextEvent(nsIThread*, bool) /build/firefox/src/xpcom/glue/nsThreadUtils.cpp:265:10

previously allocated by thread T0 here:
    #0 0x7fba81b13ed9 in malloc _asan_rtl_
    #1 0x7fba7c200a7d in moz_xmalloc
    #2 0x7fba7541b93d in mozilla::CounterStyleManager::BuildCounterStyle(nsAString_internal const&) /build/firefox/src/objdir-ff-asan/layout/style/../../dist/include/mozilla/mozalloc.h:208:12
    #3 0x7fba754212ca in mozilla::CustomCounterStyle::ComputeExtends() /build/firefox/src/layout/style/CounterStyleManager.cpp:1604:31
    #4 0x7fba7541c217 in mozilla::CustomCounterStyle::GetPrefix(nsAString_internal&) /build/firefox/src/layout/style/CounterStyleManager.cpp:1634:5
    #5 0x7fba75952c9c in nsBulletFrame::GetListItemText(nsAString_internal&) /build/firefox/src/layout/generic/nsBulletFrame.cpp:468:3
    #6 0x7fba75953ea7 in nsBulletFrame::GetDesiredSize(nsPresContext*, nsRenderingContext*, nsHTMLReflowMetrics&, float) /build/firefox/src/layout/generic/nsBulletFrame.cpp:577:7
    #7 0x7fba75954b62 in nsBulletFrame::GetMinISize(nsRenderingContext*) /build/firefox/src/layout/generic/nsBulletFrame.cpp:636:3
    #8 0x7fba75892705 in nsLayoutUtils::IntrinsicForContainer(nsRenderingContext*, nsIFrame*, nsLayoutUtils::IntrinsicISizeType, unsigned int) /build/firefox/src/layout/base/nsLayoutUtils.cpp:3848:16
    #9 0x7fba759a4c6b in nsFrame::AddInlineMinISize(nsRenderingContext*, nsIFrame::InlineMinISizeData*) /build/firefox/src/layout/generic/nsFrame.cpp:3853:25
    #10 0x7fba7591447d in nsBlockFrame::GetMinISize(nsRenderingContext*) /build/firefox/src/layout/generic/nsBlockFrame.cpp:734:11
    #11 0x7fba759a9b4e in nsFrame::ShrinkWidthToFit(nsRenderingContext*, int)
    #12 0x7fba75967494 in nsContainerFrame::ComputeAutoSize(nsRenderingContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, bool) /build/firefox/src/layout/generic/nsContainerFrame.cpp:906:27
    #13 0x7fba759a76ac in nsFrame::ComputeSize(nsRenderingContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, unsigned int) /build/firefox/src/layout/generic/nsFrame.cpp:4063:24
    #14 0x7fba75a1d5b1 in nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin const*, nsMargin const*, nsIAtom*) /build/firefox/src/layout/generic/nsHTMLReflowState.cpp:2149:9
    #15 0x7fba759e92f0 in nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) /build/firefox/src/layout/generic/nsHTMLReflowState.cpp:365:3
    #16 0x7fba759401fe in nsBlockFrame::ComputeFloatWidth(nsBlockReflowState&, nsRect const&, nsIFrame*) /build/firefox/src/layout/generic/nsBlockFrame.cpp:5774:3
    #17 0x7fba7594e00e in nsBlockReflowState::AddFloat(nsLineLayout*, nsIFrame*, int) /build/firefox/src/layout/generic/nsBlockReflowState.cpp:554:8
    #18 0x7fba758dc604 in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) /build/firefox/src/layout/generic/nsLineLayout.h:155:12
    #19 0x7fba75935871 in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) /build/firefox/src/layout/generic/nsBlockFrame.cpp:3799:3
    #20 0x7fba759340f2 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) /build/firefox/src/layout/generic/nsBlockFrame.cpp:3599:5
    #21 0x7fba7592cc72 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) /build/firefox/src/layout/generic/nsBlockFrame.cpp:3469:9
    #22 0x7fba7591e74a in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) /build/firefox/src/layout/generic/nsBlockFrame.cpp:2615:5
    #23 0x7fba75917359 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) /build/firefox/src/layout/generic/nsBlockFrame.cpp:1136:3
    #24 0x7fba75931cd2 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) /build/firefox/src/layout/generic/nsBlockReflowContext.cpp:290:3
    #25 0x7fba75929a8e in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) /build/firefox/src/layout/generic/nsBlockFrame.cpp:3196:5
    #26 0x7fba7591e702 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) /build/firefox/src/layout/generic/nsBlockFrame.cpp:2612:5
    #27 0x7fba75917359 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) /build/firefox/src/layout/generic/nsBlockFrame.cpp:1136:3
    #28 0x7fba7595b9cc in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) /build/firefox/src/layout/generic/nsContainerFrame.cpp:945:3
    #29 0x7fba7595c983 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) /build/firefox/src/layout/generic/nsContainerFrame.cpp:945:3

SUMMARY: AddressSanitizer: heap-use-after-free ??:0 ??
Shadow bytes around the buggy address:
  0x0c208004db10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c208004db20: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c208004db30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c208004db40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c208004db50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c208004db60: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd
  0x0c208004db70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c208004db80: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c208004db90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c208004dba0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c208004dbb0: fd fd fd fd fd fd fd fd fd fd 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
  ASan internal:           fe
==29679==ABORTING

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

[Tracking Requested - why for this release]:

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why is it that you only want to do this (the fix for bug 1066089) if IsDependentStyle() is true?

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #3)
> Why is it that you only want to do this (the fix for bug 1066089) if
> IsDependentStyle() is true?

Because all custom styles are dependent: http://dxr.mozilla.org/mozilla-central/source/layout/style/CounterStyleManager.cpp#1772

Putting it inside the check of dependent style could avoid extra check from builtin styles (though there is little difference).

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch for beta & aurora

Since patch for bug 1066089 is not uplifted to beta and aurora, the previous patch will fail to be applied in those versions. This patch removes the part added in bug 1066089, hence could be applied cleanly for beta and aurora.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, so the fix here is that you're now running this code for the "!newRule" case as well?

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #6)
> Oh, so the fix here is that you're now running this code for the "!newRule"
> case as well?

That's right. This is the case not be processed in bug 1066089.

However, like bug 1066089, I guess there might be some other problem besides which is fixed here. This bug again indicates that there is some style structures are not reconstructed after a style change. Any idea?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's probably related to bug 931668.  It might be that certain changes resulting from @counter-style rule changes need to trigger a restyle that includes the eRestyle_ForceDescendants bit set.  (What's the codepath that leads to the restyle?)

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #8)
> It's probably related to bug 931668. 

No, I don't think so. Bug 931668 should not affect beta and aurora, and the improper change flags have been fixed in bug 1069716.

> It might be that certain changes
> resulting from @counter-style rule changes need to trigger a restyle that
> includes the eRestyle_ForceDescendants bit set.  (What's the codepath that
> leads to the restyle?)

The restyle is called here: http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2210

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Xidorn Quan (UTC-7) from comment #9)
> The restyle is called here:
> http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.
> cpp#2210


Hmmm.  This should already imply ForceDescendants.  I wonder if my patch in bug 1074634, or something else related, would help.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8498280 [details] [diff] [review]
patch

We should take this anyway, but we should figure out what the other problem here is.

r=dbaron

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #10)
> (In reply to Xidorn Quan (UTC-7) from comment #9)
> > The restyle is called here:
> > http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.
> > cpp#2210
> 
> Hmmm.  This should already imply ForceDescendants.  I wonder if my patch in
> bug 1074634, or something else related, would help.

No, it doesn't. The testcase still crashes after that patch applied. We may need to look deeper into the testcase then. I found the "float: left" be one of the necessary triggers, and the only useful element style in the testcase for this bug.

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8498280 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 966166
[User impact if declined]: The browser could be crashed by some code
[Describe test coverage new/current, TBPL]: no
[Risks and why]: No risk from my point of view. It fixes the incomplete lifetime management of CounterStyle instances.
[String/UUID change made/needed]: n/a

When uplifting to beta and aurora, you may want to use attachment 8498362 [details] [diff] [review] instead of this one, as the patch for bug 1066089 wasn't uplifted to those versions.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/215212e08b53
https://hg.mozilla.org/releases/mozilla-aurora/rev/ee3cd82e3acb
https://hg.mozilla.org/releases/mozilla-beta/rev/d79568d581e6

Please make sure this test gets checked in at some point.

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/215212e08b53

Comment 16

[Mats Palmgren (inactive)]

IsBullet() is a virtual method and CustomCounterStyle is allocated from
the general heap, so this looks potentially exploitable to me.

Comment 17

[Mats Palmgren (inactive)]

Xidorn, please file a follow-up bug (make it security-sensitive for now) to
"figure out what the other problem here is" per dbaron in comment 11.  Thanks.

Comment 18

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Mats Palmgren (:mats) from comment #16)
> IsBullet() is a virtual method and CustomCounterStyle is allocated from
> the general heap, so this looks potentially exploitable to me.

IsBullet() has to be a virtual method, but it might be removed later. The problem here I think is unrelated to the specific method, but the lifetime management of both CounterStyle and style structs.

Is there any better way to allocate it so that it could be safer?

Comment 19

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Mats Palmgren (:mats) from comment #17)
> Xidorn, please file a follow-up bug (make it security-sensitive for now) to
> "figure out what the other problem here is" per dbaron in comment 11. 
> Thanks.

The follow-up bug is bug 1077687.

Comment 20

[Mats Palmgren (inactive)]

Allocating the instances of the CounterStyle sub-classes from the pres shell arena
would have made this crash non-exploitable, but since they are ref-counted that's
a bit tricky to implement, IIRC.  Let me see if I can find an example of that...

Style structs are already allocated from the pres shell arena.

Comment 21

[Mats Palmgren (inactive)]

Perhaps it's not as tricky as I first thought;  nsRuleNode is an example that is
ref-counted and allocated from the shell arena.  It looks like you just need
to override Destroy() to explicitly call the destructor and then FreeByObjectID,
see nsRuleNode::DestroyInternal.

Comment 22

[Xidorn Quan [:xidorn] UTC+11]

Attachment: simplified testcase

Comment 23

[Mats Palmgren (inactive)]

FYI, you should wait with landing the crashtest until the bug is public, in case
you didn't know.

Comment 24

[Mats Palmgren (inactive)]

Regarding the arena allocation, this is a problem:
http://mxr.mozilla.org/mozilla-central/source/layout/style/CounterStyleManager.cpp#615
because when the pres shell is destroyed everything in the arena
must have been deleted already, so we can't have things there that
outlives the pres shell.  So you should exclude this class and
continue to allocate it from the general heap.  (It's not a security
risk because it's never freed if I understand correctly.)

Comment 25

[Mats Palmgren (inactive)]

(Perhaps we don't even allocate those from the heap?
It looks like they live in a static gBuiltinStyleTable array.)

Comment 26

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Mats Palmgren (:mats) from comment #25)
> (Perhaps we don't even allocate those from the heap?
> It looks like they live in a static gBuiltinStyleTable array.)

That's right. They are never a problem.

Comment 27

[Mats Palmgren (inactive)]

OK, good.  Feel free to file a separate bug about switching to arena allocation,
if you think it's doable.  I'll be happy to review any patches there.

Comment 28

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Mats Palmgren (:mats) from comment #27)
> OK, good.  Feel free to file a separate bug about switching to arena
> allocation,
> if you think it's doable.  I'll be happy to review any patches there.

Bug 1077718

Comment 29

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue several times using the following:
- poc from comment #0
- Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1412270005/

Went through the following verification:

fx35: PASSED
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1412602997/

fx34: PASSED
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-asan/1412612510/

fx33: PASSED
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-linux64-asan/1412598973/

Test Cases used for all of the above channels:

- opened the poc in several regular window/tab's without any issues
- opened the poc in several private window/tab's without any issues
- opened the poc in several e10s window/tab's without any issues [m-c specific]

Comment 30

[Daniel Veditz [:dveditz]]

if this is a regression from bug 966166 then Firefox 32 should not be affected.