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

Lack of checks for transaction reuse could leads to unauthorized re-execution of transactions or repeated actions

0x5chn0uf

Summary

The MondrianWallet2::_executeTransaction function does not check for transaction reuse when owners execute transactions.

This allows an owner to re-execute the same validated transaction several times, potentially compromising the integrity and security of protocol.

In the absence of transaction reuse verification mechanisms, a malicious owner can repeatedly initiate unauthorized actions, resulting in financial losses and other risks for authorized users of the contract. Potentially, if the transaction is stuck in an infinite loop, this can also lead to a DoS.

Vulnerability Details

  1. A malicious owner defines its transaction

  2. The malicious owner uses his transaction as many times as he likes

Proof of Code

Copy / Paste the following test into ModrianWallet2Test.t.sol

function testZkOwnerCanReeuseSameSignature() public {
assertEq(usdc.balanceOf(address(mondrianWallet)), 0);
// Arrange
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
uint256 TIME_EXECUTED = 10;
vm.startPrank(mondrianWallet.owner());
for (uint256 i = 0; i < TIME_EXECUTED; i++) {
mondrianWallet.executeTransaction(
EMPTY_BYTES32,
EMPTY_BYTES32,
transaction
);
}
vm.stopPrank();
​
// Assert
assertEq(
usdc.balanceOf(address(mondrianWallet)),
TIME_EXECUTED * AMOUNT
);
}

Impact

By repeatedly executing the same transaction with the same signature, contract does not check whether a transaction with this signature has already been processed. This could potentially lead to unwanted side-effects or repeated actions that affect the protocol integrity.

Tools Used

Manual review

Recommendations

Add a mapping to store the nonces of transactions already executed and modify the _executeTransaction function to check whether the transaction nonce has already been used before executing it:

contract MondrianWallet2 {
+ mapping(uint256 => bool) private usedNonces;
...
function _executeTransaction(Transaction memory _transaction) internal {
address to = address(uint160(_transaction.to));
uint128 value = Utils.safeCastToU128(_transaction.value);
bytes memory data = _transaction.data;
+ uint256 nonce = _transaction.nonce;
​
+ // Check if nonce has already been used
+ require(!usedNonces[nonce], "Transaction nonce already used");
+ usedNonces[nonce] = true;
Updates

Lead Judging Commences

bube Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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