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

`MondrianWallet::_validateSignature` does not check recovered address from signature verification, allowing any malicious actor to send operations to the wallet

ECunha

Summary

MondrianWallet::validateUserOp is not checking the recovered address from ECDSA.recover against an allowed address. Consequently, this contract is vulnerable to any kind of operation from malicious actors.

Vulnerability Details

When sending an operation, the EntryPoint contract calls MondrianWallet::validateUserOp to validate the user operation. One of the processes during this validation is to check the signature from PackedUserOperation.signature by calling MondrianWallet::_validateSignature internal function. The issue is the returned value from ECDSA.recover is never checked against an allowed address or the owner.

Below are the steps to check this vulnerability.

Proof of Code Create a new smart contract in `contracts` folder called `MondrianWalletHarness.sol` to enable calling `_validateSignature` internal function with the following code:
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.24;
​
import {MondrianWallet, PackedUserOperation} from "./MondrianWallet.sol";
​
contract MondrianWalletHarness is MondrianWallet {
constructor(address entryPoint) MondrianWallet(entryPoint) {}
​
function validateSignature(
PackedUserOperation calldata userOp,
bytes32 userOpHash
) external pure returns (uint256) {
return _validateSignature(userOp, userOpHash);
}
}

Create a new file in ignition/modules folder called MondrianModuleHarness.js with the code:

const { buildModule } = require("@nomicfoundation/hardhat-ignition/modules")
const { networkConfig } = require("../../helper-config")
const { network } = require("hardhat")
const { MockEntryPoint } = require("./MockEntryPoint.js")
​
const MondrianModuleHarness = buildModule("MondrianModuleHarness", (m) => {
const chainId = network.config.chainId
​
let entryPointAddress
if (chainId == 1337) {
console.log("Deploying MockEntryPoint")
ignition.deploy(MockEntryPoint).then((mockEntryPoint) => {
entryPointAddress = mockEntryPoint.address
})
}
entryPointAddress = entryPointAddress || m.getParameter("entryPointAddress", networkConfig[chainId].entryPoint)
const mondrianWalletHarness = m.contract("MondrianWalletHarness", [entryPointAddress])
return { mondrianWalletHarness }
})
​
module.exports = MondrianModuleHarness

In MondrianWallet.test.js file, place the following test:

it("Anyone can send operations", async function () {
// Deploying contract harness to be able to call _validateSignature internal function
const mondrianWalletHarnessDeployment = await ignition.deploy(MondrianModuleHarness)
const mondrianWalletHarness = mondrianWalletHarnessDeployment.mondrianWalletHarness
​
// Second account of the getSigners list. The first is the contract deployer.
const [_, account] = await ethers.getSigners();
​
// Generating and signing message
const message = ethers.keccak256(ethers.toUtf8Bytes("test"))
const sig = await account.signMessage(message)
​
// Building PackedUserOperation
const userOp = [account.address, 0, "0x", "0x", ethers.ZeroHash, 0, ethers.ZeroHash, "0x", sig]
​
// Calling function to validate signature
const result = await mondrianWalletHarness.validateSignature(userOp, message)
​
// Checking result (0 for signature validation success)
assert.equal(result, 0)
})

Then just use yarn test in the command line to check the result.

Impact

Any actor (EOAs or smart contracts) can perform any operations on this contract, making it vulnerable to any malicious transaction, such as:

  • drain funds

  • transfer NFTs

  • take ownership

among others.

Tools Used

Manual review, Hardhat, and Solidity

Recommendations

Check the recovered address against the contract owner:

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

Lead Judging Commences

inallhonesty Lead Judge about 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.