Decoder doesn't check output size before allocating

[klauspost]

Please answer these questions before submitting your issue. Thanks!

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

go version go1.12.1 windows/amd64

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

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\mfiles\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=e:\gopath\
set GOPROXY=
set GORACE=
set GOROOT=c:\go
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=e:\temp\wintemp\go-build152196250=/tmp/go-build -gno-record-gcc-switches

Package version: 809b919c325d7887bff7bd876162af73db53e878 (v.1.4.0 tag)

What did you do?

Fuzz test. Using ref, refErr := zstd.Decompress(nil, data).

It seems like the decompressor will attempt to allocate whatever value is returned from ZSTD_getDecompressedSize, meaning you can crash the decoder with a very small payload. The value is also cast from uint64 to int allowing negative values, which will panic: runtime error: makeslice: len out of range. Note that int can also be 32 bits.

Sample: a43e4d2eb6da97e23c23964ee90a093262f69564.zst.gz (gzipped to make github happy).

What did you expect to see?

No crash. The decoder should fall back to streaming over a certain size.

What did you see instead?

runtime: VirtualAlloc of 52983526064128 bytes failed with errno=1455
fatal error: out of memory

runtime stack:
runtime.throw(0x5c33c3, 0xd)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/panic.go:617 +0x79
runtime.sysMap(0xc000400000, 0x303030400000, 0x6ab778)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mem_windows.go:122 +0x138
runtime.(*mheap).sysAlloc(0x693a40, 0x303030304000, 0x693a50, 0x181818182)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/malloc.go:633 +0x1d4
runtime.(*mheap).grow(0x693a40, 0x181818182, 0x0)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mheap.go:1232 +0x49
runtime.(*mheap).allocSpanLocked(0x693a40, 0x181818182, 0x6ab788, 0x203000000000000)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mheap.go:1150 +0x3d1
runtime.(*mheap).alloc_m(0x693a40, 0x181818182, 0x303030300101, 0x10000)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mheap.go:977 +0xd0
runtime.(*mheap).alloc.func1()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mheap.go:1048 +0x53
runtime.(*mheap).alloc(0x693a40, 0x181818182, 0x10101, 0xc00008dce8)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/mheap.go:1047 +0x91
runtime.largeAlloc(0x303030303030, 0x101, 0x1cadd8)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/malloc.go:1055 +0xa0
runtime.mallocgc.func1()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/malloc.go:950 +0x4d
runtime.systemstack(0xff5fe0)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/asm_amd64.s:351 +0x6b
runtime.mstart()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/proc.go:1153

goroutine 1 [running]:
runtime.systemstack_switch()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/asm_amd64.s:311 fp=0xc00008dc68 sp=0xc00008dc60 pc=0x451b40
runtime.mallocgc(0x303030303030, 0x599e20, 0xc000004001, 0x4390000)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/malloc.go:949 +0x8b1 fp=0xc00008dd08 sp=0xc00008dc68 pc=0x40cb91
runtime.makeslice(0x599e20, 0x303030303030, 0x303030303030, 0x4ce327)
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/slice.go:49 +0x73 fp=0xc00008dd38 sp=0xc00008dd08 pc=0x43d853
github.com/DataDog/zstd.Decompress(0x0, 0x0, 0x0, 0x4390000, 0x12, 0x200000, 0x0, 0x0, 0x0, 0x0, ...)
	e:/temp/wintemp/go-fuzz-build588063767/gopath/src/github.com/DataDog/zstd/zstd.go:130 +0x44a fp=0xc00008de08 sp=0xc00008dd38 pc=0x4c3c9a
github.com/klauspost/compress/zstd.Fuzz(0x4390000, 0x12, 0x200000, 0x0)
	e:/gopath/src/github.com/klauspost/compress/zstd/fuzz_decompress.go:69 +0x2bb fp=0xc00008df10 sp=0xc00008de08 pc=0x4d541b
github.com/dvyukov/go-fuzz/go-fuzz-dep.Main(0x5cb4c8)
	e:/temp/wintemp/go-fuzz-build588063767/gopath/src/github.com/dvyukov/go-fuzz/go-fuzz-dep/main.go:54 +0xbd fp=0xc00008df80 sp=0xc00008df10 pc=0x4dacbd
main.main()
	e:/temp/wintemp/go-fuzz-build588063767/gopath/src/github.com/klauspost/compress/zstd/go.fuzz.main/main.go:10 +0x34 fp=0xc00008df98 sp=0xc00008df80 pc=0x4db554
runtime.main()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/proc.go:200 +0x20c fp=0xc00008dfe0 sp=0xc00008df98 pc=0x42e4ac
runtime.goexit()
	e:/temp/wintemp/go-fuzz-build588063767/goroot/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc00008dfe8 sp=0xc00008dfe0 pc=0x453cf1

goroutine 4 [runnable]:
github.com/klauspost/compress/zstd.(*blockDec).startDecoder(0xc000080000)
	e:/gopath/src/github.com/klauspost/compress/zstd/blockdec.go:186
created by github.com/klauspost/compress/zstd.newBlockDec
	e:/gopath/src/github.com/klauspost/compress/zstd/blockdec.go:105 +0x174
exit status 2

[klauspost]

I just confirmed this to still be the case with the current 1.x.

This makes it extremely trivial to craft a DOS package, where you can crash any application feeding a very small zstd payload.

[kevinconaway]

Thanks for reporting this. Would you help me understand the behavior of ZSTD_getDecompressedSize here?

For the file that you posted, is the value that it is returning (52983525027888) a valid size? How was the file generated? The genesis of the file of course isn't relevant but I'm not really familiar with the internals of the algorithm so I'd like to understand the context here a bit more

Thanks.

[klauspost]

@kevinconaway The input is from a fuzz test. The value is likely from FrameContentSize which is a header value indicating the expected frame size.

The problem is that you can just set this value to whatever and crash the decoder. You need some sanity validation of this value.