safe-smart-account
safe-smart-account copied to clipboard
Add stricter checks on signature length
Fixes #754
This PR enforces stricter checks on the signature length during verification. The checkNSignatures
now checks that after completing the signature verification, the offset
points to the end of the signature data. This ensures that no additional bytes are present than required for the verification to work.
Without this change, currently there is no restriction on length of signature submitted for verification due to which an attacker can possibly append additional bytes when using Safe + 4337 module and hit verificationGasLimit
. This can cause Safe to pay more for verification than needed.
Note: A transaction will fail with GS028
or GS021
based on the how signatures are submitted when signatures contain additional approvals than required threshold. Wallet and other applications have to consider this during error handling if relevant.
Changes in PR
-
Safe
contract checks if signature data does not contain additional bytes data than required - Add test cases:
- Signature verification reverts if additional bytes are added to signatures than required
- Signature verification reverts signature count exceeds required threshold
- Fix tests
- In Ethers.js defaultAbiCoder pads the bytes data to nearest word which appends additional zeros to signature data. This lead to failing test
- Submit only required number of signatures in Compatibility Fallback Handler tests and benchmarking
- New error
GS028
Codesize change
This PR
Safe 21877 bytes (limit is 24576)
SafeL2 22657 bytes (limit is 24576)
Main branch
Safe 21832 bytes (limit is 24576)
SafeL2 22612 bytes (limit is 24576)
Gas implications
TODO
Problem
If the signatures payload contains more approvals from owners than required threshold
, the signature validation will fail. This is a breaking change for wallet