code-423n4 / 2023-05-maia-findings

24 stars 13 forks source link

User underpay for the remote call execution gas on root chain #612

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L836 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/BranchBridgeAgent.sol#L1066 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/BranchBridgeAgent.sol#L1032 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L811

Vulnerability details

Impact

User underpay for the remote call execution gas, meaning Incorrect minExecCost that being deposited at _replenishGas call inside _payExecutionGas function.

Proof of Concept

Multi chain contracts - anycall v7 lines https://github.com/anyswap/multichain-smart-contracts/blob/645d0053d22ed63005b9414b5610879094932304/contracts/anycall/v7/AnycallV7Upgradeable.sol#L265

https://github.com/anyswap/multichain-smart-contracts/blob/645d0053d22ed63005b9414b5610879094932304/contracts/anycall/v7/AnycallV7Upgradeable.sol#L167

https://github.com/anyswap/multichain-smart-contracts/blob/645d0053d22ed63005b9414b5610879094932304/contracts/anycall/v7/AnycallV7Upgradeable.sol#L276

ulysses-omnichain contract lines https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L811

https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L851

The user is paying incorrect minimum execution cost for Anycall Mutlichain L820, the value of minExecCost is calculated incorrectly. AnycallV7 protocol considers a premium fee _feeData.premium on top of the TX gas price which is not considered here.

let's get into the flow from the start, the anyExec call that being called by the executer L265 when an anycall request comes from a source chain includes chargeDestFee modifier

    function anyExec(
        address _to,
        bytes calldata _data,
        string calldata _appID,
        RequestContext calldata _ctx,
        bytes calldata _extdata
    )
        external
        virtual
        lock
        whenNotPaused
        chargeDestFee(_to, _ctx.flags)
        onlyMPC
    {
        IAnycallConfig(config).checkExec(_appID, _ctx.from, _to);

now, chargeDestFee modifier will call chargeFeeOnDestChain function as well at L167

/// @dev Charge an account for execution costs on this chain
/// @param _from The account to charge for execution costs
    modifier chargeDestFee(address _from, uint256 _flags) {
        if (_isSet(_flags, AnycallFlags.FLAG_PAY_FEE_ON_DEST)) {
            uint256 _prevGasLeft = gasleft();
            _;
            IAnycallConfig(config).chargeFeeOnDestChain(_from, _prevGasLeft);
        } else {
            _;
        }
    }

as you see L198-L210, inside chargeFeeOnDestChain function is including_feeData.premium for the execution cost totalCost.

function chargeFeeOnDestChain(address _from, uint256 _prevGasLeft)
        external
        onlyAnycallContract
    {
        if (!_isSet(mode, FREE_MODE)) {
            uint256 gasUsed = _prevGasLeft + EXECUTION_OVERHEAD - gasleft();
            uint256 totalCost = gasUsed * (tx.gasprice + _feeData.premium);
            uint256 budget = executionBudget[_from];
            require(budget > totalCost, "no enough budget");
            executionBudget[_from] = budget - totalCost;
            _feeData.accruedFees += uint128(totalCost);
        }
    }

The conclusion: minExecCost calculation doesn't include _feeData.premium at L811 according to multichain AnycallV7 protocol.

You should include _feeData.premium as well in minExecCost same as L204

uint256 totalCost = gasUsed * (tx.gasprice + _feeData.premium);

Note: This also applicable on: _payFallbackGas() in RootBridgeAgent at L836 _payFallbackGas() in BranchBridgeAgent at L1066 _payExecutionGas in BranchBridgeAgent at L1032

Tools Used

Manual Review

Recommended Mitigation Steps

add _feeData.premium to minExecCost at _payExecutionGas function L811

you need to get _feeData.premium first from AnycallV7Config by premium() function at L286-L288

uint256 minExecCost = (tx.gasprice  + _feeData.premium) * (MIN_EXECUTION_OVERHEAD + _initialGas - gasleft()));

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as primary issue

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-sponsor commented 1 year ago

0xBugsy marked the issue as sponsor confirmed

c4-judge commented 1 year ago

trust1995 marked the issue as selected for report

c4-sponsor commented 1 year ago

0xBugsy marked the issue as sponsor acknowledged

c4-sponsor commented 1 year ago

0xBugsy marked the issue as sponsor confirmed

0xBugsy commented 1 year ago

We recognize the audit's findings on Anycall Gas Management. These will not be rectified due to the upcoming migration of this section to LayerZero.