351501 - Null deref crash [@ nsSVGUtils::CoordToFloat] with large percentage "stroke-width"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Tom Ferris]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

A denial of service (null pointer) vulnerability exists within firefox 1.0.5.6 when processig a specially crafted .svg file.  This was tested on OS X 10.4.7 (x86).  Below is a testcase to trigger this issue:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" baseProfile="full">
<text stroke-width="1234567890%">
</text>
</svg>

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x001b1cdc in nsSVGUtils::CoordToFloat ()
(gdb) bt
#0  0x001b1cdc in nsSVGUtils::CoordToFloat ()
#1  0x00556140 in nsSVGGlyphFrame::GetStrokeWidth ()
#2  0x005563e6 in nsSVGGlyphFrame::GetStrokePaintType ()
#3  0x004d6c30 in nsSVGCairoGlyphGeometry::GetCoveredRegion ()
#4  0x004d5c18 in nsSVGCairoGlyphGeometry::Update ()
#5  0x005558fc in nsSVGGlyphFrame::UpdateGeometry ()
#6  0x0054a1ba in nsSVGTextFrame::UpdateGlyphPositioning ()
#7  0x0054ab07 in nsSVGTextFrame::NotifyRedrawUnsuspended ()
#8  0x0058bb9a in nsSVGOuterSVGFrame::UnsuspendRedraw ()
#9  0x0058c0cd in nsSVGOuterSVGFrame::DidReflow ()
#10 0x00531d0f in CanvasFrame::Reflow ()


Reproducible: Always

Comment 1

[Tom Ferris]

Attachment: crash testcase

crash testcase

Comment 2

[Bernd]

Attachment: patch

do_query can return null...

Comment 3

[Bernd]

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

this fixes the do_query error but there might be more.

Comment 4

[Bernd]

Daniel can't we lift the security on this? After all its a plain zero deref

Comment 5

[tor]

Attachment: use the right content node

Deja-vu (again).  We've run across this problem before - this is the appropriate fix.

Comment 6

[Bernd]

I think its bad style to not check the return of do_query in other words: http://lxr.mozilla.org/seamonkey/search?string=%3D+do_QueryInterface%28aContent%29
this is the first time I see in layout that a do_query is not verified.

Comment 7

[Scooter Morris]

Comment on attachment 236991 [details] [diff] [review]
use the right content node

Yup, yet another glyph parent is not what you think!  Nit: is it worth a comment above the call to CoordToFloat to explain why we're calling mContent->GetParent()?

With that, r=scooter

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 236991 [details] [diff] [review]
use the right content node

I do think you should also check the do_QueryInterface as bernd suggested.

Comment 9

[tor]

Attachment: Add bernd's null check and comment

Crash fix, low risk, similar fix has been on trunk since 2006-07-19 (bug 344892).

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 237747 [details] [diff] [review]
Add bernd's null check and comment

a=beltzner on behalf of 181drivers

Comment 11

[tor]

Checked in on MOZILLA_1_8_BRANCH.

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 237747 [details] [diff] [review]
Add bernd's null check and comment

approved for 1.8.0 branch, a=dveditz for drivers

Comment 13

[tor]

Checked in on MOZILLA_1_8_0_BRANCH.

Comment 14

[Carsten Book [:Tomcat]]

Verified Fixed for 1.8.0.8 on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.8) Gecko/20061025 Firefox/1.5.0.8