Closed
Bug 787722
(CVE-2012-4188)
Opened 12 years ago
Closed 12 years ago
Heap-buffer-overflow in Convolve3x3
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
People
(Reporter: attekett, Assigned: jwatt)
Details
(4 keywords, Whiteboard: [asan][advisory-tracking+])
Attachments
(2 files)
398 bytes,
image/svg+xml
|
Details | |
1.51 KB,
patch
|
roc
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
lsblakk
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
Reproducible with ASAN-build from https://people.mozilla.com/~choller/firefox/asan/20120831-mozilla-central-linux64-debug-fcc533f691e9+asan.html
ASAN-report:
==1095== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7fae06810023 at pc 0x7fae417985cc bp 0x7fff9dfb8c80 sp 0x7fff9dfb8c78
READ of size 1 at 0x7fae06810023 thread T0
#0 0x7fae417985cc in Convolve3x3 /home/attekett/firefox/src/content/svg/content/src/nsSVGFilters.cpp:4926
0x7fae06810023 is located 3 bytes to the right of 4000-byte region [0x7fae0680f080,0x7fae06810020)
allocated by thread T0 here:
#0 0x422e7c in posix_memalign ??:0
#1 0x7fae428f6565 in TryAllocAlignedBytes /home/attekett/firefox/src/gfx/thebes/gfxImageSurface.cpp:89
#2 0x7fae41764886 in nsSVGFE::SetupScalingFilter(nsSVGFilterInstance*, nsSVGFE::Image const*, nsSVGFE::Image const*, nsIntRect const&, nsSVGNumberPair*) /home/attekett/firefox/src/content/svg/content/src/nsSVGFilters.cpp:153
#3 0x7fae41782077 in nsSVGFELightingElement::Filter(nsSVGFilterInstance*, nsTArray<nsSVGFE::Image const*, nsTArrayDefaultAllocator> const&, nsSVGFE::Image const*, nsIntRect const&) /home/attekett/firefox/src/content/svg/content/src/nsSVGFilters.cpp:5009
#4 0x7fae4169d8e5 in nsSVGFilterInstance::Render(gfxASurface**) /home/attekett/firefox/src/layout/svg/base/src/nsSVGFilterInstance.cpp:583
#5 0x7fae416944d4 in nsSVGFilterFrame::PaintFilteredFrame(nsRenderingContext*, nsIFrame*, nsSVGFilterPaintCallback*, nsRect const*) /home/attekett/firefox/src/layout/svg/base/src/nsSVGFilterFrame.cpp:444
.
.
.
Reporter | ||
Updated•12 years ago
|
Whiteboard: [asan]
![]() |
Assignee | |
Comment 1•12 years ago
|
||
I'm looking into this, but it's going to take some time to grok this part of the filters code.
OS: Linux → All
Hardware: x86_64 → All
![]() |
Assignee | |
Comment 2•12 years ago
|
||
I don't understand the relevant parts of the SVG filter code in enough detail to feel satisfied that it's doing the right thing. That said, the issue seems to be that in nsSVGFELightingElement::Filter we end up looping over sourceData and targetData for each pixel in dataRect, but dataRect is 24x11 px while the images that own sourceData and targetData are only 25x10 px. In other words we loop over 264 pixels when only 250 have been allocated for the images. The root cause of the problem seems to be that SetupScalingFilter (called right at the top of nsSVGFELightingElement::Filter) has a rounding issue when scaling aDataRect which causes it to return the 24x11 px result.mDataRect.
Assignee: nobody → jwatt
![]() |
Assignee | |
Comment 3•12 years ago
|
||
Attachment #660408 -
Flags: review?(roc)
Attachment #660408 -
Flags: review?(roc) → review+
![]() |
Assignee | |
Comment 4•12 years ago
|
||
status-firefox15:
--- → affected
status-firefox16:
--- → affected
status-firefox17:
--- → affected
status-firefox18:
--- → fixed
tracking-firefox16:
--- → ?
tracking-firefox17:
--- → ?
Comment 5•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Comment 7•12 years ago
|
||
Assuming ESR-10 is affected because SVG Filters were around then.
status-firefox-esr10:
--- → affected
tracking-firefox-esr10:
--- → ?
Keywords: csec-bounds,
sec-critical
![]() |
Assignee | |
Comment 8•12 years ago
|
||
Comment on attachment 660408 [details] [diff] [review]
patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): issue has been in the code since forever
User impact if declined: may or may not be exploitable in different ways in different versions and on different platforms
Testing completed (on m-c, etc.): baked on m-c for a couple of days
Risk to taking this patch (and alternatives if risky): very low risk
String or UUID changes made by this patch: none
Attachment #660408 -
Flags: approval-mozilla-esr10?
Attachment #660408 -
Flags: approval-mozilla-beta?
Attachment #660408 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #660408 -
Flags: approval-mozilla-esr10?
Attachment #660408 -
Flags: approval-mozilla-esr10+
Attachment #660408 -
Flags: approval-mozilla-beta?
Attachment #660408 -
Flags: approval-mozilla-beta+
Attachment #660408 -
Flags: approval-mozilla-aurora?
Attachment #660408 -
Flags: approval-mozilla-aurora+
Updated•12 years ago
|
![]() |
Assignee | |
Comment 10•12 years ago
|
||
Updated•12 years ago
|
Whiteboard: [asan] → [asan][advisory-tracking+]
Updated•12 years ago
|
Alias: CVE-2012-4188
Updated•12 years ago
|
Group: core-security
Flags: sec-bounty+
Updated•9 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•