code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L524-L541

Vulnerability details

impact

In the _curveSwap function, there is a crucial condition check to determine the values of a and b. If coin0 is equal to weth, the values should be set as (0, 1) or (a, b). If not, they should be set as (b, a). However, there is a discrepancy in the way these values are assigned, leading to a misidentification of the assets. This affects the calculation within the _curveSwap function, causing incorrect balance adjustments.

proof of concepts

the _curveSwap is as follow:

function _curveSwap(
        uint256 _amount,
        bool _ethToDpxEth,
        bool validate,
        uint256 minAmount
    ) internal returns (uint256 amountOut) {
        IStableSwap dpxEthCurvePool = IStableSwap(addresses.dpxEthCurvePool);

        // First compute a reverse swapping of dpxETH to ETH to compute the amount of ETH required
        address coin0 = dpxEthCurvePool.coins(0);
        (uint256 a, uint256 b) = coin0 == weth ? (0, 1) : (1, 0);

        // validate the swap for peg functions
        if (validate) {
            //@audit what if a was b and b was a ?! should say if (a == coin0 == weth)
            uint256 ethBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(a);
            uint256 dpxEthBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(b);
            _ethToDpxEth
                ? _validate(
                    ethBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                )
                : _validate(
                    dpxEthBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                );
        }

        // calculate minimum amount out
        uint256 minOut = _ethToDpxEth
            ? (((_amount * getDpxEthPrice()) / 1e8) -
                (((_amount * getDpxEthPrice()) * slippageTolerance) / 1e16))
            : (((_amount * getEthPrice()) / 1e8) -
                (((_amount * getEthPrice()) * slippageTolerance) / 1e16));

        // Swap the tokens
        amountOut = dpxEthCurvePool.exchange(
            _ethToDpxEth ? int128(int256(a)) : int128(int256(b)),
            _ethToDpxEth ? int128(int256(b)) : int128(int256(a)),
            _amount,
            minAmount > 0 ? minAmount : minOut
        );
    }

The issue arises because the condition (a == coin0 == weth) is not appropriately checked. This affects the balance calculations within the _curveSwap function.

Tools used

manual review

recommendation

recommend to add check as the follow:

function _curveSwap(
        uint256 _amount,
        bool _ethToDpxEth,
        bool validate,
        uint256 minAmount
    ) internal returns (uint256 amountOut) {
       ...
       if(a = coin0 == weth) {
         uint256 ethBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(a);
            uint256 dpxEthBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(b);
            _ethToDpxEth
                ? _validate(
                    ethBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                )
                : _validate(
                    dpxEthBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                );
       } else{
         uint256 ethBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(b);
            uint256 dpxEthBalance = IStableSwap(addresses.dpxEthCurvePool)
                .balances(a);
            _ethToDpxEth
                ? _validate(
                    ethBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                )
                : _validate(
                    dpxEthBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
                    14
                );
       }
    }