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

Signature verification does not enforce a maximum size on the signature bytes #754

Open
nlordell opened this issue Apr 22, 2024 · 0 comments
Labels

Comments

@nlordell
Copy link
Collaborator

nlordell commented Apr 22, 2024

Description

The function checkNSignatures does not enforce a size limit on the signature bytes. This means that they can be padded with arbitrary data. This can be used by manipulating the signatures by padding them with additional data while still remaining valid and, since the signatures bytes get copied from calldata into memory, increase the total gas consumption of the checkNSignatures function. This is an issue when the Safe is combined with the ERC-4337 module, where gas costs for the ERC-4337 user operation are paid by the account. A malicious relayer can grief the account by padding the signatures bytes to include extra 0s causing the account to pay more in fees than it would have with an optimally encoded signatures bytes.

A workaround for existing Safes is to set a strict verificationGasLimit for ERC-4337 user operations, which would set a strict upper bound on how much gas can be paid during signature verification and limit the potential additional fees.

Shoutout to @adamegyed for submitting this issue as part of the bug bounty program.

Solution

The solution is to require stricter encoding of Safe signatures bytes and disallow padding. This can be implemented by:

  • Requiring the s value to be a specific offset (equal to the length of the fixed signature part 65*n plus the length of the previous contract signatures that have been verified)
  • Requiring the length to not have any additional bytes
@nlordell nlordell added the bug label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant