-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Added cargo-fuzz tests for Rlp crate #6145
Changes from 1 commit
1bb6244
2ae9599
5d00521
d059d69
e2799de
40dc411
17bd401
5373ba4
34fb2bc
3766434
fbc3436
728e3e1
c3d80a8
5b84221
25b74cd
e3151fb
3786e22
a7f2279
0d23592
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
|
||
target | ||
corpus | ||
artifacts |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
|
||
[package] | ||
name = "rlp-fuzz" | ||
version = "0.0.1" | ||
authors = ["Automatically generated"] | ||
publish = false | ||
|
||
[package.metadata] | ||
cargo-fuzz = true | ||
|
||
[dependencies] | ||
rand = "0.3" | ||
|
||
[dependencies.rlp] | ||
path = ".." | ||
[dependencies.libfuzzer-sys] | ||
git = "https://github.com/rust-fuzz/libfuzzer-sys.git" | ||
|
||
# Prevent this from interfering with workspaces | ||
[workspace] | ||
members = ["."] | ||
|
||
[[bin]] | ||
name = "untrusted" | ||
path = "fuzz_targets/untrusted.rs" | ||
|
||
[[bin]] | ||
name = "append_raw" | ||
path = "fuzz_targets/append_raw.rs" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
#![no_main] | ||
#[macro_use] extern crate libfuzzer_sys; | ||
extern crate rlp; | ||
extern crate rand; | ||
|
||
use rlp::RlpStream; | ||
use rand::Rng; | ||
|
||
fuzz_target!(|data: &[u8]| { | ||
let mut rls = RlpStream::new(); | ||
|
||
// Generate random item count to further fuzz append_raw's parsing | ||
let mut rng = rand::thread_rng(); | ||
let rand_item_count = rng.gen_range::<usize>(1 as usize, 4096 as usize); | ||
|
||
let _ = rls.append_raw(data, rand_item_count); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
what do you expect to find by fuzz-testing this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. trying to get full fuzz-coverage of the classes in the rlp crate, and I hadn't considered where the data may be coming from. I agree it makes more sense to fully fuzz the UntrustedRlp struct, and am focusing more on that now that the foundational stuff is fixed. Also still becoming more familiar with the code base, another expectation/goal of writing fuzz tests for all the code. If for some reason or another, a developer decides to use a normally internal function/struct, it would be beneficial to know if that feature is buggy. |
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
#![no_main] | ||
#[macro_use] extern crate libfuzzer_sys; | ||
extern crate rlp; | ||
|
||
use rlp::UntrustedRlp; | ||
|
||
fuzz_target!(|data: &[u8]| { | ||
let _ = UntrustedRlp::new(data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. creating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, what do you mean by all the lists? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RLP is a data format consisting mostly of nested lists, so you could try iterating over all of them recursively. This might give the fuzzer more to work with in terms of exploring code paths There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added another fuzz test that creates a list (prepending
So now I'm thinking of creating lists of various types from the fuzz data, and recursively calling What about taking variable length slices from the raw fuzz data, then using those as list items, prepended by their length? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
fn iter_recursive(rlp: UntrustedRlp) {
for x in rlp.iter() {
iter_recursive(x)
}
} this would essentially walk the whole RLP structure recursively There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this sounds like it requires building a minimally well-formed list in the RLP structure. is this what you are talking about building from the fuzz data? i don't fully understand your thinking on this. i get the code coverage, just trying to figure out guiding the fuzzer without constraining it too much. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after reading through
Do you think throwing fuzz data into these list items will get the right code/crash coverage (using your recursive function to traverse the RLP structure)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the interesting thing about RLP is that actually, each item is either one of a few things:
This is why it's called "recursive length prefix". What I'm suggesting is that we have the fuzzer try and generate data which we treat as a bunch of nested lists of bytes (and try to walk accordingly). This will allow the fuzzer to attempt to explore almost all code paths in the decoder, possibly finding some well-formed RLP that leads to panic. Most of the data it generates will end up being invalid RLP, but it may find some valid RLPs which are not handled correctly. Isn't this the usual strategy with fuzz testing? e.g. if you have a PNG decoder, you try to have the fuzzer generate data that breaks the decoder, not to make the fuzzer generate a picture of a horse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Currently, I'm using the Thanks for the link to the spec, I'll go over it again to get more ideas for what to throw at this thing. I'm understanding better how RLP really works now. Hopefully this leads to some really exhaustive fuzz code.
Absolutely agree. I'm trying to add more to the fuzzer corpus, so that the fuzzer's mutation algorithms can be more efficient in breaking the RLP decoder. Like sending in a PNG of a horse with a trojan encoded inside :) |
||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you set this to
Parity Technologies <admin@parity.io>
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can do :)