Closed Bug 1133160 Opened 9 years ago Closed 9 years ago

Heap-buffer-overflow in mozilla::gfx::FilterSupport::PostFilterExtentsForPrimitive

Categories

(Core :: Graphics, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox36 --- unaffected
firefox37 --- unaffected
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: inferno, Assigned: milan)

References

Details

(Keywords: sec-moderate)

Attachments

(2 files)

Attached image Testcase
==18737==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000081744 at pc 0x7fec7e30689a bp 0x7fff473d7850 sp 0x7fff473d7848
READ of size 4 at 0x602000081744 thread T0 (Web Content)
    #0 0x7fec7e306899 in mozilla::gfx::FilterSupport::PostFilterExtentsForPrimitive(mozilla::gfx::FilterPrimitiveDescription const&, nsTArray<nsIntRegion> const&) gfx/src/FilterSupport.cpp:1482:13
    #1 0x7fec7e307638 in mozilla::gfx::FilterSupport::ComputePostFilterExtents(mozilla::gfx::FilterDescription const&, nsIntRegion const&) gfx/src/FilterSupport.cpp:1542:26
    #2 0x7fec82626b72 in nsFilterInstance::ComputePostFilterExtents() layout/svg/nsFilterInstance.cpp:513:5
    #3 0x7fec8262679d in nsFilterInstance::GetPostFilterBounds(nsIFrame*, gfxRect const*, nsRect const*) layout/svg/nsFilterInstance.cpp:147:10
    #4 0x7fec82683f5b in nsSVGUtils::GetPostFilterVisualOverflowRect(nsIFrame*, nsRect const&) layout/svg/nsSVGUtils.cpp:178:10
    #5 0x7fec8236f1cf in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize, nsSize*) layout/generic/nsFrame.cpp:5252:11
    #6 0x7fec8267587a in nsSVGPathGeometryFrame::ReflowSVG() layout/svg/nsSVGPathGeometryFrame.cpp:399:3
    #7 0x7fec8260b249 in nsSVGDisplayContainerFrame::ReflowSVG() layout/svg/nsSVGContainerFrame.cpp:356:7
    #8 0x7fec8266a164 in nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/svg/nsSVGOuterSVGFrame.cpp:436:5
    #9 0x7fec8227b41a in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:958:5
    #10 0x7fec8240cceb in nsInlineFrame::ReflowInlineFrame(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsIFrame*, unsigned int&) layout/generic/nsInlineFrame.cpp:773:3
    #11 0x7fec8240b9aa in nsInlineFrame::ReflowFrames(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsInlineFrame.cpp:657:7
    #12 0x7fec8240a91b in nsInlineFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsInlineFrame.cpp:433:3
    #13 0x7fec8227b41a in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:958:5
    #14 0x7fec822de4ea in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:3968:3
    #15 0x7fec822dcc05 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3766:5
    #16 0x7fec822d4093 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3632:9
    #17 0x7fec822c47d6 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2717:5
    #18 0x7fec822bc087 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1159:3
    #19 0x7fec8230b8cd in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsContainerFrame.cpp:977:3
    #20 0x7fec8230cb15 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:977:3
    #21 0x7fec823a433a in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:509:3
    #22 0x7fec823a5771 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:620:3
    #23 0x7fec823a787e in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:854:3
    #24 0x7fec823192b3 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:1022:3
    #25 0x7fec824e31fb in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsViewportFrame.cpp:216:7
    #26 0x7fec8222947c in PresShell::DoReflow(nsIFrame*, bool) layout/base/nsPresShell.cpp:9360:3
    #27 0x7fec8223d200 in PresShell::ProcessReflowCommands(bool) layout/base/nsPresShell.cpp:9520:24
    #28 0x7fec8223c534 in PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) layout/base/nsPresShell.cpp:4351:11
    #29 0x7fec81fd3e11 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:1645:11
    #30 0x7fec81fdc863 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) layout/base/nsRefreshDriver.cpp:198:5
    #31 0x7fec7cb72981 in nsTimerImpl::Fire() xpcom/threads/nsTimerImpl.cpp:631:7
    #32 0x7fec7cb735f0 in nsTimerEvent::Run() xpcom/threads/nsTimerImpl.cpp:724:3
    #33 0x7fec7cb69365 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:855:7
    #34 0x7fec7cbc1bec in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:265:10
    #35 0x7fec7d401f9e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:99:21
    #36 0x7fec7d3aea91 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:233:3
    #37 0x7fec819ddc8f in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:164:3
    #38 0x7fec834b2e13 in XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:738:12
    #39 0x7fec7d3aea91 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:233:3
    #40 0x7fec834b223c in XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:575:7
    #41 0x4db34e in content_process_main(int, char**) ipc/contentproc/plugin-container.cpp:211:19
    #42 0x7fec7a15dec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
0x602000081744 is located 4 bytes to the right of 16-byte region [0x602000081730,0x602000081740)
allocated by thread T0 (Web Content) here:
    #0 0x4b5b08 in __interceptor_malloc _asan_rtl_
    #1 0x7fec8952d6ed in moz_xmalloc memory/mozalloc/mozalloc.cpp:52:17
    #2 0x7fec7e30c89b in mozilla::gfx::CopyAttribute(unsigned int const&, mozilla::gfx::FilterAttribute*, void*) objdir-ff-asan/dist/include/mozilla/mozalloc.h:209:12
    #3 0x7fec7e32c3c1 in nsBaseHashtable<nsUint32HashKey, nsAutoPtr<mozilla::gfx::FilterAttribute>, mozilla::gfx::FilterAttribute*>::s_EnumReadStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) objdir-ff-asan/dist/include/nsBaseHashtable.h:391:25
    #4 0x7fec7cbc5cf2 in PL_DHashTableEnumerate(PLDHashTable*, PLDHashOperator (*)(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*), void*) xpcom/glue/pldhash.cpp:751:28
    #5 0x7fec7e30a0ad in mozilla::gfx::FilterPrimitiveDescription::FilterPrimitiveDescription(mozilla::gfx::FilterPrimitiveDescription const&) objdir-ff-asan/dist/include/nsBaseHashtable.h:174:12
    #6 0x7fec806472bf in mozilla::gfx::FilterPrimitiveDescription* nsTArray_Impl<mozilla::gfx::FilterPrimitiveDescription, nsTArrayInfallibleAllocator>::AppendElements<mozilla::gfx::FilterPrimitiveDescription, nsTArrayInfallibleAllocator>(nsTArray_Impl<mozilla::gfx::FilterPrimitiveDescription, nsTArrayInfallibleAllocator> const&) objdir-ff-asan/dist/include/nsTArray.h:486:34
    #7 0x7fec82627746 in nsFilterInstance::nsFilterInstance(nsIFrame*, nsIContent*, mozilla::dom::UserSpaceMetrics const&, nsTArray<nsStyleFilter> const&, nsSVGFilterPaintCallback*, gfxMatrix const&, nsRegion const*, nsRegion const*, nsRect const*, gfxRect const*) objdir-ff-asan/dist/include/nsTArray.h:824:53
    #8 0x7fec82626769 in nsFilterInstance::GetPostFilterBounds(nsIFrame*, gfxRect const*, nsRect const*) layout/svg/nsFilterInstance.cpp:139:20
    #9 0x7fec82683f5b in nsSVGUtils::GetPostFilterVisualOverflowRect(nsIFrame*, nsRect const&) layout/svg/nsSVGUtils.cpp:178:10
    #10 0x7fec8236f1cf in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize, nsSize*) layout/generic/nsFrame.cpp:5252:11
    #11 0x7fec8267587a in nsSVGPathGeometryFrame::ReflowSVG() layout/svg/nsSVGPathGeometryFrame.cpp:399:3
    #12 0x7fec8260b249 in nsSVGDisplayContainerFrame::ReflowSVG() layout/svg/nsSVGContainerFrame.cpp:356:7
    #13 0x7fec8266a164 in nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/svg/nsSVGOuterSVGFrame.cpp:436:5
    #14 0x7fec8227b41a in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:958:5
    #15 0x7fec8240cceb in nsInlineFrame::ReflowInlineFrame(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsIFrame*, unsigned int&) layout/generic/nsInlineFrame.cpp:773:3
    #16 0x7fec8240b9aa in nsInlineFrame::ReflowFrames(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsHTMLReflowMetrics&, unsigned int&) layout/generic/nsInlineFrame.cpp:657:7
    #17 0x7fec8240a91b in nsInlineFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsInlineFrame.cpp:433:3
    #18 0x7fec8227b41a in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) layout/generic/nsLineLayout.cpp:958:5
    #19 0x7fec822de4ea in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) layout/generic/nsBlockFrame.cpp:3968:3
    #20 0x7fec822dcc05 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) layout/generic/nsBlockFrame.cpp:3766:5
    #21 0x7fec822d4093 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp:3632:9
    #22 0x7fec822c47d6 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) layout/generic/nsBlockFrame.cpp:2717:5
    #23 0x7fec822bc087 in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsBlockFrame.cpp:1159:3
    #24 0x7fec8230b8cd in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsContainerFrame.cpp:977:3
    #25 0x7fec8230cb15 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:977:3
    #26 0x7fec823a433a in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) layout/generic/nsGfxScrollFrame.cpp:509:3
    #27 0x7fec823a5771 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) layout/generic/nsGfxScrollFrame.cpp:620:3
    #28 0x7fec823a787e in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsGfxScrollFrame.cpp:854:3
    #29 0x7fec823192b3 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) layout/generic/nsContainerFrame.cpp:1022:3

Shadow bytes around the buggy address:
  0x0c0480008290: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c04800082a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c04800082b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c04800082c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c04800082d0: fa fa 00 04 fa fa 00 04 fa fa 00 00 fa fa 00 04
=>0x0c04800082e0: fa fa 00 fa fa fa 00 00[fa]fa 00 04 fa fa 00 04
  0x0c04800082f0: fa fa 00 00 fa fa 00 04 fa fa 00 fa fa fa 00 00
  0x0c0480008300: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c0480008310: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c0480008320: fa fa fd fa fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c0480008330: fa fa fd fa fa fa 00 fa fa fa fd fa fa fa 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap 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
==18737==ABORTING
Flags: sec-bounty?
Never mind my previous comment, that's an unrelated Nightly crash.
Component: GFX: Color Management → Graphics
Looking at SVGFEColorMatrixElement::GetPrimitiveDescription, it appears we can have the color matrix values type with the size set by the caller, rather than "20" that we need for a 5x4 matrix.   FilterSupport::PostFilterExtentsForPrimitive, on the other hand, indexes element 19 in the color matrix values case, without checking for the size.
In most of the other places we check if there are exactly 20 elements in the matrix.
Assignee: nobody → milan
Comment on attachment 8566268 [details] [diff] [review]
Make sure filter color matrix has 20 elements. r=mstange

Thanks!
Attachment #8566268 - Flags: review?(mstange) → review+
Can we get into similar difficulties with component transfer? Around here

http://mxr.mozilla.org/mozilla-central/source/gfx/src/FilterSupport.cpp#1441

what makes sure coefficients.length() == 4 in release builds?
(In reply to Robert Longson from comment #8)
> Can we get into similar difficulties with component transfer? Around here
> 
> http://mxr.mozilla.org/mozilla-central/source/gfx/src/FilterSupport.cpp#1441
> 
> what makes sure coefficients.length() == 4 in release builds?

Nothing, it seems, it just relies on the creator of the PrimitiveDescription to ensure that. :-(
At least SVGFECompositeElement::GetPrimitiveDescription gets it right.
I don't know this code, but I don't think we can get anything but length 4 because of how we set it.

With the matrix one, this line gets us in trouble: https://dxr.mozilla.org/mozilla-central/source/dom/svg/SVGFEColorMatrixElement.cpp#111 because we take the values given, regardless of the size, so we can give 1 where 20 is needed.

With SVG_FECOMPOSITE_OPERATOR_ARITHMETIC, I think we always get the four numbers set in https://dxr.mozilla.org/mozilla-central/source/dom/svg/SVGFECompositeElement.cpp#124, but I may be way off.
I like the idea of Robert's patch from bug 1134103, and it can just replace this patch if we decide to go with it.  In the meantime, we'll get this one landed.  Try in comment 6 is green.
Keywords: checkin-needed
 My patch is incomplete and doesn't yet work. I'll open another bug for the changes I want to make.
Comment on attachment 8566268 [details] [diff] [review]
Make sure filter color matrix has 20 elements. r=mstange

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily. You might be able to read a few values from memory, but influencing the location of the read is probably very tricky.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Yes.

Which older supported branches are affected by this flaw?
None, the problem is only on Nightly at the moment.

If not all supported branches, which bug introduced the flaw?
Bug 1092634

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
No backports needed

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely, doesn't need much testing.
Attachment #8566268 - Flags: sec-approval?
Comment on attachment 8566268 [details] [diff] [review]
Make sure filter color matrix has 20 elements. r=mstange

If this is nightly only, it is all cool. No sec-approval+ needed.
Attachment #8566268 - Flags: sec-approval?
https://hg.mozilla.org/mozilla-central/rev/3615c8ae27ad
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Looks like you can read at most one filter value out of memory. Not very useful on its own but could maybe be helpful to bypass ASLR combined with another exploitable bug.
Flags: sec-bounty? → sec-bounty+
Keywords: sec-moderate
Group: core-security
You need to log in before you can comment on or make changes to this bug.