Signature Replay attack on re-created smart accounts

[code423n4]

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 a selfdestruct operation, all the smart contract`s storage is gone. That means if user re-creates their smart contract account, the nonces 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:

1. Alice creates a SmartAccount proxy using the SmartAccountFactory.deployCounterFactualWallet with the following args:
   _owner: address(0xabababababababababababababababababababab)
   _entryPoint: address(0xcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd)
   _handler: address(0xefefefefefefefefefefefefefefefefefefefef)
   _index: 0
2. Alice deposit 10 ETH to her smart contract account
3. Alice transfers the deposited 10 ETH to Bob (this is her first transaction, so the nonce=0)
4. At some point Alice decides she want to temporaly delete her account
5. After a few days Alice wants to create a smart contract account with the same address as the previous one
6. She calls SmartAccountFactory.deployCounterFactualWallet with the same args from 1.
7. Alice assumes that she is starting to use this account from scratch as it is absolutely clear
8. Alice deposits 10 ETH again and hodls
9. Bob notices what Alice has done and decides to replay the first transaction Alice has executed (and signed) for her previous account
10. Her first transaction was exactly sending 10 ETH to Bob
11. As the nonce of the previous first transaction was 0 and the current nonce in the re-created account is also 0, Bob can execute this transaction and steal Alice's 10 ETH

NOTE: Although the domainSeparator is used to calculate the account transaction hash, this doesn't prevent the signature replay as the verifyingContract (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 the Transaction struct and check against block.timestamp in SmartAccount.execTransaction.

struct Transaction {
        address to;
        uint256 value;
        bytes data;
        Enum.Operation operation;
        uint256 targetTxGas;
	uint256 expirationTime // @audit add expiration time to prevent replay attack on re-created smart contract
    }

[c4-judge]

gzeon-c4 marked the issue as duplicate of #127

[c4-sponsor]

livingrockrises marked the issue as disagree with severity

[c4-sponsor]

livingrockrises marked the issue as sponsor disputed

[livingrockrises]

explain delete account and recreated smart account

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)

[c4-judge]

gzeon-c4 marked the issue as grade-b

[GeorgeHNTR]

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

[gzeon-c4]

The risk is low because the smart account being self-destructed and later funded with value is very unlikely to happen.