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

Insecure external call

mishraji874

Description: The MondrianWallet2 contract uses a low-level call in its executeTransaction function without properly handling the possibility of a failed execution. While the contract does revert with a custom error MondrianWallet2__ExecutionFailed(), it doesn't distinguish between different types of failures, which could lead to unexpected behavior and potential loss of funds.

Impact: If a transaction fails due to reasons other than running out of gas (e.g., the called contract reverts), the wallet contract will revert, but it won't provide any detailed information about the failure. This lack of granularity in error handling could make it difficult for users and integrating systems to understand why a transaction failed, potentially leading to repeated failed attempts or misinterpretation of the contract's state.

Proof of Concept:

Below is the test code which proves that there is a insecure external call done by the executeTransaction method.

Paste this code in the new test file and see the result

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test, console} from "forge-std/Test.sol";
import {MondrianWallet2} from "../src/MondrianWallet2.sol";
import {Transaction} from "lib/foundry-era-contracts/src/system-contracts/contracts/libraries/MemoryTransactionHelper.sol";
import {BOOTLOADER_FORMAL_ADDRESS} from "lib/foundry-era-contracts/src/system-contracts/contracts/Constants.sol";
​
contract FailingContract {
function fail() external pure {
revert("Intentional failure");
}
}
​
contract ProofOfConcept is Test {
MondrianWallet2 wallet;
FailingContract failingContract;
address owner = address(0x1);
​
function setUp() public {
vm.prank(owner);
wallet = new MondrianWallet2();
failingContract = new FailingContract();
}
​
function testUncheckedLowLevelCall() public {
Transaction memory transaction = Transaction({
txType: 1,
from: uint256(uint160(address(wallet))),
to: uint256(uint160(address(failingContract))),
gasLimit: 100000,
gasPerPubdataByteLimit: 800,
maxFeePerGas: 2000000000,
maxPriorityFeePerGas: 1000000000,
paymaster: 0,
nonce: 0,
value: 0,
reserved: [uint256(0), uint256(0), uint256(0), uint256(0)],
data: abi.encodeWithSignature("fail()"),
signature: hex"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef00",
factoryDeps: new bytes32[](0),
paymasterInput: hex"",
reservedDynamic: hex""
});
​
vm.prank(BOOTLOADER_FORMAL_ADDRESS);
vm.expectRevert(bytes4(0x50058470)); // This is the selector for MondrianWallet2__ExecutionFailed()
wallet.executeTransaction(bytes32(0), bytes32(0), transaction);
}
}

Recommended Mitigation:

  1. Use a try-catch block to handle the low-level call and capture more detailed error information:

function executeTransaction(bytes32, bytes32, Transaction memory _transaction) external payable requireFromBootLoaderOrOwner {
try this.executeCall(_transaction.to, _transaction.value, _transaction.data) {
// Transaction succeeded
} catch Error(string memory reason) {
// The call reverted with a reason string
revert MondrianWallet2__ExecutionFailed(reason);
} catch (bytes memory lowLevelData) {
// The call reverted without a reason string
revert MondrianWallet2__ExecutionFailed("Unknown error");
}
}
​
function executeCall(uint256 _to, uint256 _value, bytes memory _data) external payable {
(bool success, bytes memory result) = address(uint160(_to)).call{value: _value}(_data);
if (!success) {
assembly {
revert(add(result, 32), mload(result))
}
}
}

  1. Consider using OpenZeppelin's Address.functionCall or similar utilities that provide more robust error handling for low-level calls.

  2. Implement a way to return detailed error information to the caller, possibly through events or returnable error codes.

  3. Add extensive logging (events) for both successful and failed transactions to aid in debugging and monitoring.

Updates

Lead Judging Commences

bube Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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