Maia DAO - Ulysses - T1MOH's results

Lines of code

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

Vulnerability details

Impact

payableCall() misses modifier requiresApprovedCaller. As a result anyone can perform any call from VirtualAccount. For example it can be ERC20/ERC721 transfer.

Proof of Concept

I really don't see need to prove such an obvious issue with executable test.

Anyone can specify arbitrary target and calldata and perform call from VirtualAccount:

    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

Add modifier requiresApprovedCaller

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/RootBridgeAgent.sol#L1212-L1214

Vulnerability details

Impact

LayerZero passes argument bytes calldata _srcAddress. According to documentation it contains 40 bytes, actually it is 2 addresses concatenated. You can check detail explanation in according documentation

It encodes remoteAddress + localAddress. Where remoteAddress is caller on source chain, and localAddress is contract on destination chain - in other words address(this).

Problem is that it decodes localAddress and compares to trusted caller instead of remoteAddress. As a result RootBridgeAgent can't receive messages from other chains

Proof of Concept

Firstly docs explain in clear manner that 20-40 bytes is address(this)

However I want to provide Tenderly debug to prove this. Take a look on Endpoint's retryPayload() on Tenderly Function Endpoint.retryPayload() is fallback used to execute cross-chain message. Submitted _srcAddress = 0x3055913c90fcc1a6ce9a358911721eeb942013a1152649ea73beab28c5b49b26eb48f7ead6d4c898 - it means:

1. remoteAddress = 0x3055913c90fcc1a6ce9a358911721eeb942013a1
2. localAddress = 0x152649ea73beab28c5b49b26eb48f7ead6d4c898

In this tx Endpoint calls ERC20 token CakeOFT to mint tokens. Address of this token is indeed this localAddress, check this url. It proves point that sender of message is encoded in first 20 bytes of _srcAddress

However you can see that RootBridgeAgent tries to read the last 20 bytes:

    modifier requiresEndpoint(address _endpoint, uint16 _srcChain, bytes calldata _srcAddress) virtual {
        if (msg.sender != address(this)) revert LayerZeroUnauthorizedEndpoint();

        if (_endpoint != getBranchBridgeAgent[localChainId]) {
            if (_endpoint != lzEndpointAddress) revert LayerZeroUnauthorizedEndpoint();

            if (_srcAddress.length != 40) revert LayerZeroUnauthorizedCaller();

@>          if (getBranchBridgeAgent[_srcChain] != address(uint160(bytes20(_srcAddress[PARAMS_ADDRESS_SIZE:])))) {
                revert LayerZeroUnauthorizedCaller();
            }
        }
        _;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Refactor check:

    modifier requiresEndpoint(address _endpoint, uint16 _srcChain, bytes calldata _srcAddress) virtual {
        if (msg.sender != address(this)) revert LayerZeroUnauthorizedEndpoint();

        if (_endpoint != getBranchBridgeAgent[localChainId]) {
            if (_endpoint != lzEndpointAddress) revert LayerZeroUnauthorizedEndpoint();

            if (_srcAddress.length != 40) revert LayerZeroUnauthorizedCaller();

-           if (getBranchBridgeAgent[_srcChain] != address(uint160(bytes20(_srcAddress[PARAMS_ADDRESS_SIZE:])))) {
+           if (getBranchBridgeAgent[_srcChain] != address(uint160(bytes20(_srcAddress[0:PARAMS_ADDRESS_SIZE])))) {
                revert LayerZeroUnauthorizedCaller();
            }
        }
        _;
    }

Assessed type

Invalid Validation