Crash [@ nsSVGFEGaussianBlurElement::SetupPredivide] opening SVG file

VERIFIED FIXED

Status

()

Core
SVG
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: bsterne, Assigned: roc)

Tracking

({regression, verified1.9.0.2})

1.9.0 Branch
regression, verified1.9.0.2
Points:
---
Bug Flags:
blocking1.9.1 -
blocking1.9.0.2 +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
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)
...
(Reporter)

Updated

10 years ago
Whiteboard: [sg:critical?]
(Reporter)

Comment 1

10 years ago
Created attachment 326365 [details]
Backtrace from SVG crash

Comment 2

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

Comment 3

10 years ago
Gavin or Dan, do you know who could take this?
Looks like this code was added with bug 399488
Assignee: nobody → jwatt
Blocks: 399488
Keywords: regression
Flags: wanted1.8.1.x-
Flags: blocking1.9.1?
Flags: blocking1.9.0.2?
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.
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).
Flags: blocking1.9.1? → blocking1.9.1-
Created attachment 328775 [details] [diff] [review]
fix

Simple stuff.
Assignee: jwatt → roc
Status: NEW → ASSIGNED
Attachment #328775 - Flags: superreview?(mats.palmgren)
Attachment #328775 - Flags: review?(longsonr)
Attachment #328775 - Flags: review?(longsonr) → review+

Comment 8

10 years ago
Comment on attachment 328775 [details] [diff] [review]
fix

sr=mats
Attachment #328775 - Flags: superreview?(mats.palmgren) → superreview+

Updated

10 years ago
Version: unspecified → 1.9.0 Branch
Attachment #328775 - Flags: approval1.9.0.2?
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...
Attachment #328775 - Flags: approval1.9.0.2? → approval1.9.0.2+
Flags: in-testsuite?
Flags: blocking1.9.0.2? → blocking1.9.0.2+
Checked into 1.9.0 with test. I also landed the test on trunk, changeset b936f6ac877d.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite? → in-testsuite+
Keywords: fixed1.9.0.2
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.0.2 → verified1.9.0.2
Group: core-security

Comment 12

9 years ago
for future reference, this is CVE-2008-4064
You need to log in before you can comment on or make changes to this bug.