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

0x3adeade - Signers can frontrun Safe.exectransaction() when using delegatecalls to make it Revert. #52

Open
sherlock-admin4 opened this issue Nov 23, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Nov 23, 2024

0x3adeade

Medium

Signers can frontrun Safe.exectransaction() when using delegatecalls to make it Revert.

Summary

Malicious Signers and hat wearers can use removeSigner() , claimSigner() to grief Safe.exectransaction() when using delegatecalls.

Root Cause

Safe's Safe.execTransaction() calls function HSG::checkAfterExecution() which is used to check If the transaction did not maliciously affect the safe, However in case of delegatecall transactions HSG::checkAfterExecution() calls HSG::_checkSafeState() which has an inbuilt check for change in number of owners:-

if (keccak256(abi.encode(_safe.getOwners())) != _existingOwnersHash) revert CannotChangeOwners();

https://github.com/sherlock-audit/2024-11-hats-protocol/blob/main/hats-zodiac/src/HatsSignerGate.sol#L960
If A signer renounces their hat and calls removeSigner() and frontruns execTransaction() It would change number of owners causing above check to fail and make transaction revert.
A wearer of Hat can also use claimSigner() to achieve the same.

Internal pre-conditions

TX uses Delegatecall operation.

External pre-conditions

No response

Attack Path

  • Wait for Safe.execTransaction() using a delegatecall to appear in TxPool
  • Frontrun Tx by using removeSigner() or claimSigner().
  • _checkSafeState() would fail and Tx would revert.

Impact

Griefing of Users, Bad UX.

PoC

No response

Mitigation

No response

@sherlock-admin2 sherlock-admin2 changed the title Joyful Gingham Tarantula - Signers can frontrun Safe.exectransaction() when using delegatecalls to make it Revert. 0x3adeade - Signers can frontrun Safe.exectransaction() when using delegatecalls to make it Revert. Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant