441368 - Crash [@ nsSVGFEGaussianBlurElement::SetupPredivide] opening SVG file

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Brandon Sterne (:bsterne)]

Attachment: testcase crashes trunk

Drew Yao from Apple Product Security reported this crash to security@mo.  The attached testcase crashes trunk on Mac and Linux (haven't tested it on Windows).  I will follow up shortly to attach the stack contents from the Mac crash.

From Drew's email:

Integer overflow in nsSVGFEGaussianBlurElement::SetupPredivide
It does not seem to affect Firefox 2, only Firefox 3.

PRUint8 *
nsSVGFEGaussianBlurElement::SetupPredivide(PRUint32 size) const
{
  PRUint8 *tmp = new PRUint8[size * 256];   // <-- integer overflow possible here, leading to an unexpectedly small buffer, and memory corruption
  if (tmp) {
    for (PRUint32 i = 0; i < 256; i++)
      memset(tmp + i * size, i, size);
  }
  return tmp;
}

To reproduce:
Open the attached 00001596.svg in Firefox 3 in Mac OS X 10.5.3 Intel.

It has the line
<feGaussianBlur in="SourceAlpha" stdDeviation="2147483648" result="blur"/>

Breakpoint 13, nsSVGFEGaussianBlurElement::SetupPredivide (this=0x1c4adaa0, size=2018542109) at /Volumes/data_apps/mozilla/content/svg/content/src/nsSVGFilters.cpp:774
774      PRUint8 *tmp = new PRUint8[size * 256];
(gdb) p size
$1001 = 2018542109
(gdb) p/x size
$1002 = 0x7850821d
(gdb) p/x size * 256
$1003 = 0x50821d00   #smaller than size


Process:         firefox-bin [55069]
Path:            /Volumes/data_apps/obj-i386-apple-darwin9.3.0/dist/MinefieldDebug.app/Contents/MacOS/firefox-bin
Identifier:      org.mozilla.firefox
Version:         3.0pre (3.0pre)
Code Type:       X86 (Native)
Parent Process:  launchd [1]

Date/Time:       2008-06-05 16:25:56.043 -0700
OS Version:      Mac OS X 10.5.3 (9D34)
Report Version:  6

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x000000006f4eb000
Crashed Thread:  0

Thread 0 Crashed:
0   libSystem.B.dylib                 0xffff06b0 __bzero + 176
1   libgklayout.dylib                 0x129ae722 nsSVGFEGaussianBlurElement::SetupPredivide(unsigned int) const + 72 (nsSVGFilters.cpp:776)
2   libgklayout.dylib                 0x129b5fb3 nsSVGFEGaussianBlurElement::GaussianBlur(unsigned char*, unsigned char*, nsSVGFilterResource*, unsigned int, unsigned int) + 255 (nsSVGFilters.cpp:828)
3   libgklayout.dylib                 0x129c2c63 nsSVGFEGaussianBlurElement::Filter(nsSVGFilterInstance*) + 537 (nsSVGFilters.cpp:896)
4   libgklayout.dylib                 0x1296a831 nsSVGFilterFrame::FilterPaint(nsSVGRenderState*, nsISVGChildFrame*) + 4063 (nsSVGFilterFrame.cpp:513)
5   libgklayout.dylib                 0x1299034a nsSVGUtils::PaintChildWithEffects(nsSVGRenderState*, nsRect*, nsIFrame*) + 698 (nsSVGUtils.cpp:1378)
6   libgklayout.dylib                 0x1297da08 nsSVGOuterSVGFrame::Paint(nsIRenderingContext&, nsRect const&, nsPoint) + 466 (nsSVGOuterSVGFrame.cpp:583)
7   libgklayout.dylib                 0x1297dbc6 nsDisplaySVG::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) + 74 (nsSVGOuterSVGFrame.cpp:466)
8   libgklayout.dylib                 0x122c0b91 nsDisplayList::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) const + 61 (nsDisplayList.cpp:295)
9   libgklayout.dylib                 0x122c1dbb nsDisplayWrapList::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) + 41 (nsDisplayList.cpp:694)
10  libgklayout.dylib                 0x122c2a50 nsDisplayClip::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&) + 128 (nsDisplayList.cpp:888)
...

Comment 1

[Brandon Sterne (:bsterne)]

Attachment: Backtrace from SVG crash

Comment 2

[Drew Yao]

A simple fix would be something like:
if (size > UINT_MAX / 256) return NULL;

Comment 3

[Brandon Sterne (:bsterne)]

Gavin or Dan, do you know who could take this?

Comment 4

[Daniel Veditz [:dveditz]]

Looks like this code was added with bug 399488

Comment 5

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

It's not very exploitable since we fill the buffer with only N 0s, N 1s, N 2s, ... N 255s. But I'll patch it.

Comment 6

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

This is not a problem on trunk since I've already removed this code and replaced it with a better approach (that doesn't allocate).

Comment 7

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

Attachment: fix

Simple stuff.

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 328775 [details] [diff] [review]
fix

sr=mats

Comment 9

[Samuel Sidler (old account; do not CC)]

Comment on attachment 328775 [details] [diff] [review]
fix

Approved for 1.9.0.2. Please land in CVS. a=ss

We should also get this in the testsuite...

Comment 10

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

Checked into 1.9.0 with test. I also landed the test on trunk, changeset b936f6ac877d.

Comment 11

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2pre) Gecko/2008082004 GranParadiso/3.0.2pre. No crash using the testcase.

Comment 12

[Marc Bejarano]

for future reference, this is CVE-2008-4064