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

Failed assertion when attempting to parse empty byte slice #7

Closed
frewsxcv opened this issue Dec 30, 2017 · 2 comments
Closed

Failed assertion when attempting to parse empty byte slice #7

frewsxcv opened this issue Dec 30, 2017 · 2 comments

Comments

@frewsxcv
Copy link
Contributor

Found with cargo-fuzz via rust-fuzz/targets#95

extern crate minidump;

use std::io::Cursor;

fn main() {
    let cursor = Cursor::new(b"");
    let _ = minidump::Minidump::read(cursor);
}
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `32`,
 right: `0`', /Users/corey/.cargo/git/checkouts/rust-minidump-836c4af26fbfd1e5/a4c7b38/src/iostuff.rs:28:4
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:68
             at src/libstd/sys_common/backtrace.rs:57
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:397
   4: std::panicking::begin_panic
             at src/libstd/panicking.rs:577
   5: std::panicking::begin_panic
             at src/libstd/panicking.rs:538
   6: std::panicking::try::do_call
             at src/libstd/panicking.rs:522
   7: minidump::iostuff::transmogrify
             at ./<panic macros>:7
   8: minidump::iostuff::read
             at /Users/corey/.cargo/git/checkouts/rust-minidump-836c4af26fbfd1e5/a4c7b38/src/iostuff.rs:40
   9: <minidump::minidump::Minidump<T>>::read
             at /Users/corey/.cargo/git/checkouts/rust-minidump-836c4af26fbfd1e5/a4c7b38/src/minidump.rs:1260
  10: foo::main
             at src/main.rs:7
  11: backtrace_vector_release
             at src/libpanic_unwind/lib.rs:101
  12: <std::thread::local::LocalKey<T>>::try_with
             at src/libstd/panicking.rs:459
             at src/libstd/rt.rs:58
  13: foo::main
@luser
Copy link
Collaborator

luser commented Jan 3, 2018

Interesting! This is an assertion failure, so it's not harmful, it's just not very nice. Minidump::read reads the minidump header here:
https://github.com/luser/rust-minidump/blob/a4c7b38c26fcaa90e4a86d9c0eb385d8ddbc7d3d/src/minidump.rs#L1260

which winds up calling this read method:
https://github.com/luser/rust-minidump/blob/a4c7b38c26fcaa90e4a86d9c0eb385d8ddbc7d3d/src/iostuff.rs#L37

which calls transmogrify, which has the failing assertion:
https://github.com/luser/rust-minidump/blob/a4c7b38c26fcaa90e4a86d9c0eb385d8ddbc7d3d/src/iostuff.rs#L28

...that code is not very good, certainly!

@luser
Copy link
Collaborator

luser commented Sep 18, 2018

I rewrote all the I/O to use the scroll crate on master and got rid of that awful transmogrify code. I changed the Minidump API slightly in the process so that Minidump::read now takes a Deref<[u8]>, so you can pass a Vec<u8> or &[u8] in there. I just added a test for this scenario and verified that it passes--it returns an error instead of panicing.

Thanks for the report!

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

2 participants