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.

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

Panic in src/archive/mode.rs:163 #328

Closed
anfedotoff opened this issue Sep 7, 2022 · 7 comments
Closed

Panic in src/archive/mode.rs:163 #328

anfedotoff opened this issue Sep 7, 2022 · 7 comments

Comments

@anfedotoff
Copy link
Contributor

Hi!
We were doing some fuzzing with libFuzzer and our tool Sydr and we found some issues. Here is the panic message:
thread 'main' panicked at 'byte index 3 is not a char boundary; it is inside 'Г' (bytes 2..4) of 00Г00', library/core/src/str/mod.rs:127:5. The problem is because we try to build utf-8 str from non-utf8 characters here. The obvious fix is to use:

std::str::from_utf8(&buffer[header_offset..header_offset+SIZEOF_FILE_IDENTIFER])?

But goblin is std-free. Do you have any ideas?
Btw, we use goblin in our tools (sydr-fuzz, casr). It is very nice crate:).

@m4b
Copy link
Owner

m4b commented Sep 7, 2022

Thanks for fuzzing, I love that!

I’m a bit busy but I’ll begin triage on issues little later this week, however for particular case of from_utf8 this is available in core: https://doc.rust-lang.org/stable/core/str/fn.from_utf8.html

so that should allow your suggested fix to work in no std environment :)

@anfedotoff
Copy link
Contributor Author

Well, I Iooked. This function does validation of utf8 strings in std. We could put this code in src/archive/mode.rs and do validation before calling buffer.pread_with::<&str>. Another way we could do the same stuff as std::str::from_utf8 and std::str::from_utf8_unchecked for pread_with::<&str>.

I think, first way is more suitable. But also it would be great to put some docs for pread_with::<&str>, that raw data should be valid utf8 string.

@m4b
Copy link
Owner

m4b commented Sep 12, 2022

Actually taking a second look here, I'm a little bit confused what the issue is? Specifically, scroll::Pread on an &str uses this implementation:

https://github.com/m4b/scroll/blob/73c1d6764446626593f19422021445c21183698e/src/ctx.rs#L699-L705

(i.e., it calls ::from_ut8 for you).

Could you attach your full stack trace? There shouldn't be any panicking at all here, since the error should be propagated down to the callee (and the unwrap shouldn't be a utf8 error unwrap in core str, since we rewrap that error in scroll to BadInput)

@anfedotoff
Copy link
Contributor Author

Hmm, that's weird... Here is the stack trace

Running: /fuzz/parse-out-old/casr/cl1/crash-c4db1c20345db68536fb2ba80131565aa5aec496
thread '<unnamed>' panicked at 'byte index 3 is not a char boundary; it is inside 'Г' (bytes 2..4) of `00Г00`', library/core/src/str/mod.rs:127:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
==22== ERROR: libFuzzer: deadly signal
    #0 0x55845dc8f501 in __sanitizer_print_stack_trace /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_stack.cpp:87:3
    #1 0x55845e70176b in fuzzer::PrintStackTrace() /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerUtil.cpp:205:38
    #2 0x55845e7108b4 in fuzzer::Fuzzer::CrashCallback() /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerLoop.cpp:232:18
    #3 0x55845e710741 in fuzzer::Fuzzer::StaticCrashSignalCallback() /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerLoop.cpp:203:19
    #4 0x55845e70d810 in fuzzer::CrashHandler(int, siginfo_t*, void*) /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerUtilPosix.cpp:47:36
    #5 0x7fcb0671b41f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1441f) (BuildId: 7b4536f41cdaa5888408e82d0836e33dcf436466)
    #6 0x7fcb0640300a in __libc_signal_restore_set /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/internal-signals.h:86:3
    #7 0x7fcb0640300a in raise /build/glibc-SzIz7B/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:48:3
    #8 0x7fcb063e2858 in abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:79:7
    #9 0x55845e7eb2e6 in std::sys::unix::abort_internal::he27a37d61b2ed41a /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/sys/unix/mod.rs:293:14
    #10 0x55845dc018f6 in std::process::abort::hfcb96511de2eae1c /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/process.rs:2119:5
    #11 0x55845e6e0e6d in libfuzzer_sys::initialize::_$u7b$$u7b$closure$u7d$$u7d$::hb1f23152cc162412 /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/src/lib.rs:51:9
    #12 0x55845e7e04bc in std::panicking::rust_panic_with_hook::ha5fcab7510d2c291 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panicking.rs:702:17
    #13 0x55845e7e0316 in std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::h1916fdb5e93d55b3 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panicking.rs:588:13
    #14 0x55845e7dd53b in std::sys_common::backtrace::__rust_end_short_backtrace::h23fd3d7e6530fb89 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/sys_common/backtrace.rs:138:18
    #15 0x55845e7e0031 in rust_begin_unwind /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panicking.rs:584:5
    #16 0x55845dc02c62 in core::panicking::panic_fmt::he089491c0abfaeea /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/panicking.rs:142:14
    #17 0x55845e83be62 in core::str::slice_error_fail_rt::h37f8d59908af7379 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/str/mod.rs
    #18 0x55845e82a716 in core::ops::function::FnOnce::call_once::h72c09a1fe2ed9152 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/ops/function.rs:248:5
    #19 0x55845e8315b7 in core::intrinsics::const_eval_select::hb5abc46ce652acd2 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/intrinsics.rs:2464:5
    #20 0x55845dc02f81 in core::str::slice_error_fail::ha596d3ae8cee3562 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/str/mod.rs:86:9
    #21 0x55845e60bc44 in core::str::traits::_$LT$impl$u20$core..slice..index..SliceIndex$LT$str$GT$$u20$for$u20$core..ops..range..Range$LT$usize$GT$$GT$::index::h29e1bc9199fb006d /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/str/traits.rs:218:21
    #22 0x55845e040fe6 in core::str::traits::_$LT$impl$u20$core..ops..index..Index$LT$I$GT$$u20$for$u20$str$GT$::index::h85f0c1bba40c3dd2 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/core/src/str/traits.rs:65:9
    #23 0x55845df8a0c3 in goblin::archive::Member::bsd_filename_length::h3d026ce1adc9a4f2 /goblin/src/archive/mod.rs:163:31
    #24 0x55845df88b36 in goblin::archive::Member::parse::hea5d70659fa22985 /goblin/src/archive/mod.rs:127:43
    #25 0x55845df98fb5 in goblin::archive::Archive::parse::h271f09086753b3e0 /goblin/src/archive/mod.rs:448:26
    #26 0x55845dcc6c34 in goblin::Object::parse::hec6b646eadddeb13 /goblin/src/lib.rs:315:57
    #27 0x55845dcc1d70 in rust_fuzzer_test_input /goblin/fuzz/fuzz_targets/parse.rs:5:13
    #28 0x55845e6e0371 in libfuzzer_sys::test_input_wrap::_$u7b$$u7b$closure$u7d$$u7d$::h7c076fd3d96c24d4 /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/src/lib.rs:27:9
    #29 0x55845e7451cc in std::panicking::try::do_call::h3b98e83e8b3ec5d4 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panicking.rs:492:40
    #30 0x55845e74588a in __rust_try libfuzzer_sys.bcd972fd-cgu.0
    #31 0x55845e744990 in std::panicking::try::h4d8a46de216c30ae /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panicking.rs:456:19
    #32 0x55845e6e109a in std::panic::catch_unwind::he124faf0abc5f713 /rustc/7480389611f9d04bd34adf41a2b3029be4eb815e/library/std/src/panic.rs:137:14
    #33 0x55845e6df86c in LLVMFuzzerTestOneInput /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/src/lib.rs:25:22
    #34 0x55845e7124c0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerLoop.cpp:553:17
    #35 0x55845e6eabe1 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerDriver.cpp:292:21
    #36 0x55845e6ef22e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerDriver.cpp:775:19
    #37 0x55845e6e0e9c in main /root/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/35ce7d7/libfuzzer/FuzzerMain.cpp:19:30
    #38 0x7fcb063e4082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #39 0x55845dc0303d in _start (/parse+0x38b03d) (BuildId: f2fb18908339fea82456f60bc3b0c260147750a1)

I also, attach the input
crash-c4db1c20345db68536fb2ba80131565aa5aec496.txt for parse fuzz target.

@manunio
Copy link

manunio commented Sep 30, 2022

Hi @m4b while working on #334 found a similar panic.

crash-1999fa19815adfa202569abdb7fcf1e06b945fe3.txt

fn main() {
    let data = std::fs::read("./crash-1999fa19815adfa202569abdb7fcf1e06b945fe3.txt").unwrap();
    let _ = goblin::Object::parse(&data);
}
❯ RUST_BACKTRACE=full cargo run
   Compiling goblin-project v0.1.0 (/home/maxx/dev/security/oss-fuzz-projects/goblin-project)
    Finished dev [unoptimized + debuginfo] target(s) in 0.32s
     Running `target/debug/goblin-project`
thread 'main' panicked at 'byte index 3 is not a char boundary; it is inside '\u{317}' (bytes 2..4) of `        ̗cra<!`', /home/maxx/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.5.4/src/archive/mod.rs:163:31
stack backtrace:
   0:     0x560e6584ccf0 - std::backtrace_rs::backtrace::libunwind::trace::h4c56f7c1d2b54c49
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   1:     0x560e6584ccf0 - std::backtrace_rs::backtrace::trace_unsynchronized::h43647f7dfa7709b7
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x560e6584ccf0 - std::sys_common::backtrace::_print_fmt::hb05bf7e901883977
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x560e6584ccf0 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd3f800102e692f91
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x560e6586747e - core::fmt::write::h7e5f4e1d134bd366
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/fmt/mod.rs:1202:17
   5:     0x560e6584aad5 - std::io::Write::write_fmt::h51d5f9bde508a4b0
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/io/mod.rs:1679:15
   6:     0x560e6584e373 - std::sys_common::backtrace::_print::he94aa0f6090079de
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x560e6584e373 - std::sys_common::backtrace::print::h925dfd7226e7fc1e
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x560e6584e373 - std::panicking::default_hook::{{closure}}::hc105f3dae8d240c7
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:295:22
   9:     0x560e6584e05f - std::panicking::default_hook::h61df943bb9a1db3e
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:314:9
  10:     0x560e6584ea1a - std::panicking::rust_panic_with_hook::hba475cb83ede0ec8
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:698:17
  11:     0x560e6584e917 - std::panicking::begin_panic_handler::{{closure}}::h08829afeef0f745a
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:588:13
  12:     0x560e6584d19c - std::sys_common::backtrace::__rust_end_short_backtrace::hd1d5fe98bb9a45ac
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:138:18
  13:     0x560e6584e632 - rust_begin_unwind
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:584:5
  14:     0x560e65775923 - core::panicking::panic_fmt::hccec6d6a555033fa
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/panicking.rs:142:14
  15:     0x560e6586a8fa - core::str::slice_error_fail_rt::h22f8edb44d9c50f8
  16:     0x560e65775b27 - core::str::slice_error_fail::h192ff7ed5a9b3de2
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/str/mod.rs:86:9
  17:     0x560e657defe7 - core::str::traits::<impl core::slice::index::SliceIndex<str> for core::ops::range::Range<usize>>::index::hf5dac83dba10b1fa
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/str/traits.rs:218:21
  18:     0x560e65823927 - core::str::traits::<impl core::ops::index::Index<I> for str>::index::h2573f6e579343008
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/str/traits.rs:65:9
  19:     0x560e6581a000 - goblin::archive::Member::bsd_filename_length::h8ae44d6c748aba90
                               at /home/maxx/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.5.4/src/archive/mod.rs:163:31
  20:     0x560e65819b92 - goblin::archive::Member::parse::h12386e74474866f9
                               at /home/maxx/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.5.4/src/archive/mod.rs:127:43
  21:     0x560e6581d4c3 - goblin::archive::Archive::parse::h1878864d160d1502
                               at /home/maxx/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.5.4/src/archive/mod.rs:448:26
  22:     0x560e6577bc20 - goblin::Object::parse::h3bbfe6f2b27ebc5f
                               at /home/maxx/.cargo/registry/src/github.com-1ecc6299db9ec823/goblin-0.5.4/src/lib.rs:315:57
  23:     0x560e65775dd9 - goblin_project::main::h30f2f44422b9935e
                               at /home/maxx/dev/security/oss-fuzz-projects/goblin-project/src/main.rs:5:13
  24:     0x560e65775f9b - core::ops::function::FnOnce::call_once::he3ffb7dafcd1d7cf
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/ops/function.rs:248:5
  25:     0x560e65775f0e - std::sys_common::backtrace::__rust_begin_short_backtrace::haf4e71076ac61513
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/sys_common/backtrace.rs:122:18
  26:     0x560e657764e1 - std::rt::lang_start::{{closure}}::h4816c0e39ddeff2e
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/rt.rs:166:18
  27:     0x560e6584898f - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hacc01340c45ccc2f
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/core/src/ops/function.rs:283:13
  28:     0x560e6584898f - std::panicking::try::do_call::hd53f55a1eb2ccf65
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:492:40
  29:     0x560e6584898f - std::panicking::try::h648613e438b32859
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:456:19
  30:     0x560e6584898f - std::panic::catch_unwind::h2e8f23574d1cdcc7
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panic.rs:137:14
  31:     0x560e6584898f - std::rt::lang_start_internal::{{closure}}::h011a594072e31990
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/rt.rs:148:48
  32:     0x560e6584898f - std::panicking::try::do_call::h73a1517b82befe0c
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:492:40
  33:     0x560e6584898f - std::panicking::try::h1bfe4a971e4b9baa
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panicking.rs:456:19
  34:     0x560e6584898f - std::panic::catch_unwind::h5fb78debc8a68445
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/panic.rs:137:14
  35:     0x560e6584898f - std::rt::lang_start_internal::h3474e9f6d6242414
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/rt.rs:148:20
  36:     0x560e657764ba - std::rt::lang_start::he81ce6c0622632dd
                               at /rustc/98ad6a5519651af36e246c0335c964dd52c554ba/library/std/src/rt.rs:165:17
  37:     0x560e65775e41 - main
  38:     0x7f83a3224083 - __libc_start_main
                               at /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
  39:     0x560e65775b5e - _start
  40:                0x0 - <unknown>

@nathaniel-daniel
Copy link
Contributor

I think this is due to the line here directly indexing into the name str without previously validating the byte indexes. In some cases, this could split a multibyte codepoint, so it wouldn't be able to return a str and panics instead. I think this line could be replaced with

if name.len() > 3 && name.get(0..3) == Some("#1/") {

to work around the issue.

@m4b
Copy link
Owner

m4b commented Oct 9, 2022

Ok sounds good let’s do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants