From 299f524d90c2c11852077eba582a579d94f33fe8 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 7 Sep 2011 13:23:16 -0400 Subject: [PATCH] image/png: check zlib checksum during Decode R=nigeltao CC=golang-dev https://golang.org/cl/4987041 --- src/pkg/image/png/reader.go | 10 ++++++ src/pkg/image/png/reader_test.go | 31 +++++++++++++++++-- src/pkg/image/png/testdata/invalid-crc32.png | Bin 0 -> 1289 bytes src/pkg/image/png/testdata/invalid-noend.png | Bin 0 -> 1277 bytes src/pkg/image/png/testdata/invalid-trunc.png | Bin 0 -> 1288 bytes src/pkg/image/png/testdata/invalid-zlib.png | Bin 0 -> 1289 bytes src/pkg/image/png/writer_test.go | 4 +-- 7 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 src/pkg/image/png/testdata/invalid-crc32.png create mode 100644 src/pkg/image/png/testdata/invalid-noend.png create mode 100644 src/pkg/image/png/testdata/invalid-trunc.png create mode 100644 src/pkg/image/png/testdata/invalid-zlib.png diff --git a/src/pkg/image/png/reader.go b/src/pkg/image/png/reader.go index 8c76afa72c6..aa023741d07 100644 --- a/src/pkg/image/png/reader.go +++ b/src/pkg/image/png/reader.go @@ -489,6 +489,16 @@ func (d *decoder) idatReader(idat io.Reader) (image.Image, os.Error) { // The current row for y is the previous row for y+1. pr, cr = cr, pr } + + // Check for EOF, to verify the zlib checksum. + n, err := r.Read(pr[:1]) + if err != os.EOF { + return nil, FormatError(err.String()) + } + if n != 0 { + return nil, FormatError("too much pixel data") + } + return img, nil } diff --git a/src/pkg/image/png/reader_test.go b/src/pkg/image/png/reader_test.go index bcc1a3db475..20884319058 100644 --- a/src/pkg/image/png/reader_test.go +++ b/src/pkg/image/png/reader_test.go @@ -10,6 +10,7 @@ import ( "image" "io" "os" + "strings" "testing" ) @@ -41,7 +42,7 @@ var filenamesShort = []string{ "basn6a16", } -func readPng(filename string) (image.Image, os.Error) { +func readPNG(filename string) (image.Image, os.Error) { f, err := os.Open(filename) if err != nil { return nil, err @@ -183,7 +184,7 @@ func TestReader(t *testing.T) { } for _, fn := range names { // Read the .png file. - img, err := readPng("testdata/pngsuite/" + fn + ".png") + img, err := readPNG("testdata/pngsuite/" + fn + ".png") if err != nil { t.Error(fn, err) continue @@ -239,3 +240,29 @@ func TestReader(t *testing.T) { } } } + +var readerErrors = []struct { + file string + err string +}{ + {"invalid-zlib.png", "zlib checksum error"}, + {"invalid-crc32.png", "invalid checksum"}, + {"invalid-noend.png", "unexpected EOF"}, + {"invalid-trunc.png", "unexpected EOF"}, +} + +func TestReaderError(t *testing.T) { + for _, tt := range readerErrors { + img, err := readPNG("testdata/" + tt.file) + if err == nil { + t.Errorf("decoding %s: missing error", tt.file) + continue + } + if !strings.Contains(err.String(), tt.err) { + t.Errorf("decoding %s: %s, want %s", tt.file, err, tt.err) + } + if img != nil { + t.Errorf("decoding %s: have image + error") + } + } +} diff --git a/src/pkg/image/png/testdata/invalid-crc32.png b/src/pkg/image/png/testdata/invalid-crc32.png new file mode 100644 index 0000000000000000000000000000000000000000..e5be4086cb2f218e53a3296144d4f3f891e80012 GIT binary patch literal 1289 zcmeAS@N?(olHy`uVBq!ia0vp^FF=?BNH82Y^nEFiQYmqbC<)F_D=AMbN@WNP(go^K z$S5f(u+rBrEYd43FGLPw%hY$H2g{z|+Msq=GT|hrQE*74Gh~R-z10&ZzXGUg`KkZCxG9W>RgY_B-2N)rO>I`lu&R^Q- zF8NQGfhXXDJeNd4!~cK-46OezG61!sn<>nI>e)*N7W4e)WB|I3pOwer0MHJgYct@s zyECJj$=QG!WSI{j;Q+Kq2;@YQ0}X8d9)lb*-vi_ZT!Ti6fr=8t(^b literal 0 HcmV?d00001 diff --git a/src/pkg/image/png/testdata/invalid-noend.png b/src/pkg/image/png/testdata/invalid-noend.png new file mode 100644 index 0000000000000000000000000000000000000000..9137270d9c3a4b2d2590e6aa658c33e86510aa98 GIT binary patch literal 1277 zcmeAS@N?(olHy`uVBq!ia0vp^FF=?BNH82Y^nD4CQYmqbC<)F_D=AMbN@WNP(go^K z$S5f(u+rBrEYd43FGLPw%hY%fP_0z|+Msq=GT|hrQE*74Gh~R-z10&ZzXGUg`KkZCxG9W>RgY_B-2N)rO>I`lu&R^Q- zF8NQGfhXXDJeNd4!~cK-46OezG61!sn<>nI>e)*N7W4e)WB|I3pOwer0MHJgYct@s zyECJj$=QG!WSI{j;Q+Kq2;@YQ0}X8d9)lb*-vi_ZT!Ti6fr=8tLPw%hY%fP_0z|+Msq=GT|hrQE*74Gh~R-z10&ZzXGUg`KkZCxG9W>RgY_B-2N)rO>I`lu&R^Q- zF8NQGfhXXDJeNd4!~cK-46OezG61!sn<>nI>e)*N7W4e)WB|I3pOwer0MHJgYct@s zyECJj$=QG!WSI{j;Q+Kq2;@YQ0}X8d9)lb*-vi_ZT!Ti6fr=8tLPw%hY%fP_0z|+Msq=GT|hrQE*74Gh~R-z10&ZzXGUg`KkZCxG9W>RgY_B-2N)rO>I`lu&R^Q- zF8NQGfhXXDJeNd4!~cK-46OezG61!sn<>nI>e)*N7W4e)WB|I3pOwer0MHJgYct@s zyECJj$=QG!WSI{j;Q+Kq2;@YQ0}X8d9)lb*-vi_ZT!Ti6fr=8t