Add some missing bounds checks.#260
Conversation
seanmonstar
left a comment
There was a problem hiding this comment.
Cool, thanks for submitting these!
How'd you find them? It might be useful to have such tests in the repo directly...
src/frame/headers.rs
Outdated
| if src.len() < 1 { | ||
| return Err(Error::MalformedMessage); | ||
| } | ||
| // TODO: Ensure payload is sized correctly |
There was a problem hiding this comment.
Maybe there's more involved, but wanted to check: does this addition essentially complete this TODO?
There was a problem hiding this comment.
Seems like it; I removed the comment.
src/proto/streams/streams.rs
Outdated
| let last_stream_id = frame.last_stream_id(); | ||
| let err = frame.reason().into(); | ||
|
|
||
| if actions.recv.max_stream_id() < last_stream_id { |
There was a problem hiding this comment.
I got a bit confused with this, until I went digging into recv to read the comments about max_stream_id. Whatcha think if there was a comment right here just saying to the effect of "if a new GOAWAY has a higher stream id than a previous GOAWAY, that's bad"?
|
Thanks @goffrie! Looks like the fuzzing has been submitted in another PR. I'm good with this! |
Ran a fuzzer and found a few places where we were panicking instead of returning errors.