Last Comment Bug 780963 - invalid cast with svg feImage
: invalid cast with svg feImage
Status: RESOLVED FIXED
[asan]
: crash, csectype-wildptr, regression, sec-critical, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Jonathan Watt [:jwatt]
:
: Jet Villegas (:jet)
Mentors:
Depends on: 784061
Blocks: 614732
  Show dependency treegraph
 
Reported: 2012-08-07 12:31 PDT by miaubiz
Modified: 2014-07-24 13:43 PDT (History)
8 users (show)
rforbes: sec‑bounty+
jwatt: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
+
verified
unaffected


Attachments
repro case (817 bytes, text/plain)
2012-08-07 12:31 PDT, miaubiz
no flags Details
asan log linux (1.85 KB, text/plain)
2012-08-07 12:32 PDT, miaubiz
no flags Details
asan log osx (1.59 KB, text/plain)
2012-08-07 12:32 PDT, miaubiz
no flags Details
crash wrangler log (osx lion) nightly (47.58 KB, text/plain)
2012-08-07 12:33 PDT, miaubiz
no flags Details
testcase 2 (warning: crashes nightly builds) (658 bytes, text/html)
2012-08-08 11:50 PDT, Daniel Holbert [:dholbert]
no flags Details
patch (6.59 KB, patch)
2012-08-13 09:36 PDT, Jonathan Watt [:jwatt]
no flags Details | Diff | Splinter Review
patch with crashtest (8.65 KB, patch)
2012-08-13 09:47 PDT, Jonathan Watt [:jwatt]
roc: review+
Details | Diff | Splinter Review
crashtest - to be checked in later (1.15 KB, patch)
2012-08-14 02:23 PDT, Jonathan Watt [:jwatt]
jwatt: checkin+
Details | Diff | Splinter Review

Description miaubiz 2012-08-07 12:31:56 PDT
Created attachment 649751 [details]
repro case

I load:

<html>
  <head>
    <script>
      onload = function() {
        el0=document.createElementNS('http://www.w3.org/2000/svg', 'svg')
        document.body.appendChild(el0)
        el0.appendChild(document.createElementNS('http://www.w3.org/2000/svg', 'g'))
        el1=document.createElementNS('http://www.w3.org/2000/svg', 'filter')
        el1.setAttribute('id','f1')
        el1.setAttribute('filterUnits', 'userSpaceOnUse')
        el0.appendChild(el1)
        el2=document.createElementNS('http://www.w3.org/2000/svg', 'feImage')
        el1.appendChild(el2)
        document.body.offsetTop
        el2.setAttribute('filter', 'url(#f1)')
        document.body.offsetTop
        el2.appendChild(document.createElementNS('http://www.w3.org/2000/svg', 'g'))
      }
    </script>
  </head>
  <body>
  </body>
</html>

and then this happens:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ???                           	000000000000000000 0 + 0
1   XUL                           	0x000000010190240c nsSVGUtils::GetCanvasTM(nsIFrame*, unsigned int) + 204
2   XUL                           	0x00000001018e539d nsAutoFilterInstance::nsAutoFilterInstance(nsIFrame*, nsSVGFilterFrame*, nsSVGFilterPaintCallback*, nsRect const*, nsRect const*, nsRect const*, gfxRect const*) + 1245

--

==15809== ERROR: AddressSanitizer global-buffer-overflow on address 0x7ffff3dce6a0 at pc 0x7fffefa45527 bp 0x7fffffff5b00 sp 0x7fffffff5af8
READ of size 8 at 0x7ffff3dce6a0 thread T0
    #0 0x7fffefa45527 in nsSVGUtils::GetCanvasTM(nsIFrame*, unsigned int) /builds/slave/try-lnx64/build/media/libvpx/vp8/encoder/x86/quantize_mmx.asm:0
0x7ffff3dce6a0 is located 0 bytes to the right of global variable 'vtable for SVGFEImageFrame (/builds/slave/try-lnx64/build/layout/svg/base/src/SVGFEImageFrame.cpp)' (0x7ffff3dce240) of size 1120

---

=================================================================
==19228== ERROR: AddressSanitizer global-buffer-overflow on address 0x00010a8b48a0 at pc 0x106318f2d bp 0x7fff5fbf6dc0 sp 0x7fff5fbf6db8
READ of size 8 at 0x00010a8b48a0 thread T0
    #0 0x106318f2d in 0x01ee5f2d (in XUL)
0x00010a8b48a0 is located 0 bytes to the right of global variable '_ZTV15SVGFEImageFrame (/Users/.../firefox/layout/svg/base/src/SVGFEImageFrame.cpp)' (0x10a8b4440) of size 1
Comment 1 miaubiz 2012-08-07 12:32:27 PDT
Created attachment 649752 [details]
asan log linux
Comment 2 miaubiz 2012-08-07 12:32:56 PDT
Created attachment 649754 [details]
asan log osx
Comment 3 miaubiz 2012-08-07 12:33:28 PDT
Created attachment 649755 [details]
crash wrangler log (osx lion) nightly
Comment 4 Daniel Veditz [:dveditz] 2012-08-08 10:30:02 PDT
Daniel: can you tell what's going on here? One of the crashes looks like a null deref perhaps (deleted nsCOMPtr?), but the other two are possibly more troubling.
Comment 5 Daniel Holbert [:dholbert] 2012-08-08 11:14:50 PDT
Yeah, this is bad.

I can repro the crash in an opt build and a debug build.

In my debug build, we crash at the last line here:
> gfxMatrix
> nsSVGUtils::GetCanvasTM(nsIFrame *aFrame, PRUint32 aFor)
> {
[...]
>  return static_cast<nsSVGGeometryFrame*>(aFrame)->GetCanvasTM(aFor);

and we crash because aFrame is _not_ a nsSVGGeometryFrame! (so the static_cast is invalid, and the method-call after that is bogus)

Instead, aFrame is a SVGFEImageFrame, which derives directly from nsFrame:
https://mxr.mozilla.org/mozilla-central/source/layout/svg/base/src/SVGFEImageFrame.cpp#16
Comment 6 Daniel Holbert [:dholbert] 2012-08-08 11:23:56 PDT
Last good nightly: 2012-07-20
First bad nightly: 2012-07-21

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3a05d298599e&tochange=045c11dd41a6

That pushlog has a lot of SVG changes from jwatt... jwatt, mind taking a look?
Comment 7 Daniel Holbert [:dholbert] 2012-08-08 11:31:42 PDT
mozilla-inbound regression range:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fe71c51ee0b9&tochange=c5cd832d82ef

Looks like a regression from Bug 614732.
Comment 8 Daniel Holbert [:dholbert] 2012-08-08 11:50:05 PDT
Created attachment 650227 [details]
testcase 2 (warning: crashes nightly builds)

Here's a tweaked testcase, with more of the SVG existing up-front rather than being created through script.
Comment 9 Daniel Holbert [:dholbert] 2012-08-08 11:53:05 PDT
Assigning to jwatt, since this appears to be a regression from Bug 614732 (per comment 6-7) and because this needs fixing soon & should have an assignee.

jwatt, if you don't have cycles for this, I could potentially take it, too.
Comment 10 Jonathan Watt [:jwatt] 2012-08-13 09:31:29 PDT
The crash stack looks like this:

  nsSVGUtils::GetCanvasTM
  nsAutoFilterInstance::nsAutoFilterInstance
  nsSVGFilterFrame::GetPostFilterBounds
  nsSVGIntegrationUtils::ComputePostEffectsVisualOverflowRect
  ComputeOutlineAndEffectsRect
  nsIFrame::FinishAndStoreOverflow
  nsFrame::UpdateOverflow
  nsCSSFrameConstructor::ProcessRestyledFrames
  mozilla::css::RestyleTracker::ProcessOneRestyle
  mozilla::css::RestyleTracker::DoProcessRestyles
  mozilla::css::RestyleTracker::ProcessRestyles
  nsCSSFrameConstructor::ProcessPendingRestyles

We don't want to call nsFrame::UpdateOverflow on NS_STATE_SVG_NONDISPLAY_CHILD frames like SVGFEImageFrame though.
Comment 11 Jonathan Watt [:jwatt] 2012-08-13 09:36:48 PDT
Created attachment 651423 [details] [diff] [review]
patch
Comment 12 Jonathan Watt [:jwatt] 2012-08-13 09:47:23 PDT
Created attachment 651427 [details] [diff] [review]
patch with crashtest
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-13 16:09:23 PDT
Comment on attachment 651427 [details] [diff] [review]
patch with crashtest

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

Don't check in the testcase yet
Comment 15 Jonathan Watt [:jwatt] 2012-08-14 02:23:00 PDT
Created attachment 651672 [details] [diff] [review]
crashtest - to be checked in later
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-08-14 17:57:03 PDT
https://hg.mozilla.org/mozilla-central/rev/ad77846165e3
Comment 17 Jonathan Watt [:jwatt] 2012-08-16 02:43:15 PDT
This only affects nightly users. Given that, how long should I wait before checking in the test?
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-08-16 03:12:49 PDT
Oh, I guess you can check that in now then :-).
Comment 19 Jonathan Watt [:jwatt] 2012-08-16 03:56:51 PDT
Cool. :-)

https://hg.mozilla.org/integration/mozilla-inbound/rev/d457d1d6504c
Comment 20 Ryan VanderMeulen [:RyanVM] 2012-08-16 18:03:11 PDT
https://hg.mozilla.org/mozilla-central/rev/d457d1d6504c
Comment 22 Virgil Dicu [:virgil] [QA] 2012-11-06 06:40:18 PST
Test cases from comment 0 and comment 8 reproducible for: 
Nighlty 17.0a1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 build ID:20120808030529

No crashes for test cases from comment 0 and comment 8 for
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 beta 4
Build ID: 20121031065642

No crashes for Mac 10.7.5 and Ubuntu 12.04.
Comment 23 Raymond Forbes[:rforbes] 2013-07-19 18:18:08 PDT
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Comment 24 Tracy Walker [:tracy] 2014-01-10 10:39:30 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.