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

24 stars 13 forks source link

`depositGasAnycallConfig` wrongly withdraws from WrappedNativeToken #305

Closed code423n4 closed 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#L1219 https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L850

Vulnerability details

Impact

RootBridgeAgent.sol#depositGasAnycallConfig can be use to topup gas to contract. https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L1219

function depositGasAnycallConfig() external payable {
        //Deposit Gas
        _replenishGas(msg.value);//@audit
    }

but due to how _replenishGas is implemented in rootBridgeAgent.sol. https://github.com/code-423n4/2023-05-maia/blob/main/src/ulysses-omnichain/RootBridgeAgent.sol#L850

function _replenishGas(uint256 _executionGasSpent) internal {
        //Unwrap Gas
        wrappedNativeToken.withdraw(_executionGasSpent);//@audit
        IAnycallConfig(IAnycallProxy(localAnyCallAddress).config()).deposit{value: _executionGasSpent}(address(this));
    }

it wrongly withdraw from WrappedNativeToken before deposit, but this value is sent directly following the call to depositGasAnycallConfig and shouldn't be unwrapped first. The unwrapped eth would be unaccounted for and remains within the contract.

Proof of Concept

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

Tools Used

Manuel Review

Recommended Mitigation Steps

depositGasAnycallConfig should not withdraw from WrappedNativeToken, you can implement _replenishGas as in BranchBridgeAgent.sol and unwrap tokens seperately before calling _replenishGas.

Assessed type

Context

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 issue #679 as primary and marked this issue as a duplicate of 679