Maia DAO - Ulysses - josephdara's results

Lines of code

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

Vulnerability details

Impact

The function payableCall() is used to make calls on to other contracts from the virtualAccount which is linked to a userAddress. This function utilizes the PayableCall struct show below.

struct PayableCall {
    address target;
    bytes callData;
    uint256 value;
}

It allows for specially crafted calldata as well as any target address with any value. This allows for the theft of any ERC20, ERC721 or ERC1155 tokens owned by the contract through transfers or approvals.

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

As seen above, the contract does not have any access controls to prevent malicious execution of calls. Passing a call with a Struct like this would easily wipe the contract:

    PayableCall({
         target: usdt address,
         value: 0,
         calldata: bytes(abi.encodeWithSignature(bytes4(keccak256("transfer(address,uint256)")), 5000e6));
      })

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend implementing the proper access control provided in the contract. The requiresApprovedCaller modifier.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchBridgeAgent.sol#L180-L336 https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L160-L344

Vulnerability details

Impact

LayerZero requires a fee to be paid to process transactions successfully. If a user doesn't include enough eth for the gas, the transaction will fail, that's why an estimatefee function is available on the layerZero Endpoint address. However no check is done to prevent users from sending insufficient eth with transactions. This causes an issue as transactions will fail on the LayerZero layer since execution will run out of gas

Proof of Concept

       function callOutAndBridge(
        address payable _refundee,
        address _recipient,
        uint16 _dstChainId,
        bytes calldata _params,
        SettlementInput calldata _sParams,
        GasParams calldata _gParams,
        bool _hasFallbackToggled
    ) external payable override lock requiresRouter {
        // Create Settlement and Perform call
        bytes memory payload = _createSettlement(
            settlementNonce,
            _refundee,
            _recipient,
            _dstChainId,
            _params,
            _sParams.globalAddress,
            _sParams.amount,
            _sParams.deposit,
            _hasFallbackToggled
        );

        //Perform Call.
        _performCall(_dstChainId, _refundee, payload, _gParams);
    }

As we can see, estimatefee is not checked at all. It also is not checked in _performCall

Tools Used

Manual Review, LayerZero Docs

Recommended Mitigation Steps

Get the estimated native fees for a transaction of a particular payload using `estimateFees(), and check against the current msg.value`` to ensure that it is enough.

Assessed type

Error