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

wip: recursion mitigations #495

Merged
merged 7 commits into from
Feb 6, 2024
Merged

wip: recursion mitigations #495

merged 7 commits into from
Feb 6, 2024

Conversation

prestwich
Copy link
Member

@prestwich prestwich commented Jan 23, 2024

Motivation

discussion aid for ongoing work on #490

Solution

  • recursion limit of 16
  • check that the decoder contains sufficient words before allocating memory

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes (additional enum variants)

@prestwich prestwich added the bug Something isn't working label Jan 23, 2024
@prestwich prestwich self-assigned this Jan 23, 2024
crates/sol-types/src/abi/decoder.rs Show resolved Hide resolved
crates/dyn-abi/src/token.rs Outdated Show resolved Hide resolved
@prestwich
Copy link
Member Author

@DaniPopes I'm unsure the best way to fix the proptest

@DaniPopes
Copy link
Member

@DaniPopes I'm unsure the best way to fix the proptest

Fixed in 0e9edd9

@DaniPopes
Copy link
Member

@prestwich anything else to be done here? Looks good enough to merge already

@prestwich
Copy link
Member Author

Are we confident this resolves the issues? or do we think additional work is needed?

@DaniPopes
Copy link
Member

DaniPopes commented Feb 6, 2024

I'd say so, in any case I would like to get this in for the next release. Further work can be in follow-ups.

@DaniPopes DaniPopes marked this pull request as ready for review February 6, 2024 20:56
@DaniPopes DaniPopes self-requested a review as a code owner February 6, 2024 20:56
@DaniPopes DaniPopes merged commit d261426 into main Feb 6, 2024
21 checks passed
@DaniPopes DaniPopes deleted the prestwich/recursion-fix branch February 6, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants