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

24 stars 13 forks source link

`sweep` should transfer WETH instead of ETH #363

Closed code423n4 closed 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#L1263

Vulnerability details

Impact

Excess gas are accumulated as WETH. However, sweep transfers ETH.

Proof of Concept

function sweep() external {
    if (msg.sender != daoAddress) revert NotDao();
    uint256 _accumulatedFees = accumulatedFees - 1;
    accumulatedFees = 1;
    SafeTransferLib.safeTransferETH(daoAddress, _accumulatedFees);
}

As below, the contract receive WETH at _gasSwapIn. Then, WETH is partially unwrapped at _replenishGas. Excess gas are stored as WETH.

function _gasSwapIn(uint256 _amount, uint24 _fromChain) internal returns (uint256) {
    //Get fromChain's Gas Pool Info
    (bool zeroForOneOnInflow, uint24 priceImpactPercentage, address gasTokenGlobalAddress, address poolAddress) =
        IPort(localPortAddress).getGasPoolInfo(_fromChain);
    ......
    //Swap imbalanced token as long as we haven't used the entire amountSpecified and haven't reached the price limit
    try IUniswapV3Pool(poolAddress).swap(
        address(this),
        zeroForOneOnInflow,
        int256(_amount),
        sqrtPriceLimitX96,
        abi.encode(SwapCallbackData({tokenIn: gasTokenGlobalAddress}))
    ) returns (int256 amount0, int256 amount1) {
        return uint256(zeroForOneOnInflow ? amount1 : amount0);
    } catch (bytes memory) {
        _forceRevert();
        return 0;
    }
}

function _payExecutionGas(uint128 _depositedGas, uint128 _gasToBridgeOut, uint256 _initialGas, uint24 _fromChain) internal {
    ......
    //Replenish Gas
    _replenishGas(minExecCost);

    //Account for excess gas
    accumulatedFees += availableGas - minExecCost;
}

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

Tools Used

Manual

Recommended Mitigation Steps

Transfer WETH in sweep.

Assessed type

Token-Transfer

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #385

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory