image/gif: GIF files with extraneous 0x00 bytes cause "gif: unknown block type: 0x00"

[kamermans]

What version of Go are you using (go version)?

$ go version
go version go1.14.1 linux/amd64

Does this issue reproduce with the latest release?

Yes (and it has probably existed for a very long time, possibly forever)

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/steve/.cache/go-build"
GOENV="/home/steve/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOOS="linux"
GOPATH="/home/steve/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build601247294=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14.1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14.1
uname -sr: Linux 5.3.0-46-generic
Distributor ID:	Ubuntu
Description:	Ubuntu 19.10
Release:	19.10
Codename:	eoan
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Ubuntu GLIBC 2.30-0ubuntu2.1) stable release version 2.30.
gdb --version: GNU gdb (Ubuntu 8.3-0ubuntu1) 8.3

What did you do?

Tried to use image/gif DecodeAll() on a GIF with extra 0x00 byte(s) before the trailer byte 0x3b.

Example:
https://play.golang.org/p/vzVJUj4TRgm

What did you expect to see?

The image loads properly (as it does in browsers, ImageMagick, gifsicle and Eye Of GNOME).

What did you see instead?

gif: unknown block type: 0x00

Comments

This is caused by a more strict implementation of the GIF file format than most other programs. We already address this issue for extraneous trailing bytes in the LZW decoder (#16146), but I am also seeing them elsewhere between blocks in real files, generally before the closing trailer byte.

Here are the hex dumps of some example files that I have found:

./test/gifs/PIL-V_AN.GIF
00000130  a8 e7 5c 3e 1f 8d d7 f1  9d e7 3b d8 c3 05 00 c4  |..\>......;.....|
00000140  60 2f 56 34 d2 2a 49 a5  e9 d1 74 c2 86 51 65 aa  |`/V4.*I...t..Qe.|
00000150  4a cc 6a 27 d9 a7 68 d4  fc 5e 21 53 31 35 50 36  |J.j'..h..^!S15P6|
00000160  03 be 05 00 00 3b                                 |.....;|
00000166
./test/gifs/CDROM.GIF
00004190  2e 00 f0 8f a2 b9 10 5c  65 5b 06 e0 55 1b c0 7c  |.......\e[..U..||
000041a0  5f 85 02 2e e7 6a b3 89  8a cc c6 4a bf 39 9c c4  |_....j.....J.9..|
000041b0  59 9c 0c c6 79 9c c8 99  9c ca b9 9c c6 19 10 00  |Y...y...........|
000041c0  00 3b                                             |.;|
000041c2
./test/gifs/LYSSKILT.GIF
00001c20  b1 a8 8f ac ac ca 69 b0  cc 79 c7 74 d3 ad 8f da  |......i..y.t....|
00001c30  d1 96 b6 cd dd 9d d7 f0  f1 f2 f3 f4 f5 f6 f7 f8  |................|
00001c40  f9 fa fb fc fd fe ff 00  03 0a 1c 48 b0 47 04 00  |...........H.G..|
00001c50  00 3b                                             |.;|
00001c52
./test/gifs/PIL-N_AN.GIF
00000140  12 dd 50 31 2a 1e 2b 38  e5 92 e7 14 f2 0c d1 1b  |..P1*.+8........|
00000150  a9 64 14 01 73 ba 29 97  b3 bd 46 af b1 09 99 04  |.d..s.)...F.....|
00000160  38 ab d7 ec b6 fb 0d 8f  cb e7 f4 ba fd 8e cf eb  |8...............|
00000170  c9 05 00 00 3b                                    |....;|
00000175
./test/gifs/PIL-O_AN.GIF
00000130  c4 f2 3c d9 b7 08 f4 be  2e f1 35 7a 37 1a 0a 98  |..<.......5z7...|
00000140  19 22 35 44 99 51 59 4c  c2 96 15 ea 31 ca b4 42  |."5D.QYL....1..B|
00000150  77 d9 9f f7 03 0e 8b c7  e4 b2 f9 8c 4e ab d7 ec  |w...........N...|
00000160  b6 fb 0d 67 17 00 00 00  3b                       |...g....;|
00000169
./test/gifs/LYS.GIF
000026b0  e8 c5 d0 5e 05 15 24 80  60 05 fe b9 77 1c 72 04  |...^..$.`...w.r.|
000026c0  2e a0 c2 09 09 d2 04 d7  85 0c 76 f0 20 6f 01 44  |..........v. o.D|
000026d0  b0 c0 03 09 0e 90 d8 00  02 50 55 80 03 28 3e 88  |.........PU..(>.|
000026e0  5c 40 00 00 3b                                    |\@..;|
000026e5
./test/gifs/MOOSE.GIF
000048d0  20 63 72 65 61 74 65 64  20 77 69 74 68 20 61 20  | created with a |
000048e0  72 65 67 69 73 74 65 72  65 64 20 76 65 72 73 69  |registered versi|
000048f0  6f 6e 20 6f 66 20 47 49  46 20 43 6f 6e 73 74 72  |on of GIF Constr|
00004900  75 63 74 69 6f 6e 20 53  65 74 00 00              |uction Set..|
0000490c
./test/gifs/SOUNDER2.GIF
00000210  9b c5 57 57 3e 31 16 63  90 a5 b8 95 5e 98 36 2b  |..WW>1.c....^.6+|
00000220  fa a9 d1 db b9 18 fd 50  09 9c 9d b9 df a3 01 67  |.......P.......g|
00000230  43 56 0c 37 32 25 8f 8a  a6 c2 e4 74 16 00 00 00  |CV.72%.....t....|
00000240  3b                                                |;|
00000241
./test/gifs/PIL-H_AN.GIF
00000140  f1 68 a9 60 70 78 2a 1a  81 c8 90 72 19 6b 66 9e  |.h.`px*....r.kf.|
00000150  50 18 f1 57 8d f5 a8 d9  5e 0d 9b b5 78 bf 61 d7  |P..W....^...x.a.|
00000160  f8 bb 49 ab 9f 67 89 b2  ed c6 c2 e3 e2 79 64 64  |..I..g.......ydd|
00000170  77 4b 0a 00 00 3b                                 |wK...;|
00000176
./test/gifs/COFFEEMA.GIF
00004a30  ea 05 4e b5 44 b6 71 06  67 f2 10 31 da ae e3 4a  |..N.D.q.g..1...J|
00004a40  21 c3 73 61 fc 85 15 7b  21 a5 f6 3a 0c 6e 18 a5  |!.sa...{!..:.n..|
00004a50  40 79 a8 ff 4a 0b 44 27  a5 46 70 05 f5 5a b0 0c  |@y..J.D'.Fp..Z..|
00004a60  db b0 b2 12 02 00 00 3b                           |.......;|
00004a68

Here are the images referenced above, they are all very small:
https://drive.google.com/drive/folders/13hKuVy_Kf2MeyWacXLPanwhH2uoIxLUe?usp=sharing

I have a simple solution so I'll put in a PR, but I'm not sure how strict you want to be in handling this (is one extraneous byte OK, but two not OK, for example?).

[gopherbot]

Change https://golang.org/cl/232057 mentions this issue: image/gif: ignore extra empty bytes between blocks while decoding

[ianlancetaylor]

CC @nigeltao

[nigeltao]

Note that MOOSE.GIF doesn't even have a trailing 0x3B byte.

[kamermans]

Yes, good point! I'm going to run some tests against a larger set of 100k GIFs, as per the code review. The files above are all really old samples (probably from the 90s), so it would be good to get a feel for the state of the situation as it is in the wild today.

[kamermans]

FYI: Here is the source of the images above:
http://cs.gettysburg.edu/~duncjo01/archive/clipart/kumc/

For example, here is the moose image:
http://cs.gettysburg.edu/~duncjo01/archive/clipart/kumc/Animals/moose.gif

[thaustad]

See also 4775b7f

[jaydenmilne]

Still seeing this one in the wild btw, you can get malformed GIFs like this on demand from UDOT traffic cameras:

https://www.udottraffic.utah.gov/map/Cctv/6585

The end of the file looks like this: 57 6C F1 C5 18 67 5C 71 40 00 00 3B. I tested one of these images and can confirm the (abandoned) changeset https://golang.org/cl/232057 would fix it.

FWIW it looks like it does parse the frames of the malformed gif fine, I'm wondering if it wouldn't be a terrible idea to have DecodeAll return the parsed image so far as well as the error.