zstd.Decompress returns no error on invalid input

[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).

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

I got plenty of examples of this.

What did you expect to see?

An error. The commandline returns:

zstd.exe -d af046bc7b9294e2e85367bf8eb2bfa4de760bc06.zst
eb2bfa4de760bc06.zst : 0 MB...     eb2bfa4de760bc06.zst : Read error (39) : premature end

What did you see instead?

No error returned.

[Viq111]

This one is a difference between the zstd cli & this Go zstd library in how size hint is interpreted.

It looks like the payload you were able to generate has a size hint of 4 but the payload needs 20 bytes to decompress.

in the zstd cli, it bails at the error: https://github.com/facebook/zstd/blob/585b5a10390831c7878feae12701f143f80ec9b8/programs/fileio.c#L1708

in this Go zstd library, we retry up to 3 times with a bigger destination buffer: https://github.com/DataDog/zstd/blob/1.x/zstd.go#L135-L140 (so starts at 4 then 8 then 16 then 32, we are able to decode). This is a side effect of allowing the user to provide a destination buffer is to retry with reallocating for her (so I guess you can say library design decision here)

By adding some logging, you can see what's happening:

ᐅ go run tes.go
Size hint: 4
Failed with size 4, retrying with 8
Failed with size 8, retrying with 16
Failed with size 16, retrying with 32
error=%!s(<nil>), lengthData=94, length=20

To double check, using the python zstd bindings (another author):

>>> zstd.decompress(a)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
zstd.Error: Decompression error: Destination buffer is too small

Marking as won't fix because the behavior is expected in current design

[klauspost]

@Viq111 But the input is corrupted. I don't see how it can be by design that it returns no error when that happens?

Here is another example: a6f346dc6989c3dfc402a82599e1bc83c1f5f8b6.zst.gz

zstd.exe -d a6f346dc6989c3dfc402a82599e1bc83c1f5f8b6.zst
99e1bc83c1f5f8b6.zst : 0 MB... zstd: a6f346dc6989c3dfc402a82599e1bc83c1f5f8b6.zst: unsupported format

.. but your library returns nothing. It seems to happen when there is additional data on the input but it fails to decode the frame header.