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.

Sign up for GitHub

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

Jump to bottom

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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@orestonce
Copy link

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
Contributor

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")?

@randall77
Copy link
Contributor

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.

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

/cc @golang/security

@ghost
Copy link

ghost 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.

@orestonce
Copy link
Author

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.

@orestonce
Copy link
Author

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.

@ghost
Copy link

ghost commented Apr 23, 2021

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

@ghost
Copy link

ghost commented Apr 23, 2021 •
edited by ghost

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.

@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
Member

cc @nigeltao

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 26, 2021
@nigeltao
Copy link
Contributor

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

@nigeltao
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@cainiaocome @randall77 @orestonce @nigeltao @cherrymui @WangLeonard and others