Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image: decode panic: runtime error: makeslice: len out of range #45694

Open
orestonce opened this issue Apr 22, 2021 · 11 comments
Open

image: decode panic: runtime error: makeslice: len out of range #45694

orestonce opened this issue Apr 22, 2021 · 11 comments

Comments

@orestonce
Copy link

@orestonce orestonce commented Apr 22, 2021

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

$ go version
go version go1.16.3 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://goproxy.io"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build3967062922=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ go run main.go

package main

import (
	"image"
	_ "image/png"
	_ "image/jpeg"
	"bytes"
	"encoding/hex"
)

func main() {
	data, err := hex.DecodeString("89504e470d0a1a0a0000000d4948445200efbfbd0d0000200804000000ea1b40ad30303030494441544889")
	//data, err := hex.DecodeString("89504e470d0a1a0a0000000d49484452002056616c75653e0403000000c6a32a2b0000002d504c54452200ff00ffff8800ff22ff000099ffff6600dd00ff77ff00ff000000ff99ddff00ff00bbffbb000044ff00ff44d2b049bd30303030494441542891")
	if err != nil {
		panic(err)
	}
	image.Decode(bytes.NewReader(data))
}

play.golang.org

What did you expect to see?

not panic

What did you see instead?

panic: runtime error: makeslice: len out of range

goroutine 1 [running]:
image.NewNRGBA(0x0, 0x0, 0xefbfbd, 0xd000020, 0x4c83c0)
/usr/local/go/src/image/image.go:393 +0x8f
image/png.(*decoder).readImagePass(0xc00003ec00, 0x7f4024488058, 0xc000064050, 0x0, 0xc000064000, 0x503d00, 0xc000064050, 0x0, 0x0)
/usr/local/go/src/image/png/reader.go:450 +0x242e
image/png.(*decoder).decode(0xc00003ec00, 0x0, 0x0, 0x0, 0x0)
/usr/local/go/src/image/png/reader.go:372 +0x638
image/png.(*decoder).parseIDAT(0xc00003ec00, 0x30303030, 0x4e14b8, 0x4)
/usr/local/go/src/image/png/reader.go:849 +0x36
image/png.(*decoder).parseChunk(0xc00003ec00, 0x0, 0x0)
/usr/local/go/src/image/png/reader.go:908 +0x3a7
image/png.Decode(0x503908, 0xc00005a1e0, 0xc00005a1e0, 0x503908, 0xc00005a1e0, 0x8)
/usr/local/go/src/image/png/reader.go:967 +0x14f
image.Decode(0x503928, 0xc000070180, 0x60, 0xc000066060, 0x56, 0x60, 0x2b, 0x0)
/usr/local/go/src/image/format.go:93 +0x102
main.main()
/root/d/main.go:17 +0xfa
exit status 2

@WangLeonard
Copy link

@WangLeonard WangLeonard commented Apr 22, 2021

image
I can reproduce this problem,
its panic on image.Decode(bytes.NewReader(data))
when make a []uint8, its len is 13707555022823040.
Is it an error in the input parameters("89504e470d0a1a0a0000000d4948445200efbfbd0d0000200804000000ea1b40ad30303030494441544889")?

Loading

@randall77
Copy link
Contributor

@randall77 randall77 commented Apr 22, 2021

Ideally Decode would return an error instead of panicing here.
Some work as been done on making Decode robust against malformed input, but it's a hard problem. I don't think we provide any guarantees.

Loading

@randall77 randall77 added this to the Unplanned milestone Apr 22, 2021
@cainiaocome
Copy link

@cainiaocome cainiaocome commented Apr 23, 2021

/cc @golang/security

Loading

@opennota
Copy link

@opennota opennota commented Apr 23, 2021

For now one can use png.DecodeConfig to check if the width and height are reasonable before decoding the entire image.

Loading

@orestonce
Copy link
Author

@orestonce orestonce commented Apr 23, 2021

Ideally

For now one can use png.DecodeConfig to check if the width and height are reasonable before decoding the entire image.

For now one can use png.DecodeConfig to check if the width and height are reasonable before decoding the entire image.

Ideally Decode would return an error instead of panicing here.
Some work as been done on making Decode robust against malformed input, but it's a hard problem. I don't think we provide any guarantees.

Because the image.Decode function has determined to return an error, any input is in line with the expectations of image.Decode. Since it is not used incorrectly, image.Decode should return an error instead of panic.

Loading

@orestonce
Copy link
Author

@orestonce orestonce commented Apr 23, 2021

For now one can use png.DecodeConfig to check if the width and height are reasonable before decoding the entire image.

I think image.Decode and image.DecodeCofing should not be related to each other, nor should panic terminate unexpectedly during normal input.

Loading

@opennota
Copy link

@opennota opennota commented Apr 23, 2021

The panic should be fixed, of course, but until it is DecodeConfig could help.

Loading

@opennota
Copy link

@opennota opennota commented Apr 23, 2021

Note that Decode will also panic if there's not enough memory to load the image, so it's a good idea to check for unreasonable image dimensions anyway, before Decode.

Loading

@seankhliao seankhliao changed the title image decode panic: runtime error: makeslice: len out of range image: decode panic: runtime error: makeslice: len out of range Apr 23, 2021
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Apr 26, 2021

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented May 2, 2021

The thing is that a valid PNG file can claim to be quite big (e.g. a million pixels wide and a thousand pixels high). Decoding it (e.g. into an image.NRGBA) would require gigabytes of memory. Some computers can actually allocate gigabytes of memory, so they could decode these big but valid PNG files. It's hard to tell a priori whether such an allocation would fail.

See also #5050

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented May 2, 2021

From #5050 (comment)

The current image.Decode and {gif,jpeg,png}.Decode functions attempt to return a newly
allocated image. I think that to avoid e.g. denial of service attacks via legitimate but
very large images, it needs to be possible to 1. decode just the WxH (and other
metadata), 2. decide whether to proceed based on that metadata and then 3. decode the
pixels into a buffer.
Note that you can more or less do this already, if you e.g. buffer the first 16K of a
reader, call image.DecodeConfig, and if you're happy with that, rewind the buffer and
call image.Decode.

which matches what @opennota said above:

For now one can use png.DecodeConfig to check if the width and height are reasonable before decoding the entire image.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
@cainiaocome @opennota @randall77 @orestonce @nigeltao @cherrymui @WangLeonard and others