Open code423n4 opened 1 year ago
gzeon-c4 marked the issue as duplicate of #127
livingrockrises marked the issue as disagree with severity
livingrockrises marked the issue as sponsor disputed
explain delete account and recreated smart account
gzeon-c4 changed the severity to QA (Quality Assurance)
gzeon-c4 marked the issue as grade-b
explain delete account and recreated smart account
Sorry for not describing it better and providing a PoC in code, I was running out of time:
delete account
= call selfdestruct
via delegatecall
for the SmartAccount contract proxy
recreated smart account
= re-deployed SmartAccount contract proxy on the same address (using create2
) after selfdestruct()
I think this is a valid finding of medium severity according to C4's severity criteria leak value with a hypothetical attack path with stated assumptions
The risk is low because the smart account being self-destructed and later funded with value is very unlikely to happen.
Lines of code
https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/BaseSmartAccount.sol#L12-L18
Vulnerability details
Signature Replay attack on re-created smart accounts
Impact
User's can loose funds or any unexpected behaviour can occur that transaction replay attacks usually lead to.
Proof of Concept
Consider the case when someone decides they want to destroy their smart account for some reason. Since the smart contract (account) is created from the factory using the
create2
opcode from, users are able to re-deploy their smart contract account again after a self-destruct. The problem comes from that after aselfdestruct
operation, all the smart contract`s storage is gone. That means if user re-creates their smart contract account, thenonces
mapping will be deleted. This will give the opportunity for any external malicious account to replay already passed transaction as the contract owner is the same and the nonces counting restart from 0.Consider the following example:
NOTE: Although the
domainSeparator
is used to calculate the account transaction hash, this doesn't prevent the signature replay as theverifyingContract
(address(this)
) is the same in both the old smart contract account (the selfdestructed one) and the new one.Tools Used
Manual review
Recommended Mitigation Steps
Add
expirationTime
propery in theTransaction
struct and check against block.timestamp inSmartAccount.execTransaction
.