x/image/vector: panic and weird drawings on RGBA and Alpha image dsts with Uniform image src

[haashemi]

Go version

go version go1.22.4 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Byfron\AppData\Local\go-build
set GOENV=C:\Users\Byfron\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Byfron\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Byfron\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\Byfron\scoop\apps\go\current
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Users\Byfron\scoop\apps\go\current\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.4
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Byfron\Documents\Code\exp-vector-overflow\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Byfron\AppData\Local\Temp\go-build1421511663=/tmp/go-build -gno-record-gcc-switches

What did you do?

Basically, drawing a vector (any) on an image.RGBA or image.Alpha as dst and a image.Uniform as src with these cases:

1. Draw a vector at top-left (a few pixels lower/negative).
2. Draw a vector vertically centered and horizontally right (more than dst's Max.X) or left (less than dst's Min.X)
3. Draw a vector at bottom-right (a few pixels higher)

A complete code with all test cases: https://github.com/haashemi/exp-vector-overflow/blob/main/main.go

A minimal code:

package main

import (
    "image"
    "image/draw"
    "image/png"
    "os"

    "golang.org/x/image/vector"
)

func main() {
    img := image.NewRGBA(image.Rect(0, 0, 50, 50))
    vec := vector.NewRasterizer(25, 25)

    // create a rectangle
    vec.LineTo(25, 0)  // top right
    vec.LineTo(25, 25) // bottom right
    vec.LineTo(0, 25)  // bottom left
    vec.LineTo(0, 0)   // top left

    // fill the img's background for visibility
    draw.Draw(img, img.Rect, image.White, image.Point{}, draw.Over)

    // Test cases:
    // Replace (0,0) to one of these:
    // 1. (-10, -10) Panics
    // 2. (35, 35) Panics
    // 3. (-10, 15) Draws those -10x pixels on the right and a pixel higher
    // 4. (35, 15) Draws those +10x pixels on the left and a pixel lower
    addPoint := image.Pt(0, 0)

    // Draw an image.Uniform on an image.RGBA
    vec.Draw(img, vec.Bounds().Add(addPoint), image.Black, image.Point{})

    // Save the image. I avoided the error handling for now.
    f, _ := os.OpenFile("go-issue/test-case.png", os.O_CREATE|os.O_WRONLY, os.ModePerm)
    png.Encode(f, img)
}

What did you see happen?

Based on the minimal code example's test cases, these happened:

1. Drawing a 25x25 vector on 50x50 image.RGBA at (-10, -10). panics.

panic: runtime error: slice bounds out of range [-2040:]

goroutine 1 [running]:
golang.org/x/image/vector.(*Rasterizer).rasterizeDstRGBASrcUniformOpOver(0xc000107ee0, 0xc000072080, {{0x19?, 0x0?}, {0xffffffff00107e28?, 0x3200?}}, 0x0, 0x0, 0x0, 0xffff)
        C:/Users/Byfron/go/pkg/mod/golang.org/x/image@v0.18.0/vector/vector.go:395 +0x2e5
golang.org/x/image/vector.(*Rasterizer).Draw(0xc000107ee0, {0x450de0, 0xc000072080}, {{0x0?, 0x450de0?}, {0xc000072080?, 0x0?}}, {0x450b10, 0x4dd7a0?}, {0x0, ...})
        C:/Users/Byfron/go/pkg/mod/golang.org/x/image@v0.18.0/vector/vector.go:287 +0x19d
main.main()
        C:/Users/Byfron/Documents/Code/exp-vector-overflow/go-issue/main.go:34 +0x159
exit status 2

2. Drawing a 25x25 vector on 50x50 image.RGBA at (35, 35). panics. (different stacktrace)

panic: runtime error: index out of range [2860] with length 2860

goroutine 1 [running]:
golang.org/x/image/vector.(*Rasterizer).rasterizeDstRGBASrcUniformOpOver(0xc000125ee0, 0xc000118040, {{0x19?, 0x0?}, {0xffffffff00125e28?, 0x3200?}}, 0x0, 0x0, 0x0, 0xffff)
        C:/Users/Byfron/go/pkg/mod/golang.org/x/image@v0.18.0/vector/vector.go:404 +0x2d2
golang.org/x/image/vector.(*Rasterizer).Draw(0xc000125ee0, {0xe40de0, 0xc000118040}, {{0x0?, 0xe40de0?}, {0xc000118040?, 0x0?}}, {0xe40b10, 0xecd7a0?}, {0x0, ...})
        C:/Users/Byfron/go/pkg/mod/golang.org/x/image@v0.18.0/vector/vector.go:287 +0x19d
main.main()
        C:/Users/Byfron/Documents/Code/exp-vector-overflow/go-issue/main.go:34 +0x157
exit status 2

3. Drawing a 25x25 vector on 50x50 image.RGBA at (-10, 15).

Draws the skipped left part of the vector at right side of the image one pixel higher.

3. Drawing a 25x25 vector on 50x50 image.RGBA at (35, 15).

Draws the skipped right part of the vector at left side of the image one pixel lower.

What did you expect to see?

This issue can be splitted in two parts:

1. Panicing Which is totally unexpected as you can't even see any notes for possible panics.

   With image.RGBA and DrawOP as draw.Over, it calls rasterizeDstRGBASrcUniformOpOver which tries to get access to dst.Pix's out of range item (both negative and positive here and here)

   The same goes for image.Alpha and maybe DrawOP of draw.Src with these two image types. (Didn't dive into it that much)

2. Invalid drawings

   As far as I found out, those overflows happened because rasterizeDstRGBASrcUniformOpOver (same for image.Alpha and maybe DrawOP as draw.Src) writes the pixel colors without taking care of the dst's bounds but just the vector's bounds.

   For the third test case, We would expect this:

   

   And for the forth case, We would expect this:

   

The easiest possible fix I could think of, is to limit the r's bounds to the dst's bounds. if the r.Min is less than dst.Min, skip those differences and start from dst.Min, and same for r.Max and dst.Max.

Here's a basic (of cource not good enough for a stdlib) patch I've made for that single method:

func (z *Rasterizer) rasterizeDstRGBASrcUniformOpOver(dst *image.RGBA, r image.Rectangle, sr, sg, sb, sa uint32) {
    z.accumulateMask()

    // NEW: Calculate the offsets between dst's Min and r's Min; The goal is to
    // start drawing from the dst's Min.
    var xOffset, yOffset, pixOffset int
    if r.Min.X < dst.Rect.Min.X {
        xOffset = dst.Rect.Min.X - r.Min.X
    }
    if r.Min.Y < dst.Rect.Min.Y {
        yOffset = dst.Rect.Min.Y - r.Min.Y
    }

    // NEW: Calculate the maximum X and Y. Basically, limit the r's Max to the dst's Max.
    xMax, yMax := r.Max.X, r.Max.Y
    if xMax > dst.Rect.Max.X {
        xMax = dst.Rect.Max.X
    }
    if yMax > dst.Rect.Max.Y {
        yMax = dst.Rect.Max.Y
    }

    // CHANGED: Removed dst.Pix[...] and just kept this. You'll find more details down bellow.
    pixOffset = dst.PixOffset(r.Min.X, r.Min.Y)

    // CHANGED: Start x and y from the calculated offset.
    // CHANGED: Using yMax and xMax instead of r.Min.X and r.Min.Y for y1 and x1.
    for y, y1 := yOffset, yMax-r.Min.Y; y < y1; y++ {
        for x, x1 := xOffset, xMax-r.Min.X; x < x1; x++ {
            ma := z.bufU32[y*z.size.X+x]

            // This formula is like rasterizeOpOver's, simplified for the
            // concrete dst type and uniform src assumption.
            a := 0xffff - (sa * ma / 0xffff)
            i := pixOffset + y*dst.Stride + 4*x // CHANGED: added pixOffset

            // CHANGED: We can't pre-calculate the pix as it could be negative
            // ore more than the dst's pixels. So I just used it directly here.
            dst.Pix[i+0] = uint8(((uint32(dst.Pix[i+0])*0x101*a + sr*ma) / 0xffff) >> 8)
            dst.Pix[i+1] = uint8(((uint32(dst.Pix[i+1])*0x101*a + sg*ma) / 0xffff) >> 8)
            dst.Pix[i+2] = uint8(((uint32(dst.Pix[i+2])*0x101*a + sb*ma) / 0xffff) >> 8)
            dst.Pix[i+3] = uint8(((uint32(dst.Pix[i+3])*0x101*a + sa*ma) / 0xffff) >> 8)
        }
    }
}

This change, fixes drawing a vector with DrawOP of draw.Over on a image.RGBA with a image.Uniform src, but still image.Alpha remains and I couldn't really find out how it works (I lack of knowledge 😅). Also I didn't do any tests on draw.Src, but as they have almost similar code, I think this issue could related to them too.

The repository I mentioned earlier (this) contains everything I could possibly think of and has a little bit more details if it helps.

[ianlancetaylor]

CC @nigeltao

[gabyhelp]

Related Issues and Documentation

• x/image/draw: panic resizing image.Uniform #60412
• x/image: LineTo has an unexpected behaviour, or is not enough documented #45723 (closed)
• x/image: draw.Copy causes stack overflow when dr and sr have same size and DstMask is not nil #23107 (closed)
• image: decode panic: runtime error: makeslice: len out of range #45694
• image/gif: panics for several GIFs #5401 (closed)
• x/image: vector.go rasterizer shifts alpha mask and is slow when target is offset and small relative image size #25657
• x/exp/shiny/iconvg: crash when rasterizing icons with translucent palette #39526 (closed)
• x/image/bmp: makeslice: len out of range #10396 (closed)
• image: Decode(r io.Reader) return invalid image without error #48612 (closed)
• image/png: panic whilst decoding png #22304 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}