image/jpeg: ensure that we can't unread a byte if we didn't read a byte.

Fixes golang#10413 Change-Id: I7a4ecd042c40f786ea7406c670d561b1c1179bf0 Reviewed-on: https://go-review.googlesource.com/8998 Reviewed-by: Rob Pike <[email protected]>
rohythakur · Apr 22, 2015 · 72e867e · 72e867e
1 parent 03eb132
commit 72e867e
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 2 deletions.
diff --git a/src/image/jpeg/reader.go b/src/image/jpeg/reader.go
@@ -217,18 +217,19 @@ func (d *decoder) readByteStuffedByte() (x byte, err error) {
 		return 0xff, nil
 	}
 
+	d.bytes.nUnreadable = 0
+
 	x, err = d.readByte()
 	if err != nil {
 		return 0, err
 	}
+	d.bytes.nUnreadable = 1
 	if x != 0xff {
-		d.bytes.nUnreadable = 1
 		return x, nil
 	}
 
 	x, err = d.readByte()
 	if err != nil {
-		d.bytes.nUnreadable = 1
 		return 0, err
 	}
 	d.bytes.nUnreadable = 2

diff --git a/src/image/jpeg/reader_test.go b/src/image/jpeg/reader_test.go
@@ -15,6 +15,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 )
 
 // TestDecodeProgressive tests that decoding the baseline and progressive
@@ -206,6 +207,61 @@ func TestTruncatedSOSDataDoesntPanic(t *testing.T) {
 	}
 }
 
+func TestVeryLargeImageWithShortData(t *testing.T) {
+	// This input is an invalid JPEG image, generated by a fuzzer, as reported
+	// by issue 10413. It is only 504 bytes, and shouldn't take long for Decode
+	// to return an error. The Start Of Frame marker gives the image dimensions
+	// as 38655 wide and 16384 high, so even if an unreadByteStuffedByte bug
+	// doesn't technically lead to in an infinite loop, such a bug can still
+	// cause an unreasonably long loop for such a short input.
+	const input = "" +
+		"\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x00\x00\x01" +
+		"\x00\x01\x00\x00\xff\xdb\x00\x43\x00\x10\x0b\x0c\x0e\x0c\x0a\x10" +
+		"\x0e\x89\x0e\x12\x11\x10\x13\x18\xff\xd8\xff\xe0\x00\x10\x4a\x46" +
+		"\x49\x46\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xdb\x00\x43" +
+		"\x00\x10\x0b\x0c\x0e\x0c\x0a\x10\x0e\x0d\x0e\x12\x11\x10\x13\x18" +
+		"\x28\x1a\x18\x16\x16\x18\x31\x23\x25\x1d\x28\x3a\x33\x3d\x3c\x39" +
+		"\x33\x38\x37\x40\x48\x5c\x4e\x40\x44\x57\x45\x37\x38\x50\x6d\x51" +
+		"\x57\x5f\x62\x67\x68\x67\x3e\x4d\x71\x79\x70\x64\x78\x5c\x65\x67" +
+		"\x63\xff\xc0\x00\x0b\x08\x40\x00\x96\xff\x01\x01\x11\x00\xff\xc4" +
+		"\x00\x1f\x00\x00\x01\x05\x01\x01\x01\x01\x01\x01\x00\x00\x00\x00" +
+		"\x00\x00\x00\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\xff" +
+		"\xc4\x00\xb5\x10\x00\x02\x01\x03\x03\x02\x04\x03\x05\x05\x04\x04" +
+		"\x00\x00\x01\x7d\x01\x02\x03\x00\x04\x11\x05\x12\x21\x31\x01\x06" +
+		"\x13\x51\x61\x07\x22\x71\x14\x32\x81\x91\xa1\x08\x23\xd8\xff\xdd" +
+		"\x42\xb1\xc1\x15\x52\xd1\xf0\x24\x33\x62\x72\x82\x09\x0a\x16\x17" +
+		"\x18\x19\x1a\x25\x26\x27\x28\x29\x2a\x34\x35\x36\x37\x38\x39\x3a" +
+		"\x43\x44\x45\x46\x47\x48\x49\x4a\x53\x54\x55\x56\x57\x58\x59\x5a" +
+		"\x00\x63\x64\x65\x66\x67\x68\x69\x6a\x73\x74\x75\x76\x77\x78\x79" +
+		"\x7a\x83\x84\x85\x86\x87\x88\x89\x8a\x92\x93\x94\x95\x96\x97\x98" +
+		"\x99\x9a\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xb2\xb3\xb4\xb5\xb6" +
+		"\xb7\xb8\xb9\xba\xc2\xc3\xc4\xc5\xc6\xc7\xff\xd8\xff\xe0\x00\x10" +
+		"\x4a\x46\x49\x46\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xdb" +
+		"\x00\x43\x00\x10\x0b\x0c\x0e\x0c\x0a\x10\x0e\x0d\x0e\x12\x11\x10" +
+		"\x13\x18\x28\x1a\x18\x16\x16\x18\x31\x23\x25\x1d\xc8\xc9\xca\xd2" +
+		"\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8" +
+		"\xe9\xea\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xff\xda\x00\x08" +
+		"\x01\x01\x00\x00\x3f\x00\xb9\xeb\x50\xb0\xdb\xc8\xa8\xe4\x63\x80" +
+		"\xdd\x31\xd6\x9d\xbb\xf2\xc5\x42\x1f\x6c\x6f\xf4\x34\xdd\x3c\xfc" +
+		"\xac\xe7\x3d\x80\xa9\xcc\x87\x34\xb3\x37\xfa\x2b\x9f\x6a\xad\x63" +
+		"\x20\x36\x9f\x78\x64\x75\xe6\xab\x7d\xb2\xde\x29\x70\xd3\x20\x27" +
+		"\xde\xaf\xa4\xf0\xca\x9f\x24\xa8\xdf\x46\xa8\x24\x84\x96\xe3\x77" +
+		"\xf9\x2e\xe0\x0a\x62\x7f\xdf\xd9"
+	c := make(chan error, 1)
+	go func() {
+		_, err := Decode(strings.NewReader(input))
+		c <- err
+	}()
+	select {
+	case err := <-c:
+		if err == nil {
+			t.Fatalf("got nil error, want non-nil")
+		}
+	case <-time.After(10 * time.Second):
+		t.Fatalf("timed out")
+	}
+}
+
 func TestExtraneousData(t *testing.T) {
 	// Encode a 1x1 red image.
 	src := image.NewRGBA(image.Rect(0, 0, 1, 1))