Last Comment Bug 441368 - Crash [@ nsSVGFEGaussianBlurElement::SetupPredivide] opening SVG file
: Crash [@ nsSVGFEGaussianBlurElement::SetupPredivide] opening SVG file
Status: VERIFIED FIXED
: [sg:critical?]
: regression, verified1.9.0.2
Product: Core
Classification: Components
Component: SVG
: 1.9.0 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Mozilla Corporation)
: General SVG Bugs
:
:
: 399488
  Show dependency treegraph
 
Reported: 2008-06-23 12:12 PDT by Brandon Sterne (:bsterne)
Modified: 2009-01-05 03:30 PST (History)
10 users (show)
roc: blocking1.9.1-
samuel.sidler+old: blocking1.9.0.2+
dveditz: wanted1.8.1.x-
roc: in‑testsuite+
See Also:
Crash Signature:


Attachments
testcase crashes trunk (678 bytes, image/svg+xml)
2008-06-23 12:12 PDT, Brandon Sterne (:bsterne)
no flags Details
Backtrace from SVG crash (55.57 KB, text/plain)
2008-06-23 13:28 PDT, Brandon Sterne (:bsterne)
no flags Details
fix (834 bytes, patch)
2008-07-09 17:04 PDT, Robert O'Callahan (:roc) (Mozilla Corporation)
longsonr: review+
matspal: superreview+
samuel.sidler+old: approval1.9.0.2+
Details | Diff | Splinter Review

Summon comment box

Description Brandon Sterne (:bsterne) 2008-06-23 12:12:06 PDT
Created attachment 326347 [details]
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) 2008-06-23 13:28:39 PDT
Created attachment 326365 [details]
Backtrace from SVG crash
Comment 2 Drew Yao 2008-07-09 14:59:50 PDT
A simple fix would be something like:
if (size > UINT_MAX / 256) return NULL;
Comment 3 Brandon Sterne (:bsterne) 2008-07-09 15:46:46 PDT
Gavin or Dan, do you know who could take this?
Comment 4 Daniel Veditz 2008-07-09 16:01:04 PDT
Looks like this code was added with bug 399488
Comment 5 Robert O'Callahan (:roc) (Mozilla Corporation) 2008-07-09 16:09:50 PDT
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) (Mozilla Corporation) 2008-07-09 16:11:37 PDT
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) (Mozilla Corporation) 2008-07-09 17:04:01 PDT
Created attachment 328775 [details] [review]
fix

Simple stuff.
Comment 8 Mats Palmgren [:mats] 2008-07-12 21:53:50 PDT
Comment on attachment 328775 [details] [review]
fix

sr=mats
Comment 9 Samuel Sidler (old account; do not CC) 2008-07-20 18:53:25 PDT
Comment on attachment 328775 [details] [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) (Mozilla Corporation) 2008-08-15 02:31:55 PDT
Checked into 1.9.0 with test. I also landed the test on trunk, changeset b936f6ac877d.
Comment 11 Marcia Knous [:marcia] 2008-08-20 12:09:06 PDT
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 2008-11-16 02:53:18 PST
for future reference, this is CVE-2008-4064

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