ASan reports a buffer overflow when the attached SVG file is opened. ================================================================= ==30601==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62b000301600 at pc 0x7fc508bce6b0 bp 0x7fffced6cab0 sp 0x7fffced6caa8 READ of size 16 at 0x62b000301600 thread T0 ==30601==WARNING: Trying to symbolize code, but external symbolizer is not initialized! #0 0x7fc508bce6af in mozilla::gfx::FilterProcessing::ApplyMorphologyHorizontal_SSE2(unsigned char*, int, unsigned char*, int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, int, mozilla::gfx::MorphologyOperator) /home/aki/src/mozilla-aurora/gfx/2d/FilterProcessingSSE2.cpp:44 #1 0x7fc508c21d78 in mozilla::gfx::ApplyMorphology(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::DataSourceSurface*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, int, int, mozilla::gfx::MorphologyOperator) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:1087 #2 0x7fc508c19aa0 in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:605 #3 0x7fc508c1cca5 in mozilla::gfx::FilterNodeSoftware::GetInputDataSourceSurface(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::FilterNodeSoftware::FormatHint, mozilla::gfx::ConvolveMatrixEdgeMode, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:691 #4 0x7fc508c2f059 in mozilla::gfx::FilterNodeCropSoftware::Render(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:2846 #5 0x7fc508c19aa0 in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:605 #6 0x7fc508c1cca5 in mozilla::gfx::FilterNodeSoftware::GetInputDataSourceSurface(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::FilterNodeSoftware::FormatHint, mozilla::gfx::ConvolveMatrixEdgeMode, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:691 #7 0x7fc508c2f612 in mozilla::gfx::FilterNodeUnpremultiplySoftware::Render(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:2903 #8 0x7fc508c19aa0 in mozilla::gfx::FilterNodeSoftware::GetOutput(mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:605 #9 0x7fc508c1cca5 in mozilla::gfx::FilterNodeSoftware::GetInputDataSourceSurface(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::FilterNodeSoftware::FormatHint, mozilla::gfx::ConvolveMatrixEdgeMode, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const*) /home/aki/src/mozilla-aurora/gfx/2d/FilterNodeSoftware.cpp:691 [...] Filing as a potential security issue due to bug type.

Probably the same as bug 944579.

Please undupe this if it isn't actually a dupe.

Not actually a dupe per bug 944579 comment 21, unduping.

This should do it. Here's a simple example with morphology radius 1: input image (4x1): abcd output image (2x1): ef(gh) The output image is only two pixels wide, but its storage has reserved four pixels because its stride has been aligned to 16 bytes (and each pixel is four bytes). Only pixels e and f will be used, g and h are only padding. Now we want to have the following: e = min{a, b, c} f = min{b, c, e} But our filter is overeager and also calculates g = min{c, d, ?} h = min{d, ?, ?} Here's where the out-of-bounds access happens. However, as long as this doesn't cause a crash, it's completely harmless because the resulting pixels only end up in the alignment padding of the target surface. So while this bug is nice to fix, it's not a security hazard, as far as I can tell. This patch replaces the out-of-bounds read by a zero-initialization of the relevant SIMD variable.

Comment on attachment 8366256 [details] [diff] [review] v1 This fails reftests, something must be wrong with the condition.

This makes more sense and passes tests.

Comment on attachment 8366614 [details] [diff] [review] v2 [Security approval request comment] This bug is not actually exploitable, as far as I can tell. We can read from a heap address that we didn't allocate into, but we never make use of the read value, it only gets processed as part of four-pixels-at-a-time SIMD processing for pixels that are only surface padding, and then ignored. Which older supported branches are affected by this flaw? Aurora If not all supported branches, which bug introduced the flaw? bug 924102 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The patch applies on Aurora without changes. How likely is this patch to cause regressions; how much testing does it need? Very unlikely. Not much further testing needed; I gave it lots of testing and we have morphology filter reftests in the tree.

By "Aurora," I assume you mean Firefox 28? We're just branching and building for next week's release so this may need to wait a few days.

Yes, I mean Firefox 28.

Turns out to be harmless, but since the discussion mentions bug 944579 which we haven't released yet we should leave this hidden a bit longer.

Go ahead and get this in. I'm clearing sec-approval as it doesn't need it as a sec-other.

Any reason to not backport this to Aurora/Beta?

Nope, no reason, it should apply as-is and is not risky. I'll request approval.

Comment on attachment 8366614 [details] [diff] [review] v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 924102 / bug 924103 User impact if declined: none, only affects automated code-checking tools, as far as we know Testing completed (on m-c, etc.): 5 days on mozilla-central Risk to taking this patch (and alternatives if risky): extremely low String or IDL/UUID changes made by this patch: none

https://hg.mozilla.org/releases/mozilla-aurora/rev/546fa9fbaa1a https://hg.mozilla.org/releases/mozilla-beta/rev/784092b96308

Matt, can you please determine if this needs testing before we release?

Confirmed crash on ASan build, 2014-01-12. Verified fixed on ASan builds of Firefox 28/29/30, 2014-02-19.