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

Initialization requirement in external functions of contract `MondrianWallet2` causes confusion and wasted user fees on failed functional calls

soupy

Summary

Functions in contract MondrianWallet2 are not usable if the contract has not been initialized after deployment

Vulnerability Details

External functions in contract MondrianWallet2 will revert with EvmError if the contract is not initialized after deployment. This could cause user in doubts with no clear clues of the failed calls and waste fees on making calls to transaction functions that will eventually fail due to no initialization done.

Proof of Concept:
In the file test/ModrianWallet2Test.t.sol, comment out the initization codes in the setUp function :

function setUp() public {
implementation = new MondrianWallet2();
ERC1967Proxy proxy = new ERC1967Proxy(address(implementation), "");
mondrianWallet = MondrianWallet2(address(proxy));
// mondrianWallet.initialize();
// mondrianWallet.transferOwnership(ANVIL_DEFAULT_ACCOUNT);
usdc = new ERC20Mock();
vm.deal(address(mondrianWallet), AMOUNT);
}

Test functions that required a prior initialization will fail for EvmError and revert with no clear message even with high verbosity added during forge test

Impact

Core functions in contract MondrianWallet2 are not usable and revert with no clear message, wasting user's fund to pay gas fees. This could also limit the contract’s intended operations until proper setup is done.

Tools Used

Manual review

Recommendations

Include condition check on all external functions that require prior ownership and upgrada related initalization. Implement a clear initialization error message to divert user on what they shall do instead of carrying out multiple effortful attempts on those function calls.

contract MondrianWallet2 is IAccount, Initializable, OwnableUpgradeable, UUPSUpgradeable {
using MemoryTransactionHelper for Transaction;
​
error MondrianWallet2__NotEnoughBalance();
error MondrianWallet2__NotFromBootLoader();
error MondrianWallet2__ExecutionFailed();
error MondrianWallet2__NotFromBootLoaderOrOwner();
error MondrianWallet2__FailedToPay();
error MondrianWallet2__InvalidSignature();
+ error MondrianWallet2__NotInitialized();
​
...
​
+ modifier requireInitialization() {
+ if (owner() == address(0)) {
+ revert MondrianWallet2__NotInitialized();
+ }
+ _;
+ }
​
...
​
function validateTransaction(bytes32, /*_txHash*/ bytes32, /*_suggestedSignedHash*/ Transaction memory _transaction)
external
payable
+ requireInitialization
requireFromBootLoader
returns (bytes4 magic)
{...}
​
function executeTransaction(bytes32, /*_txHash*/ bytes32, /*_suggestedSignedHash*/ Transaction memory _transaction)
external
payable
+ requireInitialization
requireFromBootLoaderOrOwner
{...}
​
function executeTransactionFromOutside(Transaction memory _transaction)
external
payable
+ requireInitialization
{...}
​
function payForTransaction(bytes32, /*_txHash*/ bytes32, /*_suggestedSignedHash*/ Transaction memory _transaction)
external
payable
+ requireInitialization
{...}
​
...
Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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