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

Crashes found by honggfuzz #3

Closed
killercup opened this issue Apr 28, 2018 · 4 comments · Fixed by #6
Closed

Crashes found by honggfuzz #3

killercup opened this issue Apr 28, 2018 · 4 comments · Fixed by #6

Comments

@killercup
Copy link
Contributor

killercup commented Apr 28, 2018

Howdy! This is a 🐛 bug report for two crashes I found with the following fuzzer script that you can also find in rust-fuzz/targets#114:

pub fn fuzz_sleep_parser_header(data: &[u8]) {
    if let Ok(header) = sleep_parser::Header::from_vec(data) {
        sleep_parser::Header::from_vec(&header.to_vec()).unwrap();
    }
}

Should this assertion of from_vec¹ -> to_vec -> from_vec hold?

If yes, with data as either of

  • b"\x05\x02W\x01\x00\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xfb\x03p\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xbb9\xb0\xf5\xf5"
  • b"\x05\x02W\x01\x00\x00\x00\x12\x12\x12\x00\x00S\xc3\xcf\x8a2\xcc\xd1\xce9\xc4K\x9343\x00602\xb5\x07"

the current git master crashes. I have not investigated further.


¹ Why is this called from_vec when it takes a slice? I'd probably call it from_bytes which is more precise.

@yoshuawuyts
Copy link
Contributor

yoshuawuyts commented Apr 29, 2018

Awesome, this is a great find! It shouldn't ever be crashing tbh, so we should probably fix this!

edit: yeah, it should probably be called from_bytes, haha. Good one!

khernyo added a commit to khernyo/sleep-parser that referenced this issue May 14, 2018
khernyo added a commit to khernyo/sleep-parser that referenced this issue May 15, 2018
khernyo added a commit to khernyo/sleep-parser that referenced this issue May 15, 2018
khernyo added a commit to khernyo/sleep-parser that referenced this issue May 15, 2018
khernyo added a commit to khernyo/sleep-parser that referenced this issue May 15, 2018
@khernyo khernyo mentioned this issue May 15, 2018
2 tasks
@yoshuawuyts
Copy link
Contributor

@killercup by the way, do you maybe still have the code for the fuzzer? Would be great if we could check it in for future parts :D

@killercup
Copy link
Contributor Author

killercup commented Jun 5, 2018

The code is in rust-fuzz/targets#114 but i can make a PR to add it here too

Edit: #5

@killercup killercup mentioned this issue Jun 5, 2018
@yoshuawuyts
Copy link
Contributor

Woah, didn't realize sleep-parser was in the test suite :D That's great!

khernyo added a commit to khernyo/sleep-parser that referenced this issue Jun 5, 2018
yoshuawuyts pushed a commit that referenced this issue Jun 7, 2018
* Simplify header tests

* Fix typo

* Fixes #3: Crashes found by Honggfuzz

* No need to verify trailing zeros according to docs

* Stricter algorithm name parsing

Give up if an unknown algorithm name is encountered. According to docs,
the allowed algorithm names are "BLAKE2b", "Ed25519" and "".

* Cleanup
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

Successfully merging a pull request may close this issue.

2 participants