Closed ruuda closed 4 years ago
Thanks for this I've been meaning to do this for a while now. See my reddit comment for an explanation on why I don't consider that specific case a bug.
This will still detect crashes when Address Sanitizer reports an invalid memory access. But that is desired, right? Because if Rust did not prevent that, it can still be dangerous, even when catching panics.
Base64-encoded file that causes an Address Sanitizer crash:
TU0AKgAAAAgAGQD+AAQAAAABAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAEAAAQAAAABAAAAoAEBAAQAAAABAAAAeAECAAMAAAADAAABPAEDAAMAAAABAAEAAAEGAAMA
AAABAAIAAAEPAAIAAAASAAABRAEQAAIAAAAMAAABWAERAAQAAAABAAHo4AESAAMAAAABAAEAAAEV
AAMAAAABAAMAAAEWAAQAAAABAAAAeAEXAAQAAAABAADhAAEaAAUAAAABAAABZAEbAAUAAAABAAAB
bAEcAAMAAAABAAEAAAEoAANNTSoAAAAACAAAAACgAQEAAAABAAIAAAExAAIAAAAKAAABdAEyAAIA
AAAUAAABAIABSgAEAAAAAgAAAZQCFAAFAAAABgAAAZyHaQAEAAAAAQAAAeCIJQAEAAA6BAAAAAB4
AQIAAAEAAejOkAMAAgAAABQAAAHMkhYAAQAAAAQBAAAAAAAAAAAAAAgACAAIAABOSUtPTiBDT1JQ
T1JBVElPTgAAAE5JS09OIEQ1MDAwAAAAASwAAAABAAABLAAAAAFWZXIuMS4wMCAAAAAyMDE1OjA5
OjA1IDE1OjU1OjI4AAACyeAAAspYAAAAAAAAAAEAAAD/AAAAAQADAAAAAwAAATwBAwADAAAAAAAA
AAABAAAA/wAAAAEAAAAAAAAAAQAAAP8AAAABMjAxNTowOTowNSA=
The error happens on tiff.rs:459
. The message is
==23461==ERROR: AddressSanitizer: SEGV on unknown address 0x100089401617 (pc 0x564bbd0ac849 bp 0x7fff4a04b210 sp 0x7fff4a04b0a0 T0)
==23461==The signal is caused by a READ memory access.
Not sure how we can end up with an AS trigger. It's probably an out-of-bounds access that rust catches being caught by AS first, no? Can you run the file under the normal rawloader binary and see what it does? I just did and got no SEGV, not even a panic. So there's something strange here.
The reason the normal program doesn't panic/crash is probably that because it adds zeroes at the end that call eventually finds one of those. Still don't know why that's an AS and not panic though.
The normal binary does not crash, but that is no surprise; asan manipulates allocations to detect these errors more easily. The topmost stackframe reported is slice/mod.rs:1200
, which calls search_while
, but it looks ok to me. It does pointer comparisons in unsafe code, so there is nothing that would cause a panic there. It looks like data
is always a slice of buf
. Where does buf
come from? Could it include invalid memory?
After some questions on #rust my best guess is that the synthetic file has an impossibly large tag that is somehow causing an invalid stack allocation somewhere. Those can indeed cause rust to SEGV. But maybe this is actually a bug in core rust?
Does Rust even have runtime-sized stack allocations? The alloca RFC was closed.
You can definitely blow the stack by allocating a lot but maybe that specific case I was mentioning is not possible. For the purpose of this PR I'd say we should disable AS if possible. At least for now I'd focus on cases that hang or crash the actual normal binary.
Here’s a file that takes about a second to decode when compiled in release mode, even though it is only 1245 bytes:
TU0AKgAAAAgAGQAAAAAAAAD///////////////8AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB0dHR0dHR0dHR0d
HR0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABNTQAqKgAAAAgAGQAA
AAAAAAD///////////////8AAAAAAAAAAAAAAAAAAAAAAP4ABAAAAAE6AAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAQFNTQAqAAAACAAZAP4ABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAB0dHR0dHR0dHR0dHR0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABNTQAqKgAAAAgAGQAAAAAAAAD///////////////8AAAAAAAAAAHIABAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AToAAAABAU1NACoAAAAIABkA/gAEAAAAAQAAAAEBAAAEAAAAAAAAAQEAAAQAAAABAAAAoAEBOzoE
AACsAAB4AQIDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACkAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAB0dHR0dHR0dHR0dHR0AAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAABNTQAqKgAAAAgAGQAAAAAAAAD///////////////8AAAAAAAAA
AAAAAAAAAAAAAP4ABAAAAAE6AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQFNTQAqAAAA
CAAZAP4ABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB0dHR0dHR0d
HR0dHR0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABNTQAqKgAAAAgA
GQAAAAAAAAD///////////////8AAAAAAAAAAAAAAAAAAAAAAP4ABAAAAAE6AAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAQFNTQAqAAAACAAZAP4ABAAAAAEAAAABAAEAAAQA
I've since added two fuzzing targets (one over metadata and another over the decoders) and "completed" several fuzzing sessions fixing a few bugs. On the way I also had to fix some bugs in afl-fuzz :) Thanks for pushing me to do this.
This pull request adds a cargo-fuzz fuzzing target. I added a readme that explains how to run the fuzzer. This requires a nightly toolchain, and it currently only works on x86-64 Linux and x86-64 Mac.
When seeded with a NEF file, it almost immediately finds an index out of bounds crash. The base64-encoded input that triggers this crash is
TU0AKgAAAAgAGQD+AAQAAA==
.