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 2e0fed5e59af3066df872807d7cb94f7f4767513..8e8531f9b6be39cc0e8a3fbd64c462ed3d8dc5b6 100644 (file)
--- 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 77810f8ccd2b0cb23aef927acc4ae23c7368a669..a035ef1ea5175e3aa61430e89d3f36eb8ec81f8d 100644 (file)
--- 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)
+}