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

Stream fuzzing fixes #715

Merged
merged 6 commits into from
May 5, 2021

Conversation

whalelephant
Copy link
Contributor

cc: #705

Focused on fuzzing StreamPacket

  • when parsing of stream packet data into valid known frames errors, it is returned instead of ignored
  • updated a fuzz test name for clarification of cause of failure
  • added new test and code for removing trailer bytes for fuzzing

TODO:

  • add new test and handle Unknown frames for fuzzing

Copy link
Collaborator

@koivunej koivunej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments and indentation nits.

crates/interledger-stream/src/packet.rs Outdated Show resolved Hide resolved
crates/interledger-stream/src/packet.rs Outdated Show resolved Hide resolved
}

Ok(buffer_unencrypted)
}
}

impl<'a> Iterator for FrameIterator<'a> {
type Item = Frame<'a>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could even be:

type Item = Result<Frame<'a>, WhateverTheErrorIs>;

And instead of returning a FrameIterator from StreamPacket::frames() we could return a impl Iterator<Item = Frame<'a>> which would be constructed from FrameIterator { ... }.map(|res| res.expect("checked at parsing time")).

This would however require some state in the FrameIterator to return None after the first Err return value.

However this isn't something that should be handled right away, perhaps polish at the end if anything.

crates/interledger-stream/src/packet.rs Outdated Show resolved Hide resolved
crates/interledger-stream/src/packet.rs Outdated Show resolved Hide resolved
@whalelephant whalelephant changed the title WIP: Stream fuzzing fixes Stream fuzzing fixes May 3, 2021
@whalelephant whalelephant marked this pull request as ready for review May 3, 2021 14:49
Copy link
Collaborator

@koivunej koivunej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, I had forgotten to approve this.

whalelephant and others added 6 commits May 5, 2021 19:47
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>
@koivunej koivunej merged commit f054cad into interledger:master May 5, 2021
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 this pull request may close these issues.

None yet

2 participants