Commit dbc3bd9a authored by Kirill Smelkov's avatar Kirill Smelkov Committed by Kamil Kisiel

decoder: Fix DOS in BINSTRING decoding

BINSTRING opcode follows 4-byte length of the data and the data itself.

Upon seeing the BINSTRING len header we were preallocating destination
buffer with len as optimization (see 14aaa14f "decoder: Preallocate .buf
capacity when we know (approximate) size of output to it"). However this
way it is easy for a malicious pickle to specify

	BINSTRING biglength (4GB)

and no data, and then the goroutine that processes such input will
allocate 4GB immediately, which in turn might cause out-of-memory DOS.

The other places where we currently grow Decoder.buf all either have 1
or 2 byte length (thus limited to 64K), or the length of the data that
was already read from input stream.

The problem was found by rerunning fuzz tests:

	"T0000"

fatal error: runtime: out of memory

runtime stack:
runtime.throw(0x514fff, 0x16)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/panic.go:608 +0x72
runtime.sysMap(0xc004000000, 0x34000000, 0x5f83d8)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mem_linux.go:156 +0xc7
runtime.(*mheap).sysAlloc(0x5dfd40, 0x34000000, 0x43c0e3, 0x7ffc94fe56b8)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/malloc.go:619 +0x1c7
runtime.(*mheap).grow(0x5dfd40, 0x18182, 0x0)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mheap.go:920 +0x42
runtime.(*mheap).allocSpanLocked(0x5dfd40, 0x18182, 0x5f83e8, 0x203000)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mheap.go:848 +0x337
runtime.(*mheap).alloc_m(0x5dfd40, 0x18182, 0x7ffc94fe0101, 0x40a87f)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mheap.go:692 +0x119
runtime.(*mheap).alloc.func1()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mheap.go:759 +0x4c
runtime.(*mheap).alloc(0x5dfd40, 0x18182, 0x7ffc94010101, 0x4135f5)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/mheap.go:758 +0x8a
runtime.largeAlloc(0x30303030, 0x440101, 0xc00005e240)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/malloc.go:1019 +0x97
runtime.mallocgc.func1()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/malloc.go:914 +0x46
runtime.systemstack(0x44eea9)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/asm_amd64.s:351 +0x66
runtime.mstart()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/proc.go:1229

goroutine 1 [running]:
runtime.systemstack_switch()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/asm_amd64.s:311 fp=0xc000034310 sp=0xc000034308 pc=0x44efa0
runtime.mallocgc(0x30303030, 0x4edc00, 0x1, 0xc0000343e8)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/malloc.go:913 +0x896 fp=0xc0000343b0 sp=0xc000034310 pc=0x40ae26
runtime.makeslice(0x4edc00, 0x30303030, 0x30303030, 0xc000018108, 0x4, 0x4)
        /tmp/go-fuzz-build515164548/goroot/src/runtime/slice.go:70 +0x77 fp=0xc0000343e0 sp=0xc0000343b0 pc=0x43b1d7
bytes.makeSlice(0x30303030, 0x0, 0x0, 0x0)
        /tmp/go-fuzz-build515164548/goroot/src/bytes/buffer.go:231 +0x9d fp=0xc000034420 sp=0xc0000343e0 pc=0x4b204d
bytes.(*Buffer).grow(0xc000084030, 0x30303030, 0x0)
        /tmp/go-fuzz-build515164548/goroot/src/bytes/buffer.go:144 +0x2e4 fp=0xc000034470 sp=0xc000034420 pc=0x4b1604
bytes.(*Buffer).Grow(0xc000084030, 0x30303030)
        /tmp/go-fuzz-build515164548/goroot/src/bytes/buffer.go:163 +0x86 fp=0xc000034498 sp=0xc000034470 pc=0x4b18b6
github.com/kisielk/og-rek.(*Decoder).loadBinString(0xc000084000, 0x203054, 0x0)
        /tmp/go-fuzz-build515164548/gopath/src/github.com/kisielk/og-rek/ogorek.go:646 +0x143 fp=0xc000034520 sp=0xc000034498 pc=0x4d5103
github.com/kisielk/og-rek.(*Decoder).Decode(0xc000084000, 0xc000080000, 0xc000084000, 0xf7eb9fa, 0x586dc)
        /tmp/go-fuzz-build515164548/gopath/src/github.com/kisielk/og-rek/ogorek.go:227 +0xf94 fp=0xc0000346d8 sp=0xc000034520 pc=0x4d09b4
github.com/kisielk/og-rek.Fuzz(0x7fbccd400000, 0x5, 0x200000, 0xc000034748)
        /tmp/go-fuzz-build515164548/gopath/src/github.com/kisielk/og-rek/fuzz.go:12 +0xb0 fp=0xc000034710 sp=0xc0000346d8 pc=0x4cf640
go-fuzz-dep.Main(0x519cf0)
        /tmp/go-fuzz-build515164548/goroot/src/go-fuzz-dep/main.go:49 +0xad fp=0xc000034780 sp=0xc000034710 pc=0x4642fd
main.main()
        /tmp/go-fuzz-build515164548/gopath/src/github.com/kisielk/og-rek/go.fuzz.main/main.go:10 +0x2d fp=0xc000034798 sp=0xc000034780 pc=0x4db1ad
runtime.main()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/proc.go:201 +0x207 fp=0xc0000347e0 sp=0xc000034798 pc=0x428ec7
runtime.goexit()
        /tmp/go-fuzz-build515164548/goroot/src/runtime/asm_amd64.s:1333 +0x1 fp=0xc0000347e8 sp=0xc0000347e0 pc=0x450f01
exit status 2
parent e7d96969
...@@ -643,7 +643,12 @@ func (d *Decoder) loadBinString() error { ...@@ -643,7 +643,12 @@ func (d *Decoder) loadBinString() error {
v := binary.LittleEndian.Uint32(b[:]) v := binary.LittleEndian.Uint32(b[:])
d.buf.Reset() d.buf.Reset()
d.buf.Grow(int(v)) // don't allow malicious `BINSTRING <bigsize> nodata` to make us out of memory
prealloc := int(v)
if maxgrow := 0x10000; prealloc > maxgrow {
prealloc = maxgrow
}
d.buf.Grow(prealloc)
_, err = io.CopyN(&d.buf, d.r, int64(v)) _, err = io.CopyN(&d.buf, d.r, int64(v))
if err != nil { if err != nil {
return err return err
......
...@@ -557,6 +557,10 @@ func TestDecodeError(t *testing.T) { ...@@ -557,6 +557,10 @@ func TestDecodeError(t *testing.T) {
// invalid protocol version // invalid protocol version
"\x80\xffI1\n.", "\x80\xffI1\n.",
// BINSTRING with big len and no data
// (might cause out-of-memory DOS if buffer is preallocated blindly)
"T\xff\xff\xff\xff.",
} }
for _, tt := range testv { for _, tt := range testv {
buf := bytes.NewBufferString(tt) buf := bytes.NewBufferString(tt)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment