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

Fixed infinite recursion on recursive hash() on CBORTag #202

Merged
merged 5 commits into from
Dec 28, 2023

Conversation

agronholm
Copy link
Owner

Relates to #198.

@coveralls
Copy link

coveralls commented Dec 28, 2023

Coverage Status

coverage: 93.164% (+0.1%) from 93.069%
when pulling 03dfb8e on cbortag-hash-fix
into c0007fc on master.

@mschwager
Copy link
Contributor

I fuzzed this branch for quite some time (multiple hours) and didn't produce any more crashes. The original segfaults were found in a matter of minutes, so I suspect we've found the low hanging fruit.

@agronholm
Copy link
Owner Author

This was no low hanging fruit, I tell you :) It was the hardest fix to date, for me at least.

@agronholm agronholm merged commit 91e289a into master Dec 28, 2023
13 checks passed
@agronholm agronholm deleted the cbortag-hash-fix branch December 28, 2023 21:10
@agronholm
Copy link
Owner Author

What input is producing segfaults now? I only remember one being mentioned, but I fixed that one already.

@mschwager
Copy link
Contributor

This was no low hanging fruit, I tell you :) It was the hardest fix to date, for me at least.

Sorry, I meant low hanging fruit for the fuzzer to find crashes, not for the fix. By low hanging fruit I just meant the fuzzer was able to find the crashes in minutes or hours rather than days or weeks of runtime 👍

What input is producing segfaults now? I only remember one being mentioned, but I fixed that one already.

As far as segfault crashes, there were two:

I re-ran the fuzzer against this branch and neither of those crashes, or any others, appeared after a few hours of runtime.

@agronholm
Copy link
Owner Author

Ok, we're on the same page then. Perhaps the MemoryError still needs to be investigated, but the rest of the errors just need to be wrapped in CBORDecodeError.

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.

3 participants