Closed Bug 795592 Opened 7 years ago Closed 7 years ago

invalid cast leading to out of bounds read in nsSVGUtils::GetCanvasTM

Categories

(Core :: SVG, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox16 --- unaffected
firefox17 + fixed
firefox18 + fixed
firefox19 + fixed
firefox-esr10 --- unaffected

People

(Reporter: inferno, Assigned: jwatt)

References

Details

(5 keywords, Whiteboard: [asan][adv-track-main17+])

Attachments

(2 files, 1 obsolete file)

Attached file Testcase
Reproduces on trunk.

=================================================================
==16818== ERROR: AddressSanitizer global-buffer-overflow on address 0x7f447c664720 at pc 0x7f446971b902 bp 0x7ffff465e830 sp 0x7ffff465e828
READ of size 8 at 0x7f447c664720 thread T0
    #0 0x7f446971b901 in nsSVGUtils::GetCanvasTM(nsIFrame*, unsigned int) src/layout/svg/nsSVGUtils.cpp:695
    #1 0x7f44695feb6d in nsAutoFilterInstance src/layout/svg/nsSVGFilterFrame.cpp:211
    #2 0x7f446960a67d in nsSVGFilterFrame::GetPostFilterBounds(nsIFrame*, gfxRect const*, nsRect const*) src/layout/svg/nsSVGFilterFrame.cpp:512
    #3 0x7f4469716dc1 in nsSVGUtils::GetPostFilterVisualOverflowRect(nsIFrame*, nsRect const&) src/layout/svg/nsSVGUtils.cpp:370
    #4 0x7f4463f86eb3 in ComputeOutlineAndEffectsRect(nsIFrame*, bool*, nsRect const&, nsSize const&, bool) src/layout/generic/nsFrame.cpp:4996
    #5 0x7f4463f8341a in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize) src/layout/generic/nsFrame.cpp:6975
    #6 0x7f4463fb06b8 in nsIFrame::RecomputePerspectiveChildrenOverflow(nsStyleContext const*, nsRect const*) src/layout/generic/nsFrame.cpp:7103
    #7 0x7f4463f84239 in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize) src/layout/generic/nsFrame.cpp:7013
    #8 0x7f4463fb06b8 in nsIFrame::RecomputePerspectiveChildrenOverflow(nsStyleContext const*, nsRect const*) src/layout/generic/nsFrame.cpp:7103
    #9 0x7f4463fb0868 in nsIFrame::RecomputePerspectiveChildrenOverflow(nsStyleContext const*, nsRect const*) src/layout/generic/nsFrame.cpp:7109
    #10 0x7f4463f84239 in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize) src/layout/generic/nsFrame.cpp:7013
    #11 0x7f4463d31ccf in nsIFrame::FinishAndStoreOverflow(nsHTMLReflowMetrics*) src/layout/svg/../generic/nsIFrame.h:2375
    #12 0x7f44696c0d52 in nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/svg/nsSVGOuterSVGFrame.cpp:459
    #13 0x7f446418bd08 in nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, bool&) src/layout/generic/nsLineLayout.cpp:831
    #14 0x7f4463e44bc1 in nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:3836
    #15 0x7f4463e3ef95 in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3632
    #16 0x7f4463e31979 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3484
    #17 0x7f4463e20737 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2571
    #18 0x7f4463e072a7 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:2021
    #19 0x7f4463df9ecd in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1070
    #20 0x7f4463e89030 in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) src/layout/generic/nsBlockReflowContext.cpp:268
    #21 0x7f4463e2bdb9 in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3208
    #22 0x7f4463e20189 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2515
    #23 0x7f4463e072a7 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) src/layout/generic/nsBlockFrame.cpp:2021
    #24 0x7f4463df9ecd in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsBlockFrame.cpp:1070
    #25 0x7f4463ee8852 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:946
    #26 0x7f44640bc9e4 in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsCanvasFrame.cpp:472
    #27 0x7f4463ee8852 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:946
    #28 0x7f44640316a8 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) src/layout/generic/nsGfxScrollFrame.cpp:524
    #29 0x7f4464036246 in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) src/layout/generic/nsGfxScrollFrame.cpp:624
    #30 0x7f446403a8a0 in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsGfxScrollFrame.cpp:865
    #31 0x7f4463ee8852 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:946
    #32 0x7f446442392f in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) src/layout/generic/nsViewportFrame.cpp:200
    #33 0x7f4463b6f56c in PresShell::DoReflow(nsIFrame*, bool) src/layout/base/nsPresShell.cpp:7372
    #34 0x7f4463b9bfe7 in PresShell::ProcessReflowCommands(bool) src/layout/base/nsPresShell.cpp:7518
    #35 0x7f4463b9a30f in PresShell::FlushPendingNotifications(mozFlushType) src/layout/base/nsPresShell.cpp:3848
    #36 0x7f4463c3d28b in nsRefreshDriver::Notify(nsITimer*) src/layout/base/nsRefreshDriver.cpp:398
    #37 0x7f446f1da49d in nsTimerImpl::Fire() src/xpcom/threads/nsTimerImpl.cpp:476
    #38 0x7f446f1db8ba in nsTimerEvent::Run() src/xpcom/threads/nsTimerImpl.cpp:556
    #39 0x7f446f19efc0 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:612
    #40 0x7f446ee3190b in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220
    #41 0x7f446d8e17c6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
    #42 0x7f446f456f91 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:208
    #43 0x7f446f456dc6 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:201
    #44 0x7f446f456cab in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:175
    #45 0x7f446cd91cda in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
    #46 0x7f446b9c7a04 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:290
    #47 0x7f44620dc9cd in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3782
    #48 0x7f44620e2845 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3848
    #49 0x7f44620e56f4 in XRE_main src/toolkit/xre/nsAppRunner.cpp:3923
    #50 0x40d013 in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174
    #51 0x40a755 in main src/browser/app/nsBrowserApp.cpp:279
    #52 0x7f447fe3cc4c in ?? ??:0
0x7f447c664720 is located 0 bytes to the right of global variable 'vtable for SVGFEImageFrame (src/layout/svg/SVGFEImageFrame.cpp)' (0x7f447c6642c0) of size 1120
Shadow byte and word:
  0x1fe88f8cc8e4: f9
  0x1fe88f8cc8e0: 00 00 00 00 f9 f9 f9 f9
More shadow bytes:
  0x1fe88f8cc8c0: 00 00 00 00 00 00 00 00
  0x1fe88f8cc8c8: 00 00 00 00 00 00 00 00
  0x1fe88f8cc8d0: 00 00 00 00 00 00 00 00
  0x1fe88f8cc8d8: 00 00 00 00 00 00 00 00
=>0x1fe88f8cc8e0: 00 00 00 00 f9 f9 f9 f9
  0x1fe88f8cc8e8: 00 00 00 00 00 00 00 00
  0x1fe88f8cc8f0: 00 00 00 00 00 00 00 00
  0x1fe88f8cc8f8: 00 00 00 00 00 00 00 00
  0x1fe88f8cc900: 00 00 00 00 00 00 00 00
Stats: 217M malloced (232M for red zones) by 347865 calls
Stats: 41M realloced by 19837 calls
Stats: 181M freed by 220209 calls
Stats: 47M really freed by 145703 calls
Stats: 424M (108620 full pages) mmaped in 106 calls
  mmaps   by size class: 8:245745; 9:40955; 10:12285; 11:14329; 12:2048; 13:1536; 14:1280; 15:256; 16:448; 17:1248; 18:128; 19:40; 20:20;
  mallocs by size class: 8:268391; 9:45268; 10:10586; 11:15989; 12:2275; 13:1643; 14:1409; 15:296; 16:514; 17:1291; 18:145; 19:39; 20:19;
  frees   by size class: 8:157861; 9:36244; 10:7320; 11:12842; 12:1441; 13:1203; 14:1225; 15:249; 16:446; 17:1276; 18:49; 19:37; 20:16;
  rfrees  by size class: 8:114535; 9:18051; 10:3815; 11:7655; 12:431; 13:402; 14:249; 15:132; 16:325; 17:79; 18:24; 19:4; 20:1;
Stats: malloc large: 1494 small slow: 1770
==16818== ABORTING
Component: General → SVG
Product: Firefox → Core
Regression from bug 768351?  Or miiiight've been one of dzbarsky's animation patches, since there are animations in the testcase, but I doubt it.
The assert at the beginning of nsSVGFilterFrame::GetPostFilterBounds goes off before the crash (the filtered frame is NONDISPLAY_CHILD). The crash is caused by casting a frame that isn't a GeometryFrame to be a GeometryFrame but we need to prevent it calling that function. Not sure where/how to block it though.
(dbolter: regression window is already tracked down here -- see comment 2. --> removing "regressionwindow-wanted")
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → longsonr
Attachment #668921 - Flags: review?(jwatt)
Attached patch patchSplinter Review
(In reply to Robert Longson from comment #4)
> Not sure where/how to block it though.

I think we should stop things earlier. Specifically I think we should prevent FinishAndStoreOverflow being called on nondisplay SVG.
Attachment #675403 - Flags: review?
Comment on attachment 675403 [details] [diff] [review]
patch

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

Looks good to me.
Attachment #675403 - Flags: review? → review+
> +               "Don't call - overflow rects not maintain on these SVG frames");

s/maintain/maintained/
Attachment #668921 - Attachment is obsolete: true
Attachment #668921 - Flags: review?(jwatt)
Assignee: longsonr → jwatt
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/4b2d566ad1ac

(In reply to Robert Longson from comment #9)
> s/maintain/maintained/

I pushed before I saw this comment, so I pushed that as a follow-up:

https://hg.mozilla.org/integration/mozilla-inbound/rev/113115cd43f8
https://hg.mozilla.org/mozilla-central/rev/4b2d566ad1ac
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
This patch also fixed bug 784061
Looks like we still need branch uplift nominations on this. Please nominate early so we have time to land this before our next beta.
Comment on attachment 675403 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): probably bug 768351
User impact if declined: sg crash
Testing completed (on m-c, etc.): baked on m-c
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Attachment #675403 - Flags: approval-mozilla-beta?
Attachment #675403 - Flags: approval-mozilla-aurora?
Attachment #675403 - Flags: approval-mozilla-beta?
Attachment #675403 - Flags: approval-mozilla-beta+
Attachment #675403 - Flags: approval-mozilla-aurora?
Attachment #675403 - Flags: approval-mozilla-aurora+
Yeah, we should check in the attached testcase once the patch is in the hands of our users.
Whiteboard: [adv-track-main17+]
Blocks: 780764
Flags: sec-bounty?
Summary: Global-buffer-overflow in nsSVGUtils::GetCanvasTM → invalid cast leading to out of bounds read in nsSVGUtils::GetCanvasTM
This bug qualifies for the security bug bounty.
Flags: sec-bounty? → sec-bounty+
Whiteboard: [adv-track-main17+] → [asan][adv-track-main17+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.