Closed Bug 780963 Opened 12 years ago Closed 12 years ago

invalid cast with svg feImage

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17
Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 + verified
firefox-esr10 --- unaffected

People

(Reporter: miaubiz, Assigned: jwatt)

References

Details

(6 keywords, Whiteboard: [asan])

Attachments

(7 files, 1 obsolete file)

Attached file 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
Attached file asan log linux
Attached file asan log osx
Component: Security → SVG
Product: Firefox → Core
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.
Keywords: testcase
Whiteboard: [asan]
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
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [asan] → [asan][sg:critical]
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?
Keywords: crash, regression
Here's a tweaked testcase, with more of the SVG existing up-front rather than being created through script.
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.
Assignee: nobody → jwatt
Keywords: sec-critical
Whiteboard: [asan][sg:critical] → [asan]
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.
Attached patch patch (obsolete) — Splinter Review
Attachment #651423 - Flags: review?(roc)
Attachment #651427 - Flags: review?(roc)
Attachment #651423 - Attachment is obsolete: true
Attachment #651423 - Flags: review?(roc)
Comment on attachment 651427 [details] [diff] [review] patch with crashtest Review of attachment 651427 [details] [diff] [review]: ----------------------------------------------------------------- Don't check in the testcase yet
Attachment #651427 - Flags: review?(roc) → review+
Attachment #651672 - Flags: checkin?(jwatt)
Whiteboard: [asan] → [asan][leave open]
This only affects nightly users. Given that, how long should I wait before checking in the test?
Oh, I guess you can check that in now then :-).
Flags: in-testsuite? → in-testsuite+
Whiteboard: [asan][leave open] → [asan]
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 784061
Keywords: verifyme
Group: core-security
Attachment #651672 - Flags: checkin?(jwatt)
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.
Attachment #651672 - Flags: checkin+
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: