From: Jeff R. Allen <jra@nella.org>
Date: Fri, 22 Mar 2013 16:30:31 +0000 (-0700)
Subject: image/gif: reject a GIF image if frame bounds larger than image bounds
X-Git-Tag: go1.1rc2~383
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=87700cf75d660597e70d2eb7e3760f12232562ff;p=gostls13.git

image/gif: reject a GIF image if frame bounds larger than image bounds

The GIF89a spec says: "Each image must fit within the
boundaries of the Logical Screen, as defined in the
Logical Screen Descriptor." Also, do not accept
GIFs which have too much data for the image size.

R=nigeltao, jra, r
CC=bradfitz, golang-dev
https://golang.org/cl/7602045
---

diff --git a/src/pkg/image/gif/reader.go b/src/pkg/image/gif/reader.go
index 2e0fed5e59..8e8531f9b6 100644
--- a/src/pkg/image/gif/reader.go
+++ b/src/pkg/image/gif/reader.go
@@ -348,7 +348,15 @@ func (d *decoder) newImageFromDescriptor() (*image.Paletted, error) {
 	width := int(d.tmp[4]) + int(d.tmp[5])<<8
 	height := int(d.tmp[6]) + int(d.tmp[7])<<8
 	d.imageFields = d.tmp[8]
-	return image.NewPaletted(image.Rect(left, top, left+width, top+height), nil), nil
+
+	// The GIF89a spec, Section 20 (Image Descriptor) says:
+	// "Each image must fit within the boundaries of the Logical
+	// Screen, as defined in the Logical Screen Descriptor."
+	bounds := image.Rect(left, top, left+width, top+height)
+	if bounds != bounds.Intersect(image.Rect(0, 0, d.width, d.height)) {
+		return nil, errors.New("gif: frame bounds larger than image bounds")
+	}
+	return image.NewPaletted(bounds, nil), nil
 }
 
 func (d *decoder) readBlock() (int, error) {
diff --git a/src/pkg/image/gif/reader_test.go b/src/pkg/image/gif/reader_test.go
index 77810f8ccd..a035ef1ea5 100644
--- a/src/pkg/image/gif/reader_test.go
+++ b/src/pkg/image/gif/reader_test.go
@@ -84,3 +84,52 @@ func TestDecode(t *testing.T) {
 		}
 	}
 }
+
+// testGIF is a simple GIF that we can modify to test different scenarios.
+var testGIF = []byte{
+	'G', 'I', 'F', '8', '9', 'a',
+	1, 0, 1, 0, // w=1, h=1 (6)
+	128, 0, 0, // headerFields, bg, aspect (10)
+	0, 0, 0, 1, 1, 1, // color map and graphics control (13)
+	0x21, 0xf9, 0x04, 0x00, 0x00, 0x00, 0xff, 0x00, // (19)
+	// frame 1 (0,0 - 1,1)
+	0x2c,
+	0x00, 0x00, 0x00, 0x00,
+	0x01, 0x00, 0x01, 0x00, // (32)
+	0x00,
+	0x02, 0x02, 0x4c, 0x01, 0x00, // lzw pixels
+	// trailer
+	0x3b,
+}
+
+func try(t *testing.T, b []byte, want string) {
+	_, err := DecodeAll(bytes.NewReader(b))
+	var got string
+	if err != nil {
+		got = err.Error()
+	}
+	if got != want {
+		t.Fatalf("got %v, want %v", got, want)
+	}
+}
+
+func TestBounds(t *testing.T) {
+	// Make the bounds too big, just by one.
+	testGIF[32] = 2
+	want := "gif: frame bounds larger than image bounds"
+	try(t, testGIF, want)
+
+	// Make the bounds too small; does not trigger bounds
+	// check, but now there's too much data.
+	testGIF[32] = 0
+	want = "gif: too much image data"
+	try(t, testGIF, want)
+	testGIF[32] = 1
+
+	// Make the bounds really big, expect an error.
+	want = "gif: frame bounds larger than image bounds"
+	for i := 0; i < 4; i++ {
+		testGIF[32+i] = 0xff
+	}
+	try(t, testGIF, want)
+}