Ulysses Omnichain - Critical Business Logic Error regarding anyCall Pay Fees on Destination Chain Implementation

[code423n4]

Lines of code

https://github.com/anyswap/multichain-smart-contracts/blob/main/contracts/anycall/v7/AnycallV7Upgradeable.sol#L207
https://github.com/anyswap/multichain-smart-contracts/blob/main/contracts/anycall/v7/AnycallV7Upgradeable.sol#L174-L186

Vulnerability details

Ulysses Omnichain - Critical Business Logic Error regarding anyCall Pay Fees on Destination Chain Implementation

Impact

Summary

There is a critical error in the Ulysses Omnichain implementation of the anyCall Pay Fees on Destination / Source Chain 'module'. The error relates to the configuration of the flags parameter and lack of any fees sent to satisfy anyCall protocol fees.

For more details on requirements to calling anyCall properly, see:

• https://github.com/anyswap/multichain-smart-contracts/blob/main/contracts/anycall/v7/AnycallV7Upgradeable.sol#L207
• https://github.com/anyswap/multichain-smart-contracts/blob/main/contracts/anycall/v7/AnycallV7Upgradeable.sol#L174-L186

Details

The implementation of anyCall within Ulysses Omnichain is currently misconfigured to pay fees on the source chain instead of the destination chain. Additionally, the implementation of _performCall in BranchBridgeAgent.sol lacks payment for the unavoidable base or custom "source fees", as is required by the anyCall protocol. Additionally, the project team have communicated through private messaging with the audit team that they intended the protocol to be configured to pay on destination. However, the flags used to configure fee payment for anyCall is set to pay all fees on the source chain.

The impact in relation to the business logic and functionality of the protocol cannot be understated, as in its current configuration any swaps between chains using the anyCall system will fail due to unavailability of gas to pay the implemented/configured fees from the source chain.

While current tests using the anyCall are currently passing using the test suite provided by the project team, this is due to the tests using mocked calls with pre-computed values rather than the actual implementations of anyCall.

It will be demonstrated below with a test that uses deployed anyCall protocol contracts, how the current implementation will fail due to incorrect flag paramters being set.

Proof of Concept

Please see the anyCall v7 integration and best practice documentation here: https://anycall.gitbook.io/anycall/the-latest-version/v7/pay-fees-on-destination-chain

For background,the the function definition for anyCall is as follows:

function anyCall( address _to, bytes calldata _data, uint256 _toChainID, uint256 _flags, bytes calldata )

_flags configures the fallback and fees settings. The are:

0: Gas fee paid on source chain. Fallback not allowed.
2: Gas fee paid on destination chain. Fallback not allowed.
4: Gas fee paid on source chain. Allow fallback
6: Gas fee paid on destination chain. Allow fallback

It can be observed below in the _performCall function within BranchBridgeAgent.sol that executes using the anycallFlags.sol library the contract is configured with `FLAG_ALLOW_FALLBACK = 0x1 << 2' which corresponds with flag 4 in the anyCall documentation to pay fees on the source chain rather than the destination chain.

function _performCall(bytes memory _calldata) internal virtual {
        //Sends message to AnycallProxy
        IAnycallProxy(localAnyCallAddress).anyCall(
            rootBridgeAgentAddress, _calldata, rootChainId, AnycallFlags.FLAG_ALLOW_FALLBACK, ""
        );
    }

Furthermore, the code implentation required to pay fees from the source chain has not been included within the _performCall function. This is likely due to the project team intended the fee structure to be pay on destination chain.

We can see from actual anyCall protocol source (from Anycallv7Upgradeable.sol), anyCall has the invariant of a base source fee, which is not satisfied by the Ulysses Omnichain protocol:

function anyCall( address _to, bytes calldata _data, uint256 _toChainID, uint256 _flags, bytes calldata ) {
    //...

    (string memory _appID, uint256 _srcFees) = IAnycallConfig(config)
        .checkCall(msg.sender, _data, _toChainID, _flags);

    // _srcFees is calculated as roughly: baseFees + _dataLength * feesPerByte
    // for example, a call to BNB chain has a base fee of 1e16 wei (0.01 ether)
    // and a fee-per-byte of 1e10 wei
    _paySrcFees(_srcFees);
    
    //...
}

function _paySrcFees(uint256 fees) internal {
    // fees always > 0, but msg.value is always 0 coming from BranchBridgeAgent::_performCall
    require(msg.value >= fees, "no enough src fee");
    
    // ...
}

The code below tests the exact manner that the ulysses-omnichain protocol calls anyCall, using the forking functionality of foundry to test against actual instances of the anyCall protocol contracts. This proves that in its current implementation, a live, deployed instance of the ulysses-omnichain protocol will fail to send transactions across the anyCall SMPC network.

// @note This is intended to be run in the test/ulysses-omnichain/ folder
// @note for full output run this with `GOERLI_RPC_URL=<alchemy/infura rpc url> forge test --match-contract TestActualOmnichain -vvvvv`
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";
import "forge-std/Vm.sol";

import {IAnycallProxy} from "../../src/ulysses-omnichain/interfaces/IAnycallProxy.sol";
import {IAnycallConfig} from "../../src/ulysses-omnichain/interfaces/IAnycallConfig.sol";

contract TestActualOmnichain is Test {
    /////////////////////////////////////////////////////////////////////////////////////////////////////
    // @note GOERLI_RPC_URL *must* be set on the command line for this to fork goerli and run correctly!!
    /////////////////////////////////////////////////////////////////////////////////////////////////////
    string GOERLI_RPC_URL = vm.envString("GOERLI_RPC_URL");
    
    uint256 fork;
    address anyCallProxyAddress = 0x965f84D915a9eFa2dD81b653e3AE736555d945f4; // via https://anycall.gitbook.io/anycall/the-latest-version/v7/how-to-integrate-anycall-v7
    address anyCallAddress = 0x7D05D38e6109A3AeEEBf0a570eb8F6856cb4b55E; // via etherscan
    address anyCallConfigAddress = 0x7EA2be2df7BA6E54B1A9C70676f668455E329d29; // via AnyCall.config(), via etherscan
    address anyCallAdmin = 0xfA9dA51631268A30Ec3DDd1CcBf46c65FAD99251; // from AnyCallConfig.admins[0], via etherscan
    
    IAnycallProxy ac = IAnycallProxy(anyCallProxyAddress);
    IAnycallConfig acc = IAnycallConfig(anyCallConfigAddress);
    
    function setUp() public {
        fork = vm.createSelectFork(GOERLI_RPC_URL);
        vm.label(anyCallProxyAddress, "anyCallProxy");
        vm.label(anyCallAddress, "anyCall");
        vm.label(anyCallConfigAddress, "anyCallConfig");
    }
    
    function test_runAnycallOnFork() public {
        address testAddress = address(this);
        address[] memory whitelist = new address[](1);
        whitelist[0] = testAddress;
        
        console.log("Test address", testAddress);

        // whitelist this test so we can make calls to anyCall
        vm.startPrank(anyCallAdmin);
        acc.initAppConfig("testOmnichain", testAddress, testAddress, 0x0, whitelist);
        vm.stopPrank();

        // call AnyCall.anyCall
        // expect request to fail w/ "no enough src fee"

        vm.expectRevert("no enough src fee");
        
        /////////////////////////////////////////////////////////////////////////////
        /////////////////////////////////////////////////////////////////////////////
        // Below is how BranchBridgeAgent.sol calls anyCall, and serves as an example 
        // to prove that the scheme used in the codebase will not work as intended
        /////////////////////////////////////////////////////////////////////////////
        /////////////////////////////////////////////////////////////////////////////

        IAnycallProxy(anyCallProxyAddress).anyCall(
            address(0), // not important, no 0 checks, won't event get a chance to send to addr
            new bytes(0), // not important, won't event get a chance to send
            97, // BNB test chain (has anyCall contracts), won't get a chance to send
            0x4, // pay on src
            "" // unused internally
        );
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Any transaction with anyCall needs to satisfy the base source chain execution fee and a flag update to implement the Pay Fees Destination Chain model that we believe the project team intended.

A code update to _performCall could be:

/** 
 * Sends message to AnycallProxy
 * @dev includes base execution fee and is configured to pay final execution fees on destination
 */
function _performCall(bytes memory _calldata, uint baseExecutionFee) internal virtual {
    IAnycallProxy(localAnyCallAddress).anyCall{value: baseSrcFees}(
        rootBridgeAgentAddress, _calldata, rootChainId, AnycallFlags.FLAG_ALLOW_FALLBACK_DST, ""
    );
}

Assessed type

DoS

[c4-judge]

trust1995 marked the issue as duplicate of #91

[c4-judge]

trust1995 marked the issue as satisfactory