Maia DAO - Ulysses - Jorgect's results

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/src/VirtualAccount.sol#L85

Vulnerability details

Impact

The payableCall function in virtualAccount.sol is missing the requiresApprovedCaller modifier so this function can be called for whatever address performing malicius operation even draim the wallet

Proof of Concept

As we can see the payableCall function in virtualAccount.sol:

file:src/VirtualAccount.sol
 function payableCall(PayableCall[] calldata calls) public payable returns (bytes[] memory returnData) {
        uint256 valAccumulator;
        uint256 length = calls.length;
        returnData = new bytes[](length);
        PayableCall calldata _call;
        for (uint256 i = 0; i < length;) {
            _call = calls[i];
            uint256 val = _call.value;
            // Humanity will be a Type V Kardashev Civilization before this overflows - andreas
            // ~ 10^25 Wei in existence << ~ 10^76 size uint fits in a uint256
            unchecked {
                valAccumulator += val;
            }

            bool success;

            if (isContract(_call.target)) (success, returnData[i]) = _call.target.call{value: val}(_call.callData);

            if (!success) revert CallFailed();

            unchecked {
                ++i;
            }
        }

        // Finally, make sure the msg.value = SUM(call[0...i].value)
        if (msg.value != valAccumulator) revert CallFailed();
    }

https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/src/VirtualAccount.sol#L85C1-L112C6

This function is missing the requiresApprovedCaller wich is implemented in the rest of the function of the virtualAccount contract

This function is making external calls and is passing values took from the virtual wallet, so malicius user can call this function passing other smart contract create for himselft taking the funds.

Tools Used

Manula review

Recommended Mitigation Steps

Add requiresApprovedCaller modifier in the payableCall function:

 function payableCall(PayableCall[] calldata calls) public payable requiresApprovedCaller returns (bytes[] memory returnData) {...}

Assessed type

Access Control

LOW REPORT

[L-01] Consider check for the lzReceive calls.

The check value for the call in lzReceive function is not checked

 function lzReceive(uint16, bytes calldata _srcAddress, uint64, bytes calldata _payload) public override {
        address(this).excessivelySafeCall(
            gasleft(),
            150,
            abi.encodeWithSelector(this.lzReceiveNonBlocking.selector, msg.sender, _srcAddress, _payload)
        );
    }

https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/src/BranchBridgeAgent.sol#L578C4-L584C6

Consider check for this value, and keep it. User can now se clearly his transaccion fail.

[L-02] Users can open whatever virtual acconunt

The fetchVirtualAccount function in the virtualAccount.sol contract allow open virtual account of whatever addres.

 function fetchVirtualAccount(address _user) external override returns (VirtualAccount account) {
        account = getUserAccount[_user];
        if (address(account) == address(0)) account = addVirtualAccount(_user);
    }

https://github.com/code-423n4/2023-09-maia/blob/c0dc3550e0754571b82d7bfd8f0282ac8fa5e42f/src/RootPort.sol#L350C4-L353C6

Consider allow user to open just his only account.