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 Length Check FV #464

Merged
merged 10 commits into from
Jul 18, 2024
Merged

Signature Length Check FV #464

merged 10 commits into from
Jul 18, 2024

Conversation

remedcu
Copy link
Member

@remedcu remedcu commented Jul 15, 2024

Closes #461

This PR adds a rule which verifies that the signature length check is ensured using the _checkSignaturesLength(...) in the Safe4337Module.

The rule ensures that a signature that the bundler could manipulate, which could clear the check in Safe using checkSignatures(...) will still get caught using the _checkSignaturesLength(...). For this, we use a canonicalSignatureHash(...) which generates the same hash for valid signatures, with and without the excess data. An example is added to show this case before the function is written.

Also have added the workflow for checking the same in CI (Currently, it uses a script, which will be modified in a later PR to be consistent with our other repos).

@remedcu remedcu self-assigned this Jul 15, 2024
@remedcu remedcu requested a review from a team as a code owner July 15, 2024 13:47
@remedcu remedcu requested review from nlordell, akshay-ap and mmv08 and removed request for a team July 15, 2024 13:47
@coveralls
Copy link

coveralls commented Jul 15, 2024

Pull Request Test Coverage Report for Build 9988150675

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 9906147334: 0.0%
Covered Lines: 51
Relevant Lines: 51

💛 - Coveralls

@remedcu remedcu requested a review from nlordell July 18, 2024 10:24
@remedcu remedcu merged commit 3f0e7b5 into main Jul 18, 2024
14 checks passed
@remedcu remedcu deleted the fv-signature-length-check branch July 18, 2024 11:12
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 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.

Formally Verify Integrity of Signature Length Check
4 participants