Error found on fuzzing

[stusmall]

I gave this project and quick fuzz and found it panics with following test case

extern crate swf_parser;
#[test]
fn test(){
    let bytes = b"CWSCCCACCGCCC";
    let _ = swf_parser::parsers::movie::parse_movie(&bytes[..]);
}

It causes the following panic:

---- test stdout ----
thread 'test' panicked at 'called Result::unwrap() on an Err value: Custom { kind: Other, error: StringError("unknown ZLIB method CM=0x3") }', src/libcore/result.rs:997:5
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.
stack backtrace:
0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
1: std::sys_common::backtrace::_print
at src/libstd/sys_common/backtrace.rs:71
2: std::panicking::default_hook::{{closure}}
at src/libstd/sys_common/backtrace.rs:59
at src/libstd/panicking.rs:197
3: std::panicking::default_hook
at src/libstd/panicking.rs:208
4: std::panicking::rust_panic_with_hook
at src/libstd/panicking.rs:474
5: std::panicking::continue_panic_fmt
at src/libstd/panicking.rs:381
6: rust_begin_unwind
at src/libstd/panicking.rs:308
7: core::panicking::panic_fmt
at src/libcore/panicking.rs:85
8: core::result::unwrap_failed
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libcore/macros.rs:16
9: <core::result::Result<T, E>>::unwrap
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libcore/result.rs:798
10: swf_parser::parsers::movie::parse_movie
at src/parsers/movie.rs:49
11: test::test
at tests/test.rs:6
12: test::test::{{closure}}
at tests/test.rs:4
13: core::ops::function::FnOnce::call_once
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libcore/ops/function.rs:231
14: <F as alloc::boxed::FnBox>::call_box
at src/libtest/lib.rs:1513
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libcore/ops/function.rs:231
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/liballoc/boxed.rs:749
15: __rust_maybe_catch_panic
at src/libpanic_unwind/lib.rs:87
16: test::run_test::run_test_inner::{{closure}}
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libstd/panicking.rs:272
at /rustc/f22dca0a1bef4141e75326caacc3cd59f3d5be8e/src/libstd/panic.rs:388
at src/libtest/lib.rs:1468

[demurgos]

Thanks for checking.
The parser is not resilient to invalid input at the moment. It won't read corrupted data, but it will panick and crash.

Making it more resilient would be great: could you share how you fuzzed the parser? It would be nice to add it to the tests.

The specific error is caused by the fact that the CWS signature indicates a zlib-encoded payload, but the rest of the input can't be understood by the decompression library.

[stusmall]

Of course. I committed it here:
stusmall@04d0f24

If you want I can make a PR.

Some documentation for cargo-fuzz can be found here: https://fuzz.rs/book/introduction.html

but basically you just need to install cargo fuzz with: "cargo install cargo-fuzz" and the run it in the rs folder with the command: "cargo fuzz run movie"

[demurgos]

Yes, it would be nice to open a PR to have it even if it is not enabled on CI yet.
It should not only fuzz parse_movie but also parse_tag and the individual tag parsers (I had some panics on invalid tag bodies).

I planned to use cargo-fuzz but wanted to focus on completion before figuring out how to propagate errors. Now that there are only a handful of tags left it would be good to look into this.
At the moment I am working on starting the Rust renderer, but I may look into fixing the parser panics next week. The most pressing issue right now is that there are not that much test samples. The parser is tested mostly on files from the web, but we eventually decided to not keep them on Github until we know if we are allowed to do it or not. It means that the code coverage of the tests is pretty poor. Having a fuzzer would help with coverage of error paths.

[demurgos]

The parser now supports the complete spec. It means that error handling becomes the main priority.

[demurgos]

@stusmall are you familiar with cargo fuzz?
I am getting out of memory (OOM) errors on trivial cases that pass without any issue when running them outside of the fuzzer (as regular unit tests). For example, I get OOM errors with an empty slice or (hex) 5b01060040.
I thought that the first error (OOM on empty) was because I used the first byte as the SWF version and it messed with the fuzzer, but the second case makes me doubt it.

It occurs with the tag fuzz target. You can clone the repo and run it with cd rs && cargo fuzz run tag.

Edit: Open a cargo-fuzz issue: rust-fuzz/cargo-fuzz#192

[demurgos]

This took a few months, but I believe this issue should be fixed soon. With the help of cargo fuzz I found all (hopefully) the panics and replaced them with proper error handling. cargo fuzz has been now running for 30minutes on my computer and generated 1 million samples without detecting any issue. I'll open the PR once cargo fuzz stops (or if I have to kill it).