1465686 - (CVE-2018-12371) Heap overflow write in SkEdgeBuilder::buildPoly

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[abbaolk]

Attachment: poc16G.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180517113820

Steps to reproduce:

This is a variant of vulnerability hrer:https://bugzilla.mozilla.org/show_bug.cgi?id=1418447

System: Ubuntu 16.04.4 LTS (64bit)
Firefox version: 60.0.1 (64-bit)
RAM size:16G
SWAP size:5.9G


Root cause:
https://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkEdgeBuilder.cpp#255

   SkPath::Iter    iter(path, true);
    SkPoint         pts[4];
    SkPath::Verb    verb;

    size_t maxEdgeCount = path.countPoints();
    if (iclip) {
        // clipping can turn 1 line into (up to) kMaxClippedLineSegments, since
        // we turn portions that are clipped out on the left/right into vertical
        // segments.
        maxEdgeCount *= SkLineClipper::kMaxClippedLineSegments;
    }

    size_t edgeSize;
    char* edge;
    switch (fEdgeType) {
        case kEdge:
            edgeSize = sizeof(SkEdge);
            edge = (char*)fAlloc.makeArrayDefault<SkEdge>(maxEdgeCount);
            break;
        case kAnalyticEdge:
            edgeSize = sizeof(SkAnalyticEdge);
            edge = (char*)fAlloc.makeArrayDefault<SkAnalyticEdge>(maxEdgeCount);
            break;
        case kBezier:
            edgeSize = sizeof(SkLine);
            edge = (char*)fAlloc.makeArrayDefault<SkLine>(maxEdgeCount);
            break;
    }
....................................
T* makeArrayDefault(size_t count) {
        uint32_t safeCount = SkTo<uint32_t>(count); 

"SkTo<uint32_t>(count)" just convert "maxEdgeCount" to a uint32_t type variable.

So if "maxEdgeCount" is larger than 0x100000000(4G),.......


Steps to reproduce:
To trigger the vulnerability, we need about 16G memory.

Open attached html file(with attached gdb) and clicker "trigger",  ff will crash in less than 10 minutes.(My computer is pretty old, with better CPU and larger memory, ff should crash more quickly)

(gdb) sharedlibrary libxul
Reading symbols from /usr/lib/firefox/libxul.so...Reading symbols from /usr/lib/debug/.build-id/8d/03b3c494a48356d5df2ea89f4fcbfa565904c4.debug...done.
done.
(gdb) bt 8
#0  SkEdgeBuilder::CombineVertical (this=0x7ffce76b9750, edge=0x7f08dae5f018, last=0x1680000)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:22
#1  0x00007f08e774ffe8 in SkEdgeBuilder::addPolyLine (shiftUp=23592960, edgePtr=@0x7ffce76b91c8: 0x7f08dae5f030, 
    edgeSize=40, edge=@0x7ffce76b91c0: 0x7f08dae5f018 "8\\\334\332\b\177", pts=0x7ffce76b9240, this=0x7ffce76b9750)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.h:110
#2  SkEdgeBuilder::buildPoly (this=this@entry=0x7ffce76b9750, path=..., iclip=iclip@entry=0x7ffce76b96b0, 
    shiftUp=shiftUp@entry=2, canCullToTheRight=canCullToTheRight@entry=true)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:306
#3  0x00007f08e7755b57 in SkEdgeBuilder::build (this=this@entry=0x7ffce76b9750, path=..., iclip=0x7ffce76b96b0, 
    shiftUp=2, canCullToTheRight=<optimized out>, edgeType=SkEdgeBuilder::kEdge)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:354
#4  0x00007f08e7755d7b in SkEdgeBuilder::build_edges (this=this@entry=0x7ffce76b9750, path=..., 
    shiftedClip=shiftedClip@entry=0x7ffce76b96b0, shiftEdgesUp=shiftEdgesUp@entry=2, 
    pathContainedInClip=pathContainedInClip@entry=false, edgeType=edgeType@entry=SkEdgeBuilder::kEdge)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:457
#5  0x00007f08e77ab814 in sk_fill_path (path=..., clipRect=..., blitter=blitter@entry=0x7ffce76b9a10, start_y=-150, 
    stop_y=150, shiftEdgesUp=shiftEdgesUp@entry=2, pathContainedInClip=false)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_Path.cpp:406
#6  0x00007f08e74f46d6 in SkScan::SAAFillPath (path=..., blitter=blitter@entry=0x7ffce76ba618, ir=..., 
    clipBounds=..., forceRLE=forceRLE@entry=false)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_AntiPath.cpp:629
#7  0x00007f08e74f4d12 in SkScan::AntiFillPath (path=..., origClip=..., blitter=<optimized out>, 
    blitter@entry=0x7ffce76ba618, forceRLE=forceRLE@entry=false, daaRecord=daaRecord@entry=0x0)
    at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_AntiPath.cpp:747
(More stack frames follow...)

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Possibly relevant https://skia.googlesource.com/skia.git/+/632de605a37588db8a0b2eab901540c1b6300b0a%5E%21/

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: validate SkArenaAlloc sizes

SkTo<uint32_t>() only does debug asserts. This replaces it with a variant that does release asserts to catch the overflows.

Comment 3

[Andrew Whalley]

I've filed crbug.com/848521 to track this on the chrome side, and included the original report and comments from Alex and Lee.  Thanks!

Comment 4

[Ryan VanderMeulen [:RyanVM]]

This was pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9019db1eaddb

Comment 5

[Lee Salzman [:lsalzman]]

Comment on attachment 8982330 [details] [diff] [review]
validate SkArenaAlloc sizes

Approval Request Comment
[Feature/Bug causing the regression]: bug 1444506
[User impact if declined]: Crashes exploitable via JS.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: Just trades a silent overflow/crash for an explicit release assert.
[String changes made/needed]: none

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9019db1eaddb

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8982330 [details] [diff] [review]
validate SkArenaAlloc sizes

Fixes a Skia sec issue. Approved for 61.0b11 and ESR 60.1.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/58b05426aa76
https://hg.mozilla.org/releases/mozilla-esr60/rev/721c4672b93d

Comment 9

[Daniel Veditz [:dveditz]]

Andrew: has Google assigned a CVE to this flaw?

Comment 10

[Andrew Whalley]

No CVE assigned yet, probably best to assign one yourself at this point.  Cheers!

Comment 11

[Al Billings [:abillings - ex-MoCo]]

CVE-2018-12371 assigned.