golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.85k stars 17.65k forks source link

image/jpeg: panic while decoding a JPEG image #4259

Closed gopherbot closed 9 years ago

gopherbot commented 12 years ago

by samuel.stauffer:

With the latest tip version of go (as of yesterday), I get a panic when decoding the
attached JPEG image. The image is grayscale and quite possibly corrupt.

When i try with go 1.0.3 I get an error returned with no panic. The error is:
"invalid JPEG format: missing 0xff00 sequence"

The panic:

panic: runtime error: index out of range

goroutine 1 [running]:
image/jpeg.(*decoder).processSOS(0x421bf000, 0x6, 0x421c44b8, 0x40000000002, 0x2, ...)
    /Users/samuelks/go/src/pkg/image/jpeg/scan.go:295 +0xdf6
image/jpeg.(*decoder).decode(0x421bf000, 0x4218f9c0, 0x421bd000, 0xbc700, 0x0, ...)
    /Users/samuelks/go/src/pkg/image/jpeg/reader.go:275 +0x74d
image/jpeg.Decode(0x4218f9c0, 0x421bd000, 0x421bd000, 0x4218f9c0, 0x421bd000, ...)
    /Users/samuelks/go/src/pkg/image/jpeg/reader.go:299 +0x4d
image.Decode(0x4218f900, 0x4219f100, 0x4219f100, 0x4218f900, 0x4219f100, ...)
    /Users/samuelks/go/src/pkg/image/format.go:84 +0x137
main.main()
    /Users/samuelks/Dropbox/ImageTestData/imgtest.go:19 +0x21c

Attachments:

  1. 718065-crash.jpeg (27917 bytes)
gopherbot commented 12 years ago

Comment 1 by samuel.stauffer:

Forgot to include "expected behavior" in my report. I would expect image decoding never
to panic and always return a valid error instead.
robpike commented 12 years ago

Comment 2:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @nigeltao.

Status changed to Accepted.

rsc commented 11 years ago

Comment 4:

Labels changed: added size-m.

rsc commented 11 years ago

Comment 5:

Labels changed: added suggested.

cespare commented 11 years ago

Comment 6:

hg bisect shows that this was introduced by the change for decoding progressive jpegs:
http://code.google.com/p/go/source/detail?r=51f26e36ba98
nigeltao commented 11 years ago

Comment 7:

Ah, that particular image has a 2x2 sampling factor, which is common for color JPEG
images (e.g. 2x2,1x1,1x1 corresponds to YCbCr 4:2:0), but is a weird thing to do for
grayscale JPEG images. Still, I guess it's valid, and Go shouldn't choke on it.
Re comment #1, you're right that it's a bug if the Go JPEG decoder ever panics on
corrupt input, but FWIW, I'm glad it panicked when it did. If this code wasn't in a
bounds-checked language (like C) then the decoder would happily have overwritten an
unsafe memory region and we would never have noticed the bug as easily. Sorry for the
late response, though.
Re comment #6, the code change for progressive images changed the symptoms from error to
panic, but the underlying bug existed beforehand in go1.0.3, as the OP noted.
I've mailed out a fix at https://golang.org/cl/7069045
nigeltao commented 11 years ago

Comment 8:

This issue was closed by revision 30ff0636b77f0e64084cc976e927355edb6d6f3.

Status changed to Fixed.