1990970 - Heap-buffer-overflow in [@ SkPixmap::erase] via CanvasTranslator

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Christian Holler (:decoder)]

Saw this in experimental IPC fuzzing, targeting canvas translation:

==1936==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fff83c78c8f at pc 0x7fffd3c45f06 bp 0x7fff5bbe9c00 sp 0x7fff5bbe9bf8                    
WRITE of size 16 at 0x7fff83c78c8f thread T57                                                                                                                                                                                                                                                                            
    #0 0x7fffd3c45f05 in skvx::Vec<4, unsigned int>::store(void*) const gfx/skia/skia/src/base/SkVx.h:153:9               
    #1 0x7fffd3c45f05 in void SK_OPTS_NS::memsetT<unsigned int>(unsigned int*, unsigned int, int) gfx/skia/skia/src/opts/SkMemset_opts.h:28:23                                                                                      
    #2 0x7fffd3c45f05 in SkPixmap::erase(SkRGBA4f<(SkAlphaType)3> const&, SkIRect const*) const::$_2::operator()(void*, unsigned long, int) const gfx/skia/skia/src/core/SkPixmap.cpp:807:17        
    #3 0x7fffd3c45f05 in SkPixmap::erase(SkRGBA4f<(SkAlphaType)3> const&, SkIRect const*) const::$_2::__invoke(void*, unsigned long, int) gfx/skia/skia/src/core/SkPixmap.cpp:805:13
    #4 0x7fffd3c455c4 in SkPixmap::erase(SkRGBA4f<(SkAlphaType)3> const&, SkIRect const*) const gfx/skia/skia/src/core/SkPixmap.cpp:819:13
    #5 0x7fffd3c44aff in SkPixmap::erase(unsigned int, SkIRect const&) const gfx/skia/skia/src/core/SkPixmap.cpp:759:18                                                                                                                                                                       
    #6 0x7fffd6ef9434 in SkPixmap::erase(unsigned int) const gfx/skia/skia/include/core/SkPixmap.h:712:52                                                                                                                                                                                     
    #7 0x7fffd6ef9434 in mozilla::gfx::SharedContextWebgl::ReadInto(unsigned char*, int, mozilla::gfx::SurfaceFormat, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::TextureHandle*) dom/canvas/DrawTargetWebgl.cpp:1200:10                                     
    #8 0x7fffd6ef98d3 in mozilla::gfx::SharedContextWebgl::ReadSnapshot(mozilla::gfx::TextureHandle*) dom/canvas/DrawTargetWebgl.cpp:1238:30
    #9 0x7fffd6ef9e2e in mozilla::gfx::DrawTargetWebgl::ReadSnapshot() dom/canvas/DrawTargetWebgl.cpp:1261:26                                                                                                                                                                                 
    #10 0x7fffd6f56ae7 in mozilla::gfx::SourceSurfaceWebgl::EnsureData() dom/canvas/SourceSurfaceWebgl.cpp:44:18                                                                                                                                                                              
    #11 0x7fffd6f56ae7 in mozilla::gfx::SourceSurfaceWebgl::GetData() dom/canvas/SourceSurfaceWebgl.cpp:50:8                                                                                                                                                                                  
    #12 0x7fffd1c98055 in mozilla::gfx::GetSkImageForSurface(mozilla::gfx::SourceSurface*, mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex&> >*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, mozilla::gfx::BaseMatrix<float> const*) gfx/2d/DrawTargetSkia.
cpp:277:30                                                                                                                                                                                                                                                                                                               
    #13 0x7fffd1ca037b in mozilla::gfx::SetPaintPattern(SkPaint&, mozilla::gfx::Pattern const&, mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex&> >&, float, SkMatrix const*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*) gfx/2d/DrawTargetSkia.cpp:595:11 
    #14 0x7fffd1c9ca04 in mozilla::gfx::AutoPaintSetup::AutoPaintSetup(SkCanvas*, mozilla::gfx::DrawOptions const&, mozilla::gfx::Pattern const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, SkMatrix const*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*) /srv/repos/firef
ox.fuzzing/gfx/2d/DrawTargetSkia.cpp:662:5                                                                                                                                                                                                                                                                               
    #15 0x7fffd1c9ca04 in mozilla::gfx::DrawTargetSkia::Fill(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&) gfx/2d/DrawTargetSkia.cpp:1020:18             
    #16 0x7fffd6f1a325 in mozilla::gfx::SharedContextWebgl::DrawPathAccel(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::StrokeOptions const*, bool, mozilla::gfx::ShadowOptions const*, bool, mozilla::gfx::BaseMatrix<float> const*) /srv/repos/firefox.fuzzi
ng/dom/canvas/DrawTargetWebgl.cpp:4787:17                                                                                                                                                                                                                                                                                
    #17 0x7fffd6f28004 in mozilla::gfx::DrawTargetWebgl::DrawSurfaceWithShadow(mozilla::gfx::SourceSurface*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::ShadowOptions const&, mozilla::gfx::CompositionOp) dom/canvas/DrawTargetWebgl.cpp:5021:25
    #18 0x7fffd1bb340b in mozilla::gfx::RecordedDrawSurfaceWithShadow::PlayEvent(mozilla::gfx::Translator*) const gfx/2d/RecordedEventImpl.h:3472:7
    #19 0x7fffd238195c in mozilla::layers::CanvasTranslator::TranslateRecording()::$_1::operator()(mozilla::gfx::RecordedEvent*) const gfx/layers/ipc/CanvasTranslator.cpp:913:33
    #20 0x7fffd238195c in bool std::__invoke_impl<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>(std::__invoke_other, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu
/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:60:14
    #21 0x7fffd238195c in std::enable_if<is_invocable_r_v<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>, bool>::type std::__invoke_r<bool, mozilla::layers::CanvasTranslator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*>(mozilla::layers::CanvasTrans
lator::TranslateRecording()::$_1&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/invoke.h:113:9
    #22 0x7fffd238195c in std::_Function_handler<bool (mozilla::gfx::RecordedEvent*), mozilla::layers::CanvasTranslator::TranslateRecording()::$_1>::_M_invoke(std::_Any_data const&, mozilla::gfx::RecordedEvent*&&) sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../includ
e/c++/10/bits/std_function.h:291:9
    #23 0x7fffd1bf8eec in std::function<bool (mozilla::gfx::RecordedEvent*)>::operator()(mozilla::gfx::RecordedEvent*) const sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/std_function.h:622:14
    #24 0x7fffd1bbab09 in bool mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::MemReader>(mozilla::gfx::MemReader&, mozilla::gfx::RecordedEvent::EventType, std::function<bool (mozilla::gfx::RecordedEvent*)> const&) gfx/2d/RecordedEventImpl.h:4819:5
    #25 0x7fffd22f7214 in mozilla::layers::CanvasTranslator::TranslateRecording() gfx/layers/ipc/CanvasTranslator.cpp:893:20
[...]
0x7fff83c78c8f is located 0 bytes after 607375-byte region [0x7fff83be4800,0x7fff83c78c8f)
allocated by thread T57 here:
    #0 0x55555570863f in __interceptor_malloc _asan_rtl_:3
    #1 0x7fffd1d5e185 in mozilla::gfx::AlignedArray<unsigned char, 16>::Realloc(unsigned long, bool) gfx/2d/Tools.h:131:40
    #2 0x7fffd1d5e185 in mozilla::gfx::SourceSurfaceAlignedRawData::Init(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, bool, unsigned char, int) gfx/2d/SourceSurfaceRawData.cpp:90:12
    #3 0x7fffd1c4a74a in mozilla::gfx::Factory::CreateDataSourceSurface(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SurfaceFormat, bool) gfx/2d/Factory.cpp:1080:16
    #4 0x7fffd6ef97f5 in mozilla::gfx::SharedContextWebgl::ReadSnapshot(mozilla::gfx::TextureHandle*) dom/canvas/DrawTargetWebgl.cpp:1233:7
    #5 0x7fffd6ef9e2e in mozilla::gfx::DrawTargetWebgl::ReadSnapshot() dom/canvas/DrawTargetWebgl.cpp:1261:26
    #6 0x7fffd6f56ae7 in mozilla::gfx::SourceSurfaceWebgl::EnsureData() dom/canvas/SourceSurfaceWebgl.cpp:44:18
    #7 0x7fffd6f56ae7 in mozilla::gfx::SourceSurfaceWebgl::GetData() dom/canvas/SourceSurfaceWebgl.cpp:50:8
    #8 0x7fffd1c98055 in mozilla::gfx::GetSkImageForSurface(mozilla::gfx::SourceSurface*, mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex&> >*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, mozilla::gfx::BaseMatrix<float> const*) gfx/2d/DrawTargetSkia.c
pp:277:30
    #9 0x7fffd1ca037b in mozilla::gfx::SetPaintPattern(SkPaint&, mozilla::gfx::Pattern const&, mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex&> >&, float, SkMatrix const*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*) gfx/2d/DrawTargetSkia.cpp:595:11
    #10 0x7fffd1c9ca04 in mozilla::gfx::AutoPaintSetup::AutoPaintSetup(SkCanvas*, mozilla::gfx::DrawOptions const&, mozilla::gfx::Pattern const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*, SkMatrix const*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const*) /srv/repos/firef
ox.fuzzing/gfx/2d/DrawTargetSkia.cpp:662:5
    #11 0x7fffd1c9ca04 in mozilla::gfx::DrawTargetSkia::Fill(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&) gfx/2d/DrawTargetSkia.cpp:1020:18
    #12 0x7fffd6f1a325 in mozilla::gfx::SharedContextWebgl::DrawPathAccel(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::StrokeOptions const*, bool, mozilla::gfx::ShadowOptions const*, bool, mozilla::gfx::BaseMatrix<float> const*) /srv/repos/firefox.fuzzi
ng/dom/canvas/DrawTargetWebgl.cpp:4787:17
    #13 0x7fffd6f28004 in mozilla::gfx::DrawTargetWebgl::DrawSurfaceWithShadow(mozilla::gfx::SourceSurface*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::ShadowOptions const&, mozilla::gfx::CompositionOp) dom/canvas/DrawTargetWebgl.cpp:5021:25
    #14 0x7fffd1bb340b in mozilla::gfx::RecordedDrawSurfaceWithShadow::PlayEvent(mozilla::gfx::Translator*) const gfx/2d/RecordedEventImpl.h:3472:7
    #15 0x7fffd238195c in mozilla::layers::CanvasTranslator::TranslateRecording()::$_1::operator()(mozilla::gfx::RecordedEvent*) const gfx/layers/ipc/CanvasTranslator.cpp:913:33
[...]
Shadow bytes around the buggy address:
  0x7fff83c78c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x7fff83c78c80: 00[07]fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x7fff83c78d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa

Posting now so we have it on file, line numbers in some places can be off due to local fuzzblocker fixes.

Comment 1

[Lee Salzman [:lsalzman]]

This needs a testcase.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: (secure)

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: (secure)

Comment 4

[Lee Salzman [:lsalzman]]

Comment on attachment 9516697 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unknown, but maybe not that difficult if you are savvy.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Seems fine in local testing.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version: 145
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency:
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:
• Is Android affected?: Yes

Comment 5

[Lee Salzman [:lsalzman]]

After looking at the patch/testcase that Christian supplied me, this seems potentially a pretty insidious bug in terms of the potential failures this could have caused just about anywhere else in the code.

GfxColorTypeToSkiaColorType can return a Skia SkColorType whose bytes-per-pixel is larger than the Moz2D SurfaceFormat. This can then cause Skia to easily overrun via read or write the supplied buffer by multiples of the actual data's size.

This is insidious precisely because I can't predict all the potential entry-points this could impact, but it can easily be avoided by just fixing it at the source of the problem. In this case, I make GfxColorTypeToSkiaColorType return a SkColorType value whose size is always less than other unmappable SurfaceFormats, i.e. kAlpha_8_SkColorType, whose size is always 1.

The part of the fix that gets tricky is also an issue, that if we allocate pixel data inside Skia based on that SkColorType, we need to make sure we don't call into Moz2D rasterization routines that assume the corresponding SurfaceFormat has a larger bytes-per-pixel or stride. Therefor, I insert checks to assure these values always agree for SourceSurfaceSkia and DrawTargetSkia on construction, which should prevent us from getting caught by this.

Comment 6

[Tom Ritter [:tjr]]

Comment on attachment 9516697 [details]
(secure)

Approved to land and request uplift

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/43543a925317
https://hg.mozilla.org/integration/autoland/rev/6d6256588ddf
r=gfx-reviewers,ahale

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6d6256588ddf

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9516697 [details]
(secure)

Approved for 144.0b8.

Comment 10

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/a7db788fd519
https://hg.mozilla.org/releases/mozilla-beta/rev/c3e0263f0f37

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9516697 [details]
(secure)

Approved for 140.4esr.

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/cd871088bf73

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/06261e44ca58