Previously on Learning Go-Fuzz
:
This time I am looking at a different package. This is a package called goexif
at https://github.com/rwcarlsen/goexif. Being a file parser, it's a prime target for Go-Fuzz
. Unfortunately it has not been updated for a while. Instead, we will be looking at a fork at https://github.com/xor-gate/goexif2.
Code and fuzzing artifacts are at:
Steps are similar to the previous part.
go get github.com/xor-gate/goexif2/exif
go get github.com/xor-gate/goexif2/tiff
Fuzz.go
.go-fuzz-build
.go-fuzz-build github.com/xor-gate/goexif2/exif
If panics have been fixed, you can clone the commit e5a111b2b4bd00d5214b1030deb301780110358d
.
The Fuzz
function is easy straight forward:
// +build gofuzz
package exif
import "bytes"
func Fuzz(data []byte) int {
_, err := Decode(bytes.NewReader(data))
if err != nil {
return 0
}
return 1
}
For samples, we need some pictures that contain exif data. The package comes with some samples inside the samples
directory but I used samples at the following repository minus corrupted.jpg
:
During fuzzing I got a lot of crashes that were caused by lack of memory. This usually happens when random bytes are read as field sizes and the size is not evaluated, thus the package will allocate very large chunks of memory.
We are instrumenting the application around 10000 times a second, this adds up and the garbage collector cannot keep up. Soon we need to download more RAM
. You can see memory usage in the following picture:
Go's GC hard at work
Looking at the fuzzer, we can see our restarts
ratio is crap. This is the ratio of restarts to executions. We want it to be around 1/10000
but we have fallen to 1/1500
. This means we are crashing a lot. After a while, Go-Fuzz
might even stop working (see stagnating total number of execs in the picture below).
Go-Fuzz stops
Looking inside crash dumps, we see most of them are about running out of memory:
runtime: out of memory: cannot allocate 25769803776-byte block (25832882176 in use)
fatal error: out of memory
runtime stack:
runtime.throw(0x547da6, 0xd)
/go-fuzz-build214414686/goroot/src/runtime/panic.go:616 +0x88
runtime.largeAlloc(0x600000000, 0x440001, 0x5f8330)
/go-fuzz-build214414686/goroot/src/runtime/malloc.go:828 +0x117
runtime.mallocgc.func1()
/go-fuzz-build214414686/goroot/src/runtime/malloc.go:721 +0x4d
runtime.systemstack(0x0)
/go-fuzz-build214414686/goroot/src/runtime/asm_amd64.s:409 +0x7e
runtime.mstart()
/go-fuzz-build214414686/goroot/src/runtime/proc.go:1175
This means we are running out of memory and it's not a legitimate crash. Before continuing we need to go and investigate the root cause.
Lesson #0: Fix Go-Fuzz
running out of memory:
-procs
. By default Go-Fuzz
uses all of your CPU cores (including virtual).Let's look at our crashes.
05/03/2018 12:16 AM 365 171e8e5ca3e3d609322376915dcfa3dd56938845
05/03/2018 12:16 AM 3,651 171e8e5ca3e3d609322376915dcfa3dd56938845.output
05/03/2018 12:16 AM 912 171e8e5ca3e3d609322376915dcfa3dd56938845.quoted
05/01/2018 11:53 PM 186 3f5b7d448a0791f5739fa0a2371bb2492b64f835
05/01/2018 11:53 PM 1,928 3f5b7d448a0791f5739fa0a2371bb2492b64f835.output
05/01/2018 11:53 PM 312 3f5b7d448a0791f5739fa0a2371bb2492b64f835.quoted
05/01/2018 11:25 PM 114 49dfc363adbbe5aac9c2f8afbb0591c3ef1de2c3
05/01/2018 11:25 PM 1,383 49dfc363adbbe5aac9c2f8afbb0591c3ef1de2c3.output
05/01/2018 11:25 PM 186 49dfc363adbbe5aac9c2f8afbb0591c3ef1de2c3.quoted
05/01/2018 11:26 PM 22 a59a2ad5701156b88c6a132e1340fe006f67280c
05/01/2018 11:26 PM 1,677 a59a2ad5701156b88c6a132e1340fe006f67280c.output
05/01/2018 11:26 PM 63 a59a2ad5701156b88c6a132e1340fe006f67280c.quoted
As we know Go-Fuzz
conveniently stores the inputs in files. We can use the following code snippet to reproduce crashes:
|
|
These two panics are similar:
panic: runtime error: makeslice: len out of range
goroutine 1 [running]:
github.com/xor-gate/goexif2/tiff.(*Tag).convertVals(0xc04205a280, 0xc042080480, 0xc04200e090)
/go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/tiff/tag.go:258 +0x88c
github.com/xor-gate/goexif2/tiff.DecodeTag(0x30a0000, 0xc042080480, 0x5605c0, 0x613170, 0x514c20, 0xc04200e06c, 0x0)
/go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/tiff/tag.go:182 +0x623
github.com/xor-gate/goexif2/tiff.DecodeDir(0x30a0000, 0xc042080480, 0x5605c0, 0x613170, 0xc042080480, 0x0, 0x0, 0x0)
// removed
A5
crash payload is:
00000000 49 49 2a 00 08 00 00 00 30 30 30 30 05 00 00 00 |II*.....0000....|
00000010 00 a0 30 30 30 30 |. 0000|
The panic is happening at https://github.com/xor-gate/goexif2/blob/develop/tiff/tag.go#L258:
|
|
We can add some print statements to the local copy the package and investigate it:
|
|
Running test-crash-a5.go
we get the value:
$ go run test-crash-a5.go
t.count: 2684354560
panic: runtime error: makeslice: len out of range
goroutine 1 [running]:
github.com/xor-gate/goexif2/tiff.(*Tag).convertVals(0xc04205a1e0, 0xc042082018, 0xc042062110)
As you have noticed, the constant 2684354560
is more than the maximum of signed int32 (2147483647
). However, when trying to cast this value locally in Windows 10 64-bit VM or on the Go playground we get different results.
Consider this mini-example:
|
|
Running this in the Windows 10 64-bit VM, does not return an error. While running the same program in Go playground returns this error prog.go:8:11: constant 2684354560 overflows int32
.
This means the playground is using 32 bit int
s and locally we are using 64 bit ones. Local is obvious because we are in a 64 bit OS. To get the OS of the Go playground we can use this other small program:
|
|
And we get:
nacl
amd64p32
amd64p32
means it's a 64-bit OS using 32-bit pointers and int
s. We can use unsafe.Sizeof
to see this.
|
|
On Go playground we get:
Size of int : 4
Size of *int : 4
Size of *float32 : 4
But locally we get:
$ go run int-pointer-size.go
Size of int : 8
Size of int*: 8
Size of *float32 : 8
Note: Pointers are just memory addresses. It does not matter what they are pointing to. As you can see*float32
has the same size as a *int32
or *int64
.
Lesson #1: int
is OS dependent. It's better to use data types with explicit lengths like int32
and int64
. Also if you do not need negative numbers, use unsigned versions (but be careful of underflows).
Now let's get back to the crash. We are trying to create a large slice and the result is an error. We can trace back this error to slice.go in Go source:
|
|
_MaxMem
is calculated in malloc.go and it dictates how much memory can be allocated. On Windows 64-bit it seems to be 32GB or 35 bits.
Root cause analysis: We are allocating too much memory.
Lesson #2: Amount of memory available for malloc is OS dependent and somewhat arbitrary.
Lesson #3: Manually check size before allocating memory for slices.
But t.Count
has to come from somewhere.
t.Count
is calculated a bit further up at line 133.
|
|
We are reading 4 bytes (Count
is uint32
) and populating t.Count
. According to RFC2306 - Tag Image File Format (TIFF) - F Profile for Facsimile:
TIFF fields (also called entries) contain a tag, its type (e.g. short, long, rational, etc.), a count (which indicates the number of values/offsets) and a value/offset.
So we get 2684354560
when we read A0 00 00 00
from our payload in little-endian:
00000000 49 49 2a 00 08 00 00 00 30 30 30 30 05 00 00 00 |II*.....0000....|
00000010 00 a0 30 30 30 30 |. 0000|
Lesson #4: After reading data, check them for validity. This is more important for field lengths.
I could not find anything about the maximum number of types in a tag in the RFC. But it's a dword
(4 bytes) so it can contain values that cause the panic in makeslice
. We can choose a large enough value that does not cause the panic. I think 2147483647
or 1<<31-1
is a good compromise.
We can add our new check to the current check:
|
|
Now both crashes are avoided:
$ go run test-crash-a5.go
err: exif: decode failed (tiff: invalid Count offset in tag)
$ go run test-crash-3f.go
err: loading EXIF sub-IFD: exif: sub-IFD ExifIFDPointer decode failed: tiff: invalid Count offset in tag
This crash payload is:
00000000 4d 4d 00 2a 00 00 00 08 00 07 30 30 30 30 30 30 |MM.*......000000|
00000010 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |0000000000000000|
00000020 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |0000000000000000|
00000030 30 30 30 30 30 30 30 30 30 30 87 69 00 04 00 00 |0000000000.i....|
00000040 00 00 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |..00000000000000|
00000050 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |0000000000000000|
00000060 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 30 |0000000000000000|
00000070 30 30 |00|
And results in:
panic: runtime error: index out of range
goroutine 1 [running]:
github.com/xor-gate/goexif2/tiff.(*Tag).Int64(...)
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/tiff/tag.go:363
github.com/xor-gate/goexif2/exif.loadSubDir(0xc042080510, 0x547f15, 0xe, 0xc042080390, 0xc042080540, 0xc042089d68)
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/exif/exif.go:211 +0x704
github.com/xor-gate/goexif2/exif.(*parser).Parse(0x613170, 0xc042080510, 0xc0420804b0, 0x0)
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/exif/exif.go:190 +0x174
github.com/xor-gate/goexif2/exif.Decode(0x560240, 0xc042080480, 0x5ae92f8f, 0x212abedc, 0x1e9999)
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/exif/exif.go:331 +0x503
github.com/xor-gate/goexif2/exif.Fuzz(0x38f0000, 0x72, 0x200000, 0xc042047f48)
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/exif/Fuzz.go:8 +0xba
go-fuzz-dep.Main(0x550580)
go-fuzz-build214414686/goroot/src/go-fuzz-dep/main.go:49 +0xb4
main.main()
go-fuzz-build214414686/gopath/src/github.com/xor-gate/goexif2/exif/go.fuzz.main/main.go:10 +0x34
exit status 2
This can be reproduced by running test-crash-49.go
. At this point we know the drill. Looking at tag.go:363:
|
|
It's known that this method can panic. We need to modify it (and the other similar ones) to return an error instead.
The fix is straightforward. Before accessing t.intVals[i]
we need to check if the index is valid. This can be accomplished by checking it against len(t.intVals[i])
.
|
|
Lesson #5: Check index against array length before access.
Now we do not panic but there's no error because it's suppressed at exif.go:211:
|
|
The new error check needs to be added to these methods:
Rat2
Int64
Int
Float
A bit further down inside the MarshalJSON
method we can see errors being ignored:
|
|
Looking at the function we can see by ignoring the errors, we will have garbage data in the JSON. However, I don't think we need to return errors here but I could be wrong.
After things are fixed, we need to add the crashes to tests. This will discover if these bug regress in the future. Unfortunately, the package uses go generate
to generate tests and I have no clue how to use it. But I know how to write normal Go test using the testing package. Our payloads are pretty small so we will embed them in the test file instead of adding extra files to the package.
|
|
Lesson #6: Add Go-Fuzz
crashes to unit tests. This is useful for regression testing.
We used our newly acquired knowledge of Go-Fuzz
on an image parser.
Go-Fuzz
can crash when running out of memory and return false positives. We can throttle it or fix memory allocation bugs before resuming.int32
and int64
instead of OS dependent ones like int
.Go-Fuzz
crashes to unit tests.Thanks for reading and please let me know if you have feedback/tips/critiques.