mirror of
https://github.com/golang/go
synced 2024-11-18 15:54:42 -07:00
image/png: fix some 32-bit int overflows
Fixes #38435 Change-Id: Ib9ae3cf7f338b2860a5688e448a125f257fe624e Reviewed-on: https://go-review.googlesource.com/c/go/+/230219 Reviewed-by: Andrew Ekstedt <andrew.ekstedt@gmail.com> Reviewed-by: Rob Pike <r@golang.org>
This commit is contained in:
Nigel Tao
parent
b7a5e7ae92
commit
bce1e25b71
@ -163,8 +163,9 @@ func (d *decoder) parseIHDR(length uint32) error {
|
||||
if w <= 0 || h <= 0 {
|
||||
return FormatError("non-positive dimension")
|
||||
}
|
||||
nPixels := int64(w) * int64(h)
|
||||
if nPixels != int64(int(nPixels)) {
|
||||
nPixels64 := int64(w) * int64(h)
|
||||
nPixels := int(nPixels64)
|
||||
if nPixels64 != int64(nPixels) {
|
||||
return UnsupportedError("dimension overflow")
|
||||
}
|
||||
// There can be up to 8 bytes per pixel, for 16 bits per channel RGBA.
|
||||
@ -498,7 +499,10 @@ func (d *decoder) readImagePass(r io.Reader, pass int, allocateOnly bool) (image
|
||||
bytesPerPixel := (bitsPerPixel + 7) / 8
|
||||
|
||||
// The +1 is for the per-row filter type, which is at cr[0].
|
||||
rowSize := 1 + (bitsPerPixel*width+7)/8
|
||||
rowSize := 1 + (int64(bitsPerPixel)*int64(width)+7)/8
|
||||
if rowSize != int64(int(rowSize)) {
|
||||
return nil, UnsupportedError("dimension overflow")
|
||||
}
|
||||
// cr and pr are the bytes for the current and previous row.
|
||||
cr := make([]uint8, rowSize)
|
||||
pr := make([]uint8, rowSize)
|
||||
|
||||
@ -661,20 +661,126 @@ func TestGray8Transparent(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDimensionOverflow(t *testing.T) {
|
||||
// These bytes come from https://golang.org/issues/22304
|
||||
//
|
||||
// It encodes a 2147483646 × 2147483646 (i.e. 0x7ffffffe × 0x7ffffffe)
|
||||
// NRGBA image. The (width × height) per se doesn't overflow an int64, but
|
||||
// (width × height × bytesPerPixel) will.
|
||||
_, err := Decode(bytes.NewReader([]byte{
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x7f, 0xff, 0xff, 0xfe, 0x7f, 0xff, 0xff, 0xfe, 0x08, 0x06, 0x00, 0x00, 0x00, 0x30, 0x57, 0xb3,
|
||||
0xfd, 0x00, 0x00, 0x00, 0x15, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x62, 0x20, 0x12, 0x8c,
|
||||
0x2a, 0xa4, 0xb3, 0x42, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x13, 0x38, 0x00, 0x15, 0x2d, 0xef,
|
||||
0x5f, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
|
||||
}))
|
||||
if _, ok := err.(UnsupportedError); !ok {
|
||||
t.Fatalf("Decode: got %v (of type %T), want non-nil error (of type png.UnsupportedError)", err, err)
|
||||
maxInt32AsInt := int((1 << 31) - 1)
|
||||
have32BitInts := 0 > (1 + maxInt32AsInt)
|
||||
|
||||
testCases := []struct {
|
||||
src []byte
|
||||
unsupportedConfig bool
|
||||
width int
|
||||
height int
|
||||
}{
|
||||
// These bytes come from https://golang.org/issues/22304
|
||||
//
|
||||
// It encodes a 2147483646 × 2147483646 (i.e. 0x7ffffffe × 0x7ffffffe)
|
||||
// NRGBA image. The (width × height) per se doesn't overflow an int64, but
|
||||
// (width × height × bytesPerPixel) will.
|
||||
{
|
||||
src: []byte{
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x7f, 0xff, 0xff, 0xfe, 0x7f, 0xff, 0xff, 0xfe, 0x08, 0x06, 0x00, 0x00, 0x00, 0x30, 0x57, 0xb3,
|
||||
0xfd, 0x00, 0x00, 0x00, 0x15, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x62, 0x20, 0x12, 0x8c,
|
||||
0x2a, 0xa4, 0xb3, 0x42, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x13, 0x38, 0x00, 0x15, 0x2d, 0xef,
|
||||
0x5f, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
|
||||
},
|
||||
// It's debatable whether DecodeConfig (which does not allocate a
|
||||
// pixel buffer, unlike Decode) should fail in this case. The Go
|
||||
// standard library has made its choice, and the standard library
|
||||
// has compatibility constraints.
|
||||
unsupportedConfig: true,
|
||||
width: 0x7ffffffe,
|
||||
height: 0x7ffffffe,
|
||||
},
|
||||
|
||||
// The next three cases come from https://golang.org/issues/38435
|
||||
|
||||
{
|
||||
src: []byte{
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x00, 0x00, 0xb5, 0x04, 0x00, 0x00, 0xb5, 0x04, 0x08, 0x06, 0x00, 0x00, 0x00, 0xf5, 0x60, 0x2c,
|
||||
0xb8, 0x00, 0x00, 0x00, 0x15, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x62, 0x20, 0x12, 0x8c,
|
||||
0x2a, 0xa4, 0xb3, 0x42, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x13, 0x38, 0x00, 0x15, 0x2d, 0xef,
|
||||
0x5f, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
|
||||
},
|
||||
// Here, width * height = 0x7ffea810, just under MaxInt32, but at 4
|
||||
// bytes per pixel, the number of pixels overflows an int32.
|
||||
unsupportedConfig: have32BitInts,
|
||||
width: 0x0000b504,
|
||||
height: 0x0000b504,
|
||||
},
|
||||
|
||||
{
|
||||
src: []byte{
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0x30, 0x6e, 0xc5,
|
||||
0x21, 0x00, 0x00, 0x00, 0x15, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x62, 0x20, 0x12, 0x8c,
|
||||
0x2a, 0xa4, 0xb3, 0x42, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x13, 0x38, 0x00, 0x15, 0x2d, 0xef,
|
||||
0x5f, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
|
||||
},
|
||||
unsupportedConfig: false,
|
||||
width: 0x04000000,
|
||||
height: 0x00000001,
|
||||
},
|
||||
|
||||
{
|
||||
src: []byte{
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0xaa, 0xd4, 0x7c,
|
||||
0xda, 0x00, 0x00, 0x00, 0x15, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x62, 0x66, 0x20, 0x12, 0x30,
|
||||
0x8d, 0x2a, 0xa4, 0xaf, 0x42, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x14, 0xd2, 0x00, 0x16, 0x00,
|
||||
0x00, 0x00,
|
||||
},
|
||||
unsupportedConfig: false,
|
||||
width: 0x08000000,
|
||||
height: 0x00000001,
|
||||
},
|
||||
}
|
||||
|
||||
for i, tc := range testCases {
|
||||
cfg, err := DecodeConfig(bytes.NewReader(tc.src))
|
||||
if tc.unsupportedConfig {
|
||||
if err == nil {
|
||||
t.Errorf("i=%d: DecodeConfig: got nil error, want non-nil", i)
|
||||
} else if _, ok := err.(UnsupportedError); !ok {
|
||||
t.Fatalf("Decode: got %v (of type %T), want non-nil error (of type png.UnsupportedError)", err, err)
|
||||
}
|
||||
continue
|
||||
} else if err != nil {
|
||||
t.Errorf("i=%d: DecodeConfig: %v", i, err)
|
||||
continue
|
||||
} else if cfg.Width != tc.width {
|
||||
t.Errorf("i=%d: width: got %d, want %d", i, cfg.Width, tc.width)
|
||||
continue
|
||||
} else if cfg.Height != tc.height {
|
||||
t.Errorf("i=%d: height: got %d, want %d", i, cfg.Height, tc.height)
|
||||
continue
|
||||
}
|
||||
|
||||
if nPixels := int64(cfg.Width) * int64(cfg.Height); nPixels > 0x7f000000 {
|
||||
// In theory, calling Decode would succeed, given several gigabytes
|
||||
// of memory. In practice, trying to make a []uint8 big enough to
|
||||
// hold all of the pixels can often result in OOM (out of memory).
|
||||
// OOM is unrecoverable; we can't write a test that passes when OOM
|
||||
// happens. Instead we skip the Decode call (and its tests).
|
||||
continue
|
||||
} else if testing.Short() {
|
||||
// Even for smaller image dimensions, calling Decode might allocate
|
||||
// 1 GiB or more of memory. This is usually feasible, and we want
|
||||
// to check that calling Decode doesn't panic if there's enough
|
||||
// memory, but we provide a runtime switch (testing.Short) to skip
|
||||
// these if it would OOM. See also http://golang.org/issue/5050
|
||||
// "decoding... images can cause huge memory allocations".
|
||||
continue
|
||||
}
|
||||
|
||||
// Even if we don't panic, these aren't valid PNG images.
|
||||
if _, err := Decode(bytes.NewReader(tc.src)); err == nil {
|
||||
t.Errorf("i=%d: Decode: got nil error, want non-nil", i)
|
||||
}
|
||||
}
|
||||
|
||||
if testing.Short() {
|
||||
t.Skip("skipping tests which allocate large pixel buffers")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user