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

20 stars 12 forks source link

Attacker can redeposit gas after "forceRevert()" to freeze all deposited gas budget of Root Bridge Agent #337

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/RootBridgeAgent.sol#L1233-L1241

Vulnerability details

Impact

forceRevert() withdraws all deposited gas budget of Root Bridge Agent to ensure that failed AnyCall execution will not be charged. However, if forceRevert() took place during a call made by virtual account, gas can be replenished later manually. As a result, the AnyCall execution will succeed but all withdrawn gas will be frozen.

Proof of Concept

function anyExecute(bytes calldata data)
    external
    virtual
    requiresExecutor
    returns (bool success, bytes memory result)
{
    uint256 _initialGas = gasleft();
    uint24 fromChainId;
    UserFeeInfo memory _userFeeInfo;

    if (localAnyCallExecutorAddress == msg.sender) {
        initialGas = _initialGas;
        (, uint256 _fromChainId) = _getContext();
        fromChainId = _fromChainId.toUint24();

        _userFeeInfo.depositedGas = _gasSwapIn(
            uint256(uint128(bytes16(data[data.length - PARAMS_GAS_IN:data.length - PARAMS_GAS_OUT]))), fromChainId).toUint128();
        _userFeeInfo.gasToBridgeOut = uint128(bytes16(data[data.length - PARAMS_GAS_OUT:data.length]));
    } else {
        fromChainId = localChainId;
        _userFeeInfo.depositedGas = uint128(bytes16(data[data.length - 32:data.length - 16]));
        _userFeeInfo.gasToBridgeOut = _userFeeInfo.depositedGas;
    }

    if (_userFeeInfo.depositedGas < _userFeeInfo.gasToBridgeOut) {
        _forceRevert();
        return (true, "Not enough gas to bridge out");
    }

    userFeeInfo = _userFeeInfo;

    // execution part
    ............

    if (initialGas > 0) {
        _payExecutionGas(userFeeInfo.depositedGas, userFeeInfo.gasToBridgeOut, _initialGas, fromChainId);
    }
}

To implement the attack, attacker callOutSigned on a branch chain to bypass lock. On the root chain, virtual account makes three external calls.

  1. retryDeposit at Arbitrum Branch Bridge Agent with an already executed nonce. The call will forceRevert() and initialGas will be nonzero since it has not been modified by reentering. As a result, all execution gas budget will be withdrawn.

    function _forceRevert() internal {
    if (initialGas == 0) revert GasErrorOrRepeatedTx();
    
    IAnycallConfig anycallConfig = IAnycallConfig(IAnycallProxy(localAnyCallAddress).config());
    uint256 executionBudget = anycallConfig.executionBudget(address(this));
    
    // Withdraw all execution gas budget from anycall for tx to revert with "no enough budget"
    if (executionBudget > 0) try anycallConfig.withdraw(executionBudget) {} catch {}
    }
  2. callOut at Arbitrum Branch Bridge Agent. The call should succeed and initialGas is deleted.

    function _payExecutionGas(uint128 _depositedGas, uint128 _gasToBridgeOut, uint256 _initialGas, uint24 _fromChain) internal {
    delete(initialGas);
    delete(userFeeInfo);
    
    if (_fromChain == localChainId) return;
  3. Directly deposit a small amount of gas at Anycall Config to ensure the success of the transaction.
    function deposit(address _account) external payable {
    executionBudget[_account] += msg.value;
    emit Deposit(_account, msg.value);
    }

    Then, the original call proceeds and _payExecutionGas will be skipped. The call will succeed, with all withdrawn gas budget permanently frozen. (In current implementation, ETH can be sweeped to dao address, but this is another mistake, sweep should transfer WETH instead.)

Tools Used

Manual

Recommended Mitigation Steps

Add a msg.sender check in _forceRevert to ensure local call will be directly reverted.

Assessed type

Reentrancy

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

peakbolt commented 1 year ago

This is an interesting attack vector. However, the impact seems like a Medium as the attack cost could be higher than the frozen execution gas budget, lowering the incentive for such an attack.That is because the attacker has to pay the tx cost and also deposit gas to the AnycallConfig for the attack to succeed. And the execution gas budget in RootBridgeAgent is likely negligible as it is intended to be replenished by the user.

xuwinnie commented 1 year ago

This is an interesting attack vector. However, the impact seems like a Medium as the attack cost could be higher than the frozen execution gas budget, lowering the incentive for such an attack.That is because the attacker has to pay the tx cost and also deposit gas to the AnycallConfig for the attack to succeed. And the execution gas budget in RootBridgeAgent is likely negligible as it is intended to be replenished by the user.

Hey @peakbolt! Actually, it could DOS the entire cross chain message sending. "If the gas fee isn't enough when you call anycall, the tx wouldn't execute until you top up with enough gas fees. This status would be reflected in the api." according to anycall V7 doc. (rip multichain) If root bridge agent has zero budget, tx will not execute, but no user is incentivized to top it up manually. The system heavily relies on the pre-deposited gas. To make it clearer, Suppose when deploying, team top up 5 unit gas. A user's tx cost 1 unit gas, then 1 unit gas is replenished. However, if the 5 unit gas is removed, the tx won't execute at all!

0xBugsy commented 1 year ago

This is an interesting attack vector. However, the impact seems like a Medium as the attack cost could be higher than the frozen execution gas budget, lowering the incentive for such an attack.That is because the attacker has to pay the tx cost and also deposit gas to the AnycallConfig for the attack to succeed. And the execution gas budget in RootBridgeAgent is likely negligible as it is intended to be replenished by the user.

Hey @peakbolt! Actually, it could DOS the entire cross chain message sending. "If the gas fee isn't enough when you call anycall, the tx wouldn't execute until you top up with enough gas fees. This status would be reflected in the api." according to anycall V7 doc. (rip multichain) If root bridge agent has zero budget, tx will not execute, but no user is incentivized to top it up manually. The system heavily relies on the pre-deposited gas. To make it clearer, Suppose when deploying, team top up 5 unit gas. A user's tx cost 1 unit gas, then 1 unit gas is replenished. However, if the 5 unit gas is removed, the tx won't execute at all!

System should execute tx as long as executionBudget is > 0, but you are correct if this value reaches 0 execution will be stopped until gas is topped up and this can be continuously depleted which is completely undesired.

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.