Maia DAO - Ulysses - btk's results

Lines of code

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

Vulnerability details

Impact

The lack of access control in VirtualAccount::payableCall function allows anyone to execute arbitrary calls with any contract from a user's VirtualAccount. This enables malicious actors to steal user's funds easily, ultimately resulting in a PublicVirtualAccount.

Proof of Concept

    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();
    }

As per of the readme the VirtualAccount should be able to hold ERC20's and ERC721's tokens, hence not posssible with a public payableCall()

Tools Used

Manual review

Recommended Mitigation Steps

Add requiresApprovedCaller to payableCall().

Assessed type

Access Control