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

Check staticcall Result From SHA-256 Precompile #457

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

nlordell
Copy link
Collaborator

@nlordell nlordell commented Jul 4, 2024

Fixes hats-finance#14, also see hats-finance#22 for some additional context.

This PR changes the _sha256 implementation to check the result from the static call. There is a very subtle bug with not checking, where, for very large inputs, you would be able to get the precompile to revert but have the function finish executing successfully (and use whatever is in the scratch space as the digest).

Note that we do not check the length of the returndata. This is intentional and the same thing that the Solidity compiler does for the builtin sha256 function.

This PR changes the `_sha256` implementation to check the result from
the static call. There is a very subtle bug with not checking, where,
for very large inputs, you would be able to get the precompile to revert
but have the function finish executing successfully (and use whatever is
in the scratch space as the digest).

Note that **we do not check the length of the `returndata`**. This is
intentional and the same thing that the Solidity compiler does for the
builtin `sha256` function.
@nlordell nlordell requested a review from a team as a code owner July 4, 2024 15:35
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for a team July 4, 2024 15:35
@nlordell nlordell merged commit 61a4e06 into main Jul 5, 2024
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

low level staticcall return value is not checked
2 participants