Using 'catch_unwind' to convert panic into error is still detected as crash
nbigaouette opened this issue · 6 comments
I wrap a function that I know is buggy into a std::panic::catch_unwind() so that when it panics my own code does not. I can then gracefully handle the error.
Unfortunately, libfuzzer still detects this as an error and stops.
Here's an example fuzzing target:
#![no_main]
use libfuzzer_sys::fuzz_target;
fn may_panic(input: &[u8]) -> u8 {
let value = *input.get(0).unwrap_or(&0);
if value > 240 {
panic!("Value is greater than 240! value={}", value);
}
value
}
fn wrapper(input: &[u8]) -> Option<u8> {
match std::panic::catch_unwind(|| may_panic(input)) {
Ok(value) => Some(value),
Err(err) => {
println!("Caught panic, returning None");
None
}
}
}
fuzz_target!(|data: &[u8]| {
wrapper(data);
});
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2547700486
INFO: Loaded 1 modules (743 inline 8-bit counters): 743 [0x10946a9c0, 0x10946aca7),
INFO: Loaded 1 PC tables (743 PCs): 743 [0x10946aca8,0x10946db18),
fuzz/target/x86_64-apple-darwin/release/issue: Running 1 inputs 1 time(s) each.
Running: fuzz/artifacts/issue/minimized-from-ac85e832d98dd32e68baaafa21973abcb7a73101
thread '<unnamed>' panicked at 'Value is greater than 240! value=243', fuzz_targets/issue.rs:7:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
==26058== ERROR: libFuzzer: deadly signal
#0 0x109605b15 in __sanitizer_print_stack_trace+0x35 (librustc-nightly_rt.asan.dylib:x86_64+0x4fb15)
rust-fuzz/cargo-fuzz#1 0x10934b14a in fuzzer::PrintStackTrace()+0x2a (issue:x86_64+0x10001d14a)
rust-fuzz/cargo-fuzz#2 0x10933d093 in fuzzer::Fuzzer::CrashCallback()+0x43 (issue:x86_64+0x10000f093)
rust-fuzz/cargo-fuzz#3 0x7fff734065fc in _sigtramp+0x1c (libsystem_platform.dylib:x86_64+0x35fc)
rust-fuzz/cargo-fuzz#4 0x10946db1f (<unknown module>)
rust-fuzz/cargo-fuzz#5 0x7fff732dc807 in abort+0x77 (libsystem_c.dylib:x86_64+0x7f807)
rust-fuzz/cargo-fuzz#6 0x1093c9528 in std::sys::unix::abort_internal::h70ee130d4bec8a65+0x8 (issue:x86_64+0x10009b528)
rust-fuzz/cargo-fuzz#7 0x10942df08 in std::process::abort::h8a4caf764e23a06d+0x8 (issue:x86_64+0x1000fff08)
rust-fuzz/cargo-fuzz#8 0x10933bf74 in libfuzzer_sys::initialize::_$u7b$$u7b$closure$u7d$$u7d$::h09a768b82a6aeb19+0x54 (issue:x86_64+0x10000df74)
rust-fuzz/cargo-fuzz#9 0x1093bdb65 in std::panicking::rust_panic_with_hook::h2f2e429268b7f845+0x255 (issue:x86_64+0x10008fb65)
rust-fuzz/cargo-fuzz#10 0x1093bd5dd in std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::he71680fea17513cb+0x8d (issue:x86_64+0x10008f5dd)
rust-fuzz/cargo-fuzz#11 0x1093ba376 in std::sys_common::backtrace::__rust_end_short_backtrace::h32d0456e90e0ce79+0x16 (issue:x86_64+0x10008c376)
rust-fuzz/cargo-fuzz#12 0x1093bd549 in rust_begin_unwind+0x49 (issue:x86_64+0x10008f549)
rust-fuzz/cargo-fuzz#13 0x10942e39a in std::panicking::begin_panic_fmt::hf8e6c99ec321f361+0x3a (issue:x86_64+0x10010039a)
rust-fuzz/cargo-fuzz#14 0x10933277b in std::panicking::try::do_call::hcdaafa6b928860c7+0x3ab (issue:x86_64+0x10000477b)
rust-fuzz/cargo-fuzz#15 0x109337eb3 in __rust_try+0x13 (issue:x86_64+0x100009eb3)
rust-fuzz/cargo-fuzz#16 0x109336da8 in issue::wrapper::h9ac1378f8c7f7847+0x128 (issue:x86_64+0x100008da8)
rust-fuzz/cargo-fuzz#17 0x109337a5f in rust_fuzzer_test_input+0x49f (issue:x86_64+0x100009a5f)
rust-fuzz/cargo-fuzz#18 0x10933c083 in __rust_try+0x13 (issue:x86_64+0x10000e083)
rust-fuzz/cargo-fuzz#19 0x10933b7d1 in LLVMFuzzerTestOneInput+0x131 (issue:x86_64+0x10000d7d1)
rust-fuzz/cargo-fuzz#20 0x10933edd1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)+0x131 (issue:x86_64+0x100010dd1)
rust-fuzz/cargo-fuzz#21 0x10935c315 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long)+0xe5 (issue:x86_64+0x10002e315)
rust-fuzz/cargo-fuzz#22 0x1093625fd in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))+0x1edd (issue:x86_64+0x1000345fd)
rust-fuzz/cargo-fuzz#23 0x109371b42 in main+0x22 (issue:x86_64+0x100043b42)
rust-fuzz/cargo-fuzz#24 0x7fff7320dcc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)
NOTE: libFuzzer has rudimentary signal handlers.
Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal
────────────────────────────────────────────────────────────────────────────────
Error: Fuzz target exited with exit code: 77
Where the file fuzz/artifacts/issue/minimized-from-ac85e832d98dd32e68baaafa21973abcb7a73101 contains a single byte: 0xF3 (243).
Can we instruct libfuzzer to not bother with these kinds of errors?
Thanks!
This appears to be related to -Cpanic=abort panic strategy.
I'm not sure why we went with the -Cpanic=abort approach; that was already in place before I started hacking on any of this stuff. Maybe someone else who remembers why it is like this can share?
It seems like using catch_unwind inside the fuzz_target! macro's expanded code would be more flexible and allow for users to catch+handle irrelevant panics inside the fuzz target.
AIUI the fuzzer works better if panics lead to immediate aborts since there's less logic to trace. But I'm not quite sure if there is a major difference.
I recall it being much better in a sense that it gives libfuzzer an opportunity to see unique crashes easier. Or at least that was the case back when I first MVPd the thing.
I recall it being much better in a sense that it gives libfuzzer an opportunity to see unique crashes easier. Or at least that was the case back when I first MVPd the thing.
This makes a ton of sense, since tools like oss-fuzz will ~essentially use only the crashing stack trace to differentiate bugs, AFAIK.
I wonder if we can make -Cpanic=abort a CLI flag in cargo fuzz? Would require some cooperation in libfuzzer-sys too for whether to set the panic hook or not, probably with env vars like our other points of cooperation.
Yeah I think the solution here is to have an on by default option