x/image/webp: excessive memory consumption

[dvyukov]

The following program allocates 3GB+ while input data is 36 bytes and image size is small.

package main

import (
    "golang.org/x/image/webp"
    "strings"
)

func main() {
    data := "RIFF0000WEBPVP8 00\xef\xbf\xbd\x01\x00\x14\x00\x00\xb24\n\x9d\x01*\x96\x00g\x00"
    cfg, err := webp.DecodeConfig(strings.NewReader(data))
    if err != nil {
        return
    }
    if cfg.Width*cfg.Height > 1e6 {
        return
    }
    if _, err := webp.Decode(strings.NewReader(data)); err != nil {
        return
    }
}

with ulimit -v 1000000 it crashes as:

fatal error: runtime: out of memory

goroutine 1 [running]:
runtime.systemstack_switch()
    src/runtime/asm_amd64.s:216 fp=0xc20803f918 sp=0xc20803f910
runtime.mallocgc(0xbfef3020, 0x4ad900, 0x1, 0xd)
    src/runtime/malloc.go:629 +0x925 fp=0xc20803f9e8 sp=0xc20803f918
runtime.newarray(0x4ad900, 0xbfef3020, 0x0)
    src/runtime/malloc.go:745 +0xcc fp=0xc20803fa28 sp=0xc20803f9e8
runtime.makeslice(0x4a5340, 0xbfef3020, 0xbfef3020, 0x0, 0x0, 0x0)
    src/runtime/slice.go:32 +0x168 fp=0xc20803fa78 sp=0xc20803fa28
golang.org/x/image/vp8.(*Decoder).parseOtherPartitions(0xc20806e000, 0x0, 0x0)
    src/golang.org/x/image/vp8/decode.go:273 +0x5e fp=0xc20803fb70 sp=0xc20803fa78
golang.org/x/image/vp8.(*Decoder).parseOtherHeaders(0xc20806e000, 0x0, 0x0)
    src/golang.org/x/image/vp8/decode.go:311 +0x1f0 fp=0xc20803fc28 sp=0xc20803fb70
golang.org/x/image/vp8.(*Decoder).DecodeFrame(0xc20806e000, 0xd08010600, 0x0, 0x0)
    src/golang.org/x/image/vp8/decode.go:338 +0x4e fp=0xc20803fca8 sp=0xc20803fc28
golang.org/x/image/webp.decode(0x7f541f68b1c0, 0xc208016420, 0x401f00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    src/golang.org/x/image/webp/decode.go:96 +0x8d5 fp=0xc20803fe58 sp=0xc20803fca8
golang.org/x/image/webp.Decode(0x7f541f68b1c0, 0xc208016420, 0x0, 0x0, 0x0, 0x0)
    src/golang.org/x/image/webp/decode.go:259 +0x55 fp=0xc20803feb8 sp=0xc20803fe58
main.main()
    webp.go:19 +0x233 fp=0xc20803ff90 sp=0xc20803feb8

The allocation request is for 0xbfef3020 (3220123680) bytes.

on commit c5f9292598396797bde21d35a38a3da25f561f81

[syst3mw0rm]

I am not able to replicate the issue. I'm not sure if I'm setting the ulimit correctly though. I run ulimit -v 1000000 as mentioned. ulimit -a output:

-t: cpu time (seconds)              unlimited
-f: file size (blocks)              unlimited
-d: data seg size (kbytes)          unlimited
-s: stack size (kbytes)             8192
-c: core file size (blocks)         0
-v: address space (kbytes)          976
-l: locked-in-memory size (kbytes)  unlimited
-u: processes                       709
-n: file descriptors                2560

After that, go run webp.go seems to work fine. Am I doing anything wrong?

[dvyukov]

Just tried again and the issue reproduces on my machine.

My environment is:
go version devel +71bf182 Tue May 12 04:02:25 2015 +0000 linux/amd64
x/image is on commit c5f9292598396797bde21d35a38a3da25f561f81
$ ulimit -v
1000000

Are you using a 64-bit OS?
I am not sure why ulimit -v says 976 on your machine. You asked for 10000000, and 976 kb is not enough even to start a Go program. Anyway, try to set ulimit -v to a smaller value.

[syst3mw0rm]

go version devel +71bf182 Tue May 12 04:02:25 2015 +0000 darwin/amd64
x/image is pointing to master - c5f9292598396797bde21d35a38a3da25f561f81

Yep, I'm using 64-bit OS.
Darwin Aamirs-MacBook-Pro.local 14.3.0 Darwin Kernel Version 14.3.0: Mon Mar 23 11:59:05 PDT 2015; root:xnu-2782.20.48~5/RELEASE_X86_64 x86_64 i386 MacBookPro11,1 Darwin

Setting lower values for ulimit -v doesn't help.

[dvyukov]

OK, probably ulimit does not work on darwin. Modify:
golang.org/x/image/vp8.(*Decoder).parseOtherPartitions(0xc20806e000, 0x0, 0x0)
src/golang.org/x/image/vp8/decode.go:273 +0x5e fp=0xc20803fb70 sp=0xc20803fa78

to print amount of bytes it allocates and/or panic if it allocates more than 1e8.

[syst3mw0rm]

yeah, seems like it. It certainly allocates a lot of memory - 3220123680

[syst3mw0rm]

I looked into it. The first 4 bytes after VP8 is chunkLen. In this case, it is [48 48 239 191] in little-endian form which actually translates to a huge number - 3220123696.

The file gives EOF error when decoding anyway. I'd just say it is a corrupt file and behaviour is expected.

[dvyukov]

The image is small and the input is small and that part is not compressed. The code should not allocate gigs of memory in such case. Otherwise it is useless for decoding of real world images.

Since there is no way to get the total input size, the code should allocate and read in, say, 1MB; if that does not EOF, allocate 2MB, copy and read in another meg, etc.

[nigeltao]

What I wrote on at the end of #10399 more or less applies here, after s/BMP/WEBP/:

---

The structure of the file format is first header, then payload. The image dimensions are in the header, and the decoder allocates a pixel buffer after the header is processed but before the payload is processed, because processing the payload involves writing the pixels to somewhere: the pixel buffer.

Decode takes an io.Reader, not an io.ReadSeeker, so at the time it allocates the pixel buffer, it cannot know whether or not there's 'enough input' remaining. Conversely, we may be decoding an image downloaded from a socket, where we cannot know how much input is remaining. So I think this is all WAI.

For decoding images in general, with compression, knowing the total file size doesn't necessarily mean that you know whether or not there's enough pixel data. LZW-style length-difference or RLE encoding means that very large uncompressed data can compress very, very well.

That the various image libraries should reject an 16x1073741836 early, checking some sort of MaxMem option before allocating a buffer, is issue #5050.

---

Reading 1MB, 2MB, etc. isn't the right fix. That would mean unnecessary re-allocation and copying of large slices as the image was filled in, which I suspect will have a measurable performance impact, as well as complicate the code. Even if you did that, RLE compression can compress uniform-color input really, really well, so all this staggered memory approach might do is make a bunch of smaller allocations on the way to OOM, when the input is small in bytes but large (and otherwise valid) in pixels.

Again, this is WAI. I appreciate that you're trying to improve the image packages by fuzzing inputs, but until issue #5050 is fixed (which won't be at least until the tree is open for 1.6), the false positive (like this issue and issue #10399) to true positive ratio is possibly becoming too high.

If you want to continue fuzzing, "comment 1" on issue #5050 has an approach to avoid "image too large" false positives: buffer the reader, call DecodeConfig, check the image width and height, and conditionally rewind the buffer and call Decode.

[nigeltao]

Ah, I spoke to early, and misread the earlier issue comments. This isn't a pixel buffer, this is a separate buffer. I'll fix this.

[nigeltao]

Apologies for mischaracterizing the bug, BTW.