Closed Bug 815489 Opened 7 years ago Closed 7 years ago

OOB write relating to mozilla::gfx::AlphaBoxBlur::Blur

Categories

(Core :: Graphics, defect, critical)

x86_64
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla20
Tracking Status
firefox17 --- unaffected
firefox18 --- unaffected
firefox19 + fixed
firefox20 --- fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected

People

(Reporter: inferno, Assigned: mats)

References

Details

(5 keywords, Whiteboard: [asan])

Attachments

(3 files)

Attached file Testcase
Reproduces on trunk, run testcase and wait for 15-20 sec for animation to run.
ASAN isn't able to unwind the stack inside the system library, so someone needs to use valgrind to get the full crash stack.

=================================================================
==32251== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fa5178f404f at pc 0x4c00c5 bp 0x7fff072c0540 sp 0x7fff072bfd08
WRITE of size 1 at 0x7fa5178f404f thread T0
    #0 0x4c00c4 in memcpy
    #1 0x7fa5503dec2f in ?? /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0
0x7fa5178f404f is located 15 bytes inside of 145414803-byte region [0x7fa5178f4040,0x7fa5203a1ad3)
freed by thread T0 here:
    #0 0x4c37d0 in __interceptor_free
    #1 0x7fa57026d405 in moz_free /usr/local/google/home/aarya/firefox/src/memory/mozalloc/mozalloc.cpp:48
    #2 0x7fa5652a6ffe in operator delete[](void*) /usr/local/google/home/aarya/firefox/src/../../dist/include/mozilla/mozalloc.h:236
    #3 0x7fa5652a6ffe in mozilla::gfx::AlphaBoxBlur::Blur() /usr/local/google/home/aarya/firefox/src/gfx/2d/Blur.cpp:523
    #4 0x7fa5611dfefc in gfxAlphaBoxBlur::Paint(gfxContext*, gfxPoint const&) /usr/local/google/home/aarya/firefox/src/gfx/thebes/gfxBlur.cpp:86
    #5 0x7fa5542e0453 in nsContextBoxBlur::DoPaint() /usr/local/google/home/aarya/firefox/src/layout/base/nsCSSRendering.cpp:4821
    #6 0x7fa5542db0e6 in nsCSSRendering::PaintBoxShadowOuter(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&) /usr/local/google/home/aarya/firefox/src/layout/base/nsCSSRendering.cpp:1345
    #7 0x7fa5544382e0 in nsDisplayBoxShadowOuter::Paint(nsDisplayListBuilder*, nsRenderingContext*) /usr/local/google/home/aarya/firefox/src/layout/base/nsDisplayList.cpp:2370
    #8 0x7fa554118506 in mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*) /usr/local/google/home/aarya/firefox/src/layout/base/FrameLayerBuilder.cpp:3274
    #9 0x7fa561591d3f in mozilla::layers::BasicThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.h:94
    #10 0x7fa5615913ee in mozilla::layers::BasicShadowableThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.cpp:402
    #11 0x7fa561589fab in mozilla::layers::BasicThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.cpp:189
    #12 0x7fa56158ea4a in mozilla::layers::BasicShadowableThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.cpp:303
    #13 0x7fa56158f433 in non-virtual thunk to mozilla::layers::BasicShadowableThebesLayer::PaintThebes(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.cpp:314
    #14 0x7fa5615187c5 in mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintContext&, gfxContext*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicLayerManager.cpp:827
    #15 0x7fa561515c05 in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicLayerManager.cpp:936
previously allocated by thread T0 here:
    #0 0x4c3890 in malloc
    #1 0x7fa57026d541 in moz_xmalloc /usr/local/google/home/aarya/firefox/src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7fa5652a0cb3 in operator new[](unsigned long) /usr/local/google/home/aarya/firefox/src/../../dist/include/mozilla/mozalloc.h:212
    #3 0x7fa5652a0cb3 in mozilla::gfx::AlphaBoxBlur::AlphaBoxBlur(mozilla::gfx::Rect const&, mozilla::gfx::IntSize const&, mozilla::gfx::IntSize const&, mozilla::gfx::Rect const*, mozilla::gfx::Rect const*) /usr/local/google/home/aarya/firefox/src/gfx/2d/Blur.cpp:388
    #4 0x7fa5611de818 in gfxAlphaBoxBlur::Init(gfxRect const&, nsIntSize const&, nsIntSize const&, gfxRect const*, gfxRect const*) /usr/local/google/home/aarya/firefox/src/gfx/thebes/gfxBlur.cpp:52
    #5 0x7fa5542dfd2e in nsContextBoxBlur::Init(nsRect const&, int, int, int, gfxContext*, nsRect const&, gfxRect const*, unsigned int) /usr/local/google/home/aarya/firefox/src/layout/base/nsCSSRendering.cpp:4794
    #6 0x7fa5542d9eed in nsCSSRendering::PaintBoxShadowOuter(nsPresContext*, nsRenderingContext&, nsIFrame*, nsRect const&, nsRect const&) /usr/local/google/home/aarya/firefox/src/layout/base/nsCSSRendering.cpp:1269
    #7 0x7fa5544382e0 in nsDisplayBoxShadowOuter::Paint(nsDisplayListBuilder*, nsRenderingContext*) /usr/local/google/home/aarya/firefox/src/layout/base/nsDisplayList.cpp:2370
    #8 0x7fa554118506 in mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*) /usr/local/google/home/aarya/firefox/src/layout/base/FrameLayerBuilder.cpp:3274
    #9 0x7fa561591d3f in mozilla::layers::BasicThebesLayer::PaintBuffer(gfxContext*, nsIntRegion const&, nsIntRegion const&, nsIntRegion const&, bool, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*) /usr/local/google/home/aarya/firefox/src/gfx/layers/basic/BasicThebesLayer.h:94
Shadow byte and word:
  0x1ff4a2f1e809: fa
  0x1ff4a2f1e808: fa fa fa fa fa fa fa fa
More shadow bytes:
  0x1ff4a2f1e7e8: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e7f0: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e7f8: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e800: fa fa fa fa fa fa fa fa
=>0x1ff4a2f1e808: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e810: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e818: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e820: fa fa fa fa fa fa fa fa
  0x1ff4a2f1e828: fa fa fa fa fa fa fa fa
Stats: 585M malloced (355M for red zones) by 346309 calls
Stats: 38M realloced by 22393 calls
Stats: 422M freed by 218373 calls
Stats: 421M really freed by 216578 calls
Stats: 703M (180045 full pages) mmaped in 440 calls
  mmaps   by size class: 7:147420; 8:40940; 9:15345; 10:6132; 11:6120; 12:2304; 13:1024; 14:704; 15:192; 16:576; 17:452; 18:30; 19:37; 20:22; 21:1; 23:1; 24:1; 26:1; 29:2;
  mallocs by size class: 7:229244; 8:57275; 9:27694; 10:10274; 11:11194; 12:3648; 13:2000; 14:2047; 15:325; 16:990; 17:1493; 18:51; 19:42; 20:26; 21:1; 23:1; 24:1; 26:1; 29:2;
  frees   by size class: 7:143922; 8:28027; 9:21238; 10:6961; 11:9083; 12:2733; 13:1795; 14:1867; 15:218; 16:947; 17:1476; 18:38; 19:40; 20:23; 21:1; 23:1; 24:1; 26:1; 29:1;
  rfrees  by size class: 7:142502; 8:27785; 9:21214; 10:6910; 11:9071; 12:2724; 13:1776; 14:1867; 15:218; 16:933; 17:1472; 18:38; 19:40; 20:23; 21:1; 23:1; 24:1; 26:1; 29:1;
Stats: malloc large: 2933 small slow: 4833
==32251== ABORTING
Severity: normal → critical
Component: General → Layout
Product: Firefox → Core
Whiteboard: [asan]
Attached file stack
The problem is that AlphaBoxBlur::Blur() swaps mData/tmpData in some cases,
deleting mData, which is used by the image surface.
Assignee: nobody → matspal
Component: Layout → Graphics
Attached patch fixSplinter Review
Always keep the mData pointer as is.  It requires a final memcpy()
in rare cases, but this is on the slow path and I guess the common
case is that both width/height > 0 which means 'a' == mData and no
memcpy() is required.

https://tbpl.mozilla.org/?tree=Try&rev=a825c93810d8
Attachment #685602 - Flags: review?(roc)
The image surface will use freed memory and possibly paint that to screen,
which I guess is sec-high.
Keywords: sec-high
Comment on attachment 685602 [details] [diff] [review]
fix

[Security approval request comment]
How easily can the security issue be deduced from the patch?
if you know C/C++ you can probably figure out the crash cause

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
no

Which older supported branches are affected by this flaw?
Aurora

If not all supported branches, which bug introduced the flaw?
bug 509052

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
same patch applies

How likely is this patch to cause regressions; how much testing does it need?
moderate
Attachment #685602 - Flags: sec-approval?
Comment on attachment 685602 [details] [diff] [review]
fix

sec-approval+. Let's get this fixed on Aurora and newer so we don't accidentally ship a sec-high.

Does this need any tests?
Attachment #685602 - Flags: sec-approval? → sec-approval+
(In reply to Al Billings [:abillings] from comment #6)
> sec-approval+. Let's get this fixed on Aurora and newer so we don't
> accidentally ship a sec-high.

Requesting tracking for 19 to make sure we don't forget it for that train.
+cc: Bas
https://hg.mozilla.org/mozilla-central/rev/e102654a3f0f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 685602 [details] [diff] [review]
fix

See comment 5, low risk.
Attachment #685602 - Flags: approval-mozilla-aurora?
Attachment #685602 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+
Group: core-security
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/54fd830abdba
Flags: in-testsuite? → in-testsuite+
Depends on: 871763
Depends on: 872108
You need to log in before you can comment on or make changes to this bug.