heap-buffer-overflow (read) at mozilla::gfx::ColorComponentAtPoint

VERIFIED FIXED in Firefox 28

Status

()

defect
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: aki.helin, Assigned: mstange)

Tracking

(5 keywords)

Trunk
mozilla30
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty -

Firefox Tracking Flags

(firefox27 unaffected, firefox28 verified, firefox29 verified, firefox30+ verified, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 unaffected, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(3 attachments)

ASan spots a heap buffer overflow (read) when the attached SVG is opened in a recent Firefox build.

==15678==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f8775a17e91 at pc 0x7f87978c03d4
 bp 0x7fff6dae1ef0 sp 0x7fff6dae1ee8
READ of size 1 at 0x7f8775a17e91 thread T0
==15678==WARNING: Trying to symbolize code, but external symbolizer is not initialized!
    #0 0x7f87978c03d3 in mozilla::gfx::ColorComponentAtPoint(unsigned char const*, int, int, int, uns
igned long, long) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:2044
    #1 0x7f87978c1ed7 in mozilla::TemporaryRef<mozilla::gfx::DataSourceSurface> mozilla::gfx::FilterN
odeLightingSoftware<mozilla::gfx::(anonymous namespace)::DistantLightSoftware, mozilla::gfx::(anonymo
us namespace)::DiffuseLightingSoftware>::DoRender<float>(mozilla::gfx::IntRectTyped<mozilla::gfx::Unk
nownUnits> const&, float, float) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:3241
    #2 0x7f87978a6af0 in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozil
la::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:605
    #3 0x7f87978a9cf5 in mozilla::gfx::FilterNodeSoftware::GetInputDataSourceSurface(unsigned int, mo
zilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::FilterNodeSoftware::Format
Hint, mozilla::gfx::ConvolveMatrixEdgeMode, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> co
nst*) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:691
    #4 0x7f87978bc0a9 in mozilla::gfx::FilterNodeCropSoftware::Render(mozilla::gfx::IntRectTyped<mozi
lla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:2846
    #5 0x7f87978a6af0 in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozil
la::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:605
    #6 0x7f87978a640a in mozilla::gfx::FilterNodeSoftware::Draw(mozilla::gfx::DrawTarget*, mozilla::g
fx::RectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits
> const&, mozilla::gfx::DrawOptions const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cp
p:573
    #7 0x7f87937b0267 in mozilla::gfx::FilterSupport::RenderFilterDescription(mozilla::gfx::DrawTarge
t*, mozilla::gfx::FilterDescription const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const
&, mozilla::gfx::SourceSurface*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozil
la::gfx::SourceSurface*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx:
:SourceSurface*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, nsTArray<mozilla::Ref
Ptr<mozilla::gfx::SourceSurface> >&) /home/aki/src/mozilla-aurora/gfx/src/FilterSupport.cpp:1116
[...]

I started running Firefox tests recently and have now filed a few filter-related SVG bugs with different ASan traces. Sorry if these turn out to have the same root cause.

Filing as a security issue based on bug type.
Could you look at this, mstange?  Thanks.
Flags: needinfo?(mstange)
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
Blocks: 924102
Keywords: regression
Probably also fixed by bug 944579. Aki, can you confirm?
Flags: needinfo?(aki.helin)
No, this one still happens in a fresh Aurora build for me.
Flags: needinfo?(aki.helin)
Posted patch access checksSplinter Review
Not sure whether we want this, since ASAN builds do this already. But this allows the checks to be run in regular debug builds, too.
Attachment #8365996 - Flags: review?(bas)
Posted patch fix bugSplinter Review
The reason for the out-of-bounds access is the bilinear sampling in ColorComponentAtPoint when called with fractional positions.
Attachment #8365999 - Flags: review?(bas)
Attachment #8365999 - Flags: review?(bas) → review+
Comment on attachment 8365996 [details] [diff] [review]
access checks

Review of attachment 8365996 [details] [diff] [review]:
-----------------------------------------------------------------

How bad is the perf impact of this? It seems to me like it could make certain filter constructions almost 'unusable' in a debug build? Then again, certain filters would probably already be unfeasible in non-opt builds.
Attachment #8365996 - Flags: review?(bas) → review+
(In reply to Bas Schouten (:bas.schouten) from comment #6)
> How bad is the perf impact of this? It seems to me like it could make
> certain filter constructions almost 'unusable' in a debug build? Then again,
> certain filters would probably already be unfeasible in non-opt builds.

It doesn't seem too bad. Convolve matrix + lighting filters are already much slower in debug builds, I'd say we're between almost usable and fairly unusable for them (in debug builds). With the access control checks we're in the same range, they don't cause us to fall into the completely unusable range.
Comment on attachment 8365996 [details] [diff] [review]
access checks

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily. 

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
They make it very clear that we're reading from a location that we shouldn't be reading from, but they don't make it clear how to trigger or exploit it.

Which older supported branches are affected by this flaw?
It started with Firefox 28, which has just moved to Beta.

If not all supported branches, which bug introduced the flaw?
Bug 924102 / bug 924103.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patches apply cleanly on all affected branches. Risk is very low.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely, does not need much testing.
Attachment #8365996 - Flags: sec-approval?
Attachment #8365999 - Flags: sec-approval?
We need a security rating on this in order to approve it. How exploitable is this?
Hard for me to say. The bug causes us to read from memory ranges slightly outside the parts that we allocated for a temporary image surface (i.e. max 1 pixel outside, in all directions, I think). The read values are then subject to a convolution operation which mixes them with values from inside the surface, and with the right setup the resulting values can be read by webpage JavaScript.
arbitrary-sized (or large) reads usually rate sec-high, but a byte inflation says this was a tiny information leak.
Since this is a sec-moderate, it can just go in. It doesn't need sec-approval.
Attachment #8365996 - Flags: sec-approval?
Attachment #8365999 - Flags: sec-approval?
Too bad an exploit using this would be limited to just the immediate effect of getting one byte past bounds. Otherwise one could e.g. fill the js heap with something of interest and start reading one byte at a time from many different positions ;)
https://hg.mozilla.org/mozilla-central/rev/9e44f7c6661a
https://hg.mozilla.org/mozilla-central/rev/0fce0bb33afb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Markus, can we get branch uplift requests please? :)
Flags: needinfo?(mstange)
Comment on attachment 8365996 [details] [diff] [review]
access checks

Sure!
Attachment #8365996 - Flags: approval-mozilla-beta?
Attachment #8365996 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mstange)
Comment on attachment 8365999 [details] [diff] [review]
fix bug

I'm requesting approval for both attachment 8365996 [details] [diff] [review] and attachment 8365999 [details] [diff] [review].

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 924102 / 924103
User impact if declined: small potential data leak
Testing completed (on m-c, etc.): 4 days on mozilla-central
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none
Attachment #8365999 - Flags: approval-mozilla-beta?
Attachment #8365999 - Flags: approval-mozilla-aurora?
Attachment #8365996 - Flags: approval-mozilla-beta?
Attachment #8365996 - Flags: approval-mozilla-beta+
Attachment #8365996 - Flags: approval-mozilla-aurora?
Attachment #8365996 - Flags: approval-mozilla-aurora+
Attachment #8365999 - Flags: approval-mozilla-beta?
Attachment #8365999 - Flags: approval-mozilla-beta+
Attachment #8365999 - Flags: approval-mozilla-aurora?
Attachment #8365999 - Flags: approval-mozilla-aurora+
Matt, can you please ensure this gets verified with tomorrow's Beta?
Flags: needinfo?(mwobensmith)
Keywords: verifyme
Verified crash in ASan build of FF28, 2014-01-15.
Confirmed fixed in ASan builds of FF28, 29, 30, 2014-02-19.
Status: RESOLVED → VERIFIED
Flags: needinfo?(mwobensmith)
Flags: sec-bounty?
Without additional demonstration of potential exploitability we feel this bug does not qualify for the security bug bounty. If you disagree and have further demonstrations of a useful info leak please mail our security address and we'll renominate this for a bounty.
Flags: sec-bounty? → sec-bounty-
Group: core-security
You need to log in before you can comment on or make changes to this bug.