libfuzzer icon indicating copy to clipboard operation
libfuzzer copied to clipboard

Using 'catch_unwind' to convert panic into error is still detected as crash

Open nbigaouette opened this issue 2 years ago • 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!

nbigaouette avatar Oct 19 '21 18:10 nbigaouette

This appears to be related to -Cpanic=abort panic strategy.

nagisa avatar Oct 19 '21 18:10 nagisa

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.

fitzgen avatar Oct 19 '21 20:10 fitzgen

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.

Manishearth avatar Oct 19 '21 20:10 Manishearth

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.

nagisa avatar Oct 19 '21 20:10 nagisa

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.

fitzgen avatar Oct 19 '21 20:10 fitzgen

Yeah I think the solution here is to have an on by default option

Manishearth avatar Oct 19 '21 20:10 Manishearth