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

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Abhishek Arya]

Attachment: 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

Comment 1

[Daniel Holbert [:dholbert]]

Testcases crashes Nightly builds: bp-e5b64006-c8cb-40d3-889d-5bc2f2120929

Regression range:
Last good nightly: 2012-07-31
First bad nightly: 2012-08-01
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9d2a7a8ca1c7&tochange=582d4c67b3a7

Comment 2

[Daniel Holbert [:dholbert]]

mozilla-inbound regression range (for the crash):

GOOD:
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/07/2012-07-31-mozilla-inbound-debug/firefox-17.0a1.en-US.debug-linux-x86_64.txt
BAD:
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012/08/2012-08-01-mozilla-inbound-debug/firefox-17.0a1.en-US.debug-linux-x86_64.txt

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cda1c40da986&tochange=59d1993da7e1

Comment 3

[Daniel Holbert [:dholbert]]

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.

Comment 4

[Robert Longson [:longsonr]]

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.

Comment 5

[Daniel Holbert [:dholbert]]

(dbolter: regression window is already tracked down here -- see comment 2. --> removing "regressionwindow-wanted")

Comment 6

[Robert Longson [:longsonr]]

Attachment: patch

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: patch

(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.

Comment 8

[Matt Woodrow (:mattwoodrow)]

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

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

Looks good to me.

Comment 9

[Robert Longson [:longsonr]]

> +               "Don't call - overflow rects not maintain on these SVG frames");

s/maintain/maintained/

Comment 10

[Jonathan Watt [: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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4b2d566ad1ac

Comment 12

[Robert Longson [:longsonr]]

This patch also fixed bug 784061

Comment 13

[Robert Longson [:longsonr]]

and bug 780764

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

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 15

[Jonathan Watt [:jwatt]]

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/563063ce23e6
https://hg.mozilla.org/releases/mozilla-aurora/rev/f912173e885c

https://hg.mozilla.org/releases/mozilla-beta/rev/e1db8fc57e3b
https://hg.mozilla.org/releases/mozilla-beta/rev/ef3f9b1edeb9

Should there be a test for this?

Comment 17

[Jonathan Watt [:jwatt]]

Yeah, we should check in the attached testcase once the patch is in the hands of our users.

Comment 18

[Daniel Veditz [:dveditz]]

This bug qualifies for the security bug bounty.