Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Missing validation check in `MondrianWallet2::executeTransactionFromOutside` allows anyone to execute transactions through the `MondrianWallet2` Account Abstraction.

Missing validation check in MondrianWallet2::executeTransactionFromOutside allows anyone to execute transactions through the MondrianWallet2 Account Abstraction. It breaks any kind of restriction of the contract and renders it unusable.

Description: The function MondrianWallet2::executeTransactionFromOutside misses a check on the result of MondrianWallet2::_validateTransaction. _validateTransaction does not revert when validation fails, but returns bytes4(0). Without check, MondrianWallet2::_executeTransaction will always be called, even when validation of the transaction failed.

function executeTransactionFromOutside(Transaction memory _transaction) external payable {
_validateTransaction(_transaction);
_executeTransaction(_transaction);
}

Impact: Because of the missing check in executeTransactionFromOutside, anyone can sign and execute a transaction. It breaks any kind of restriction of the contract, allowing for immediate draining of all funds from the contract (among other actions) and renders it effectively unusable.

Proof of Concept:

  1. A malicious user creates a transaction.

  2. The malicious user signs the transaction with a random signature.

  3. The malicious user calls MondrianWallet2::executeTransactionFromOutside with the randomly signed transaction.

  4. The transaction does not revert and is successfully executed.

Proof of Concept

Place the following in ModrianWallet2Test.t.sol.

// Please note that you will also need --system-mode=true to run this test.
function testMissingValidateCheckAllowsExecutionUnvalidatedTransactions() public onlyZkSync {
// setting up accounts
address THIRD_PARTY_ACCOUNT = makeAddr("3rdParty");
vm.deal(address(mondrianWallet), AMOUNT);
address dest = address(usdc);
uint256 value = 0;
bytes memory functionData = abi.encodeWithSelector(ERC20Mock.mint.selector, address(mondrianWallet), AMOUNT);
Transaction memory transaction = _createUnsignedTransaction(mondrianWallet.owner(), 113, dest, value, functionData);
// Act
// we sign transaction with a random signature
bytes32 unsignedTransactionHash = MemoryTransactionHelper.encodeHash(transaction);
uint256 RANDOM_KEY = 0x00000000000000000000000000000000000000000000000000000000007ceda5;
(uint8 v, bytes32 r, bytes32 s) = vm.sign(RANDOM_KEY, unsignedTransactionHash);
Transaction memory signedTransaction = transaction;
signedTransaction.signature = abi.encodePacked(r, s, v);
// and the transaction still passes.
vm.prank(THIRD_PARTY_ACCOUNT);
mondrianWallet.executeTransactionFromOutside(signedTransaction);
assertEq(usdc.balanceOf(address(mondrianWallet)), AMOUNT);
}

Recommended Mitigation: Add a check, using the result from _validateTransaction. Note that when validation succeeds, _validateTransaction returns the selector of IAccount::validateTransaction. MondrianWallet2 already imports this value as ACCOUNT_VALIDATION_SUCCESS_MAGIC.

Add a check that _validateTransaction returns the value of ACCOUNT_VALIDATION_SUCCESS_MAGIC. If the check fails, revert with the error function error MondrianWallet2__InvalidSignature(). This error function is already present in MondrianWallet2.sol.

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

Lead Judging Commences

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