From 1be1b0af357759f1e162d5cd3906725c8b93eec2 Mon Sep 17 00:00:00 2001
From: Benny Siegert <bsiegert@gmail.com>
Date: Tue, 28 Apr 2015 17:45:40 +0200
Subject: [PATCH] tiff: reject TIFF images with unsorted IFD tags.

The spec says that these images are invalid. Add a test with an invalid
tiff generated by go-fuzz.

Fixes golang/go#10549

Change-Id: I3fd3ae5e607202b41735a2d930f55cb7997f7a9b
Reviewed-on: https://go-review.googlesource.com/9377
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
---
 tiff/reader.go      | 25 ++++++++++++++++---------
 tiff/reader_test.go | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/tiff/reader.go b/tiff/reader.go
index df39e82..cfe6fed 100644
--- a/tiff/reader.go
+++ b/tiff/reader.go
@@ -118,8 +118,9 @@ func (d *decoder) ifdUint(p []byte) (u []uint, err error) {
 }
 
 // parseIFD decides whether the the IFD entry in p is "interesting" and
-// stows away the data in the decoder.
-func (d *decoder) parseIFD(p []byte) error {
+// stows away the data in the decoder. It returns the tag number of the
+// entry and an error, if any.
+func (d *decoder) parseIFD(p []byte) (int, error) {
 	tag := d.byteOrder.Uint16(p[0:2])
 	switch tag {
 	case tBitsPerSample,
@@ -138,17 +139,17 @@ func (d *decoder) parseIFD(p []byte) error {
 		tImageWidth:
 		val, err := d.ifdUint(p)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		d.features[int(tag)] = val
 	case tColorMap:
 		val, err := d.ifdUint(p)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		numcolors := len(val) / 3
 		if len(val)%3 != 0 || numcolors <= 0 || numcolors > 256 {
-			return FormatError("bad ColorMap length")
+			return 0, FormatError("bad ColorMap length")
 		}
 		d.palette = make([]color.Color, numcolors)
 		for i := 0; i < numcolors; i++ {
@@ -166,15 +167,15 @@ func (d *decoder) parseIFD(p []byte) error {
 		// must terminate the import process gracefully.
 		val, err := d.ifdUint(p)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		for _, v := range val {
 			if v != 1 {
-				return UnsupportedError("sample format")
+				return 0, UnsupportedError("sample format")
 			}
 		}
 	}
-	return nil
+	return int(tag), nil
 }
 
 // readBits reads n bits from the internal buffer starting at the current offset.
@@ -428,10 +429,16 @@ func newDecoder(r io.Reader) (*decoder, error) {
 		return nil, err
 	}
 
+	prevTag := -1
 	for i := 0; i < len(p); i += ifdLen {
-		if err := d.parseIFD(p[i : i+ifdLen]); err != nil {
+		tag, err := d.parseIFD(p[i : i+ifdLen])
+		if err != nil {
 			return nil, err
 		}
+		if tag <= prevTag {
+			return nil, FormatError("tags are not sorted in ascending order")
+		}
+		prevTag = tag
 	}
 
 	d.config.Width = int(d.firstVal(tImageWidth))
diff --git a/tiff/reader_test.go b/tiff/reader_test.go
index f5c02e6..f1cf93b 100644
--- a/tiff/reader_test.go
+++ b/tiff/reader_test.go
@@ -192,6 +192,24 @@ func TestDecodeLZW(t *testing.T) {
 	compare(t, img0, img1)
 }
 
+// TestDecodeTagOrder tests that a malformed image with unsorted IFD entries is
+// correctly rejected.
+func TestDecodeTagOrder(t *testing.T) {
+	data, err := ioutil.ReadFile("../testdata/video-001.tiff")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Swap the first two IFD entries.
+	ifdOffset := int64(binary.LittleEndian.Uint32(data[4:8]))
+	for i := ifdOffset + 2; i < ifdOffset+14; i++ {
+		data[i], data[i+12] = data[i+12], data[i]
+	}
+	if _, _, err := image.Decode(bytes.NewReader(data)); err == nil {
+		t.Fatal("got nil error, want non-nil")
+	}
+}
+
 // TestDecompress tests that decoding some TIFF images that use different
 // compression formats result in the same pixel data.
 func TestDecompress(t *testing.T) {