image/png: integer underflow when decoding

This change addresses an integer underflow appearing only on systems
using a 32-bit int type. The patch addresses the problem by limiting the
length of unknown chunks to 0x7fffffff. This value appears to already be
checked for when parsing other chunk types, so the bug shouldn't appear
elsewhere in the package. The PNG spec recommends the maximum size for
any chunk to remain under 2^31, so this shouldn't cause errors with
valid images.

Fixes #12687

Change-Id: I17f0e1683515532c661cf2b0b2bc65309d1b7bb7
Reviewed-on: https://go-review.googlesource.com/14766
Reviewed-by: Nigel Tao <nigeltao@golang.org>

diff --git a/src/image/png/reader.go b/src/image/png/reader.go
index ae6b775b4e701f65e7528a65e18e3a3a883eb688..9e6f985f7e2b41ce67f36ea90c6471dc3bcbecc9 100644 (file)
--- a/src/image/png/reader.go
+++ b/src/image/png/reader.go
@@ -727,6 +727,9 @@ func (d *decoder) parseChunk() error {
                d.stage = dsSeenIEND
                return d.parseIEND(length)
        }
+       if length > 0x7fffffff {
+               return FormatError(fmt.Sprintf("Bad chunk length: %d", length))
+       }
        // Ignore this chunk (of a known length).
        var ignored [4096]byte
        for length > 0 {

diff --git a/src/image/png/reader_test.go b/src/image/png/reader_test.go
index f89e7efe7fedebb97d31970e34def3d2089ac755..f058f6b2275ccb83020dfb1a8c8cc57577800f62 100644 (file)
--- a/src/image/png/reader_test.go
+++ b/src/image/png/reader_test.go
@@ -408,6 +408,18 @@ func TestMultipletRNSChunks(t *testing.T) {
        }
 }
 
+func TestUnknownChunkLengthUnderflow(t *testing.T) {
+       data := []byte{0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0xff, 0xff,
+               0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x06, 0xf4, 0x7c, 0x55, 0x04, 0x1a,
+               0xd3, 0x11, 0x9a, 0x73, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e, 0x00, 0x00,
+               0x01, 0x00, 0xff, 0xff, 0xff, 0xff, 0x07, 0xf4, 0x7c, 0x55, 0x04, 0x1a,
+               0xd3}
+       _, err := Decode(bytes.NewReader(data))
+       if err == nil {
+               t.Errorf("Didn't fail reading an unknown chunk with length 0xffffffff")
+       }
+}
+
 func benchmarkDecode(b *testing.B, filename string, bytesPerPixel int) {
        b.StopTimer()
        data, err := ioutil.ReadFile(filename)