From 87700cf75d660597e70d2eb7e3760f12232562ff Mon Sep 17 00:00:00 2001
From: "Jeff R. Allen" <jra@nella.org>
Date: Fri, 22 Mar 2013 09:30:31 -0700
Subject: [PATCH] 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
---
 src/pkg/image/gif/reader.go      | 10 ++++++-
 src/pkg/image/gif/reader_test.go | 49 ++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/pkg/image/gif/reader.go b/src/pkg/image/gif/reader.go
index 2e0fed5e59a..8e8531f9b6b 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 77810f8ccd2..a035ef1ea51 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)
+}