Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
nbigaouette opened this issue Oct 19, 2021 · 6 comments
Open

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

nbigaouette opened this issue Oct 19, 2021 · 6 comments

Comments

@nbigaouette
Copy link

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!
@nagisa
Copy link
Member

nagisa commented Oct 19, 2021

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

@fitzgen fitzgen transferred this issue from rust-fuzz/cargo-fuzz Oct 19, 2021
@fitzgen
Copy link
Member

fitzgen commented Oct 19, 2021

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.

@Manishearth
Copy link
Member

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.

@nagisa
Copy link
Member

nagisa commented Oct 19, 2021

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.

@fitzgen
Copy link
Member

fitzgen commented Oct 19, 2021

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.

@Manishearth
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fitzgen @nbigaouette @nagisa @Manishearth