Maia DAO - Ulysses - TangYuanShen's results

Lines of code

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

Vulnerability details

Impact

The VirtualAccount contract allows users to manage their assets and perform calls. Each function is protected with a modifier requiresApprovedCaller as access control. However, one critical function payableCall is missing this access control and it can be called by anyone.

This will allow anyone to withdraw all ERC20, ERC721, etc. tokens from the VirtualAccount and interact with the protocol on behalf of the VirtualAccount.

Proof of Concept

In the function payableCall, it can be seen that there is no access control whatsoever:

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

Tools Used

Manual review

Recommended Mitigation Steps

The modifier requiresApprovedCaller should be added to the payableCall function in VirtualAccount.

Assessed type

Access Control