_curveSwap: getDpxEthPrice and getEthPrice is in wrong order

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L545

Vulnerability details

Impact

When _curveSwap is called with minAmount = 0 (upperDepeg/lowerDepeg use the default slippage 0.5%), the minOut will be a wrong value which leads to slippage protect failure.

Proof of Concept

In _curveSwap, the getDpxEthPrice and getEthPrice is in wrong order:

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

When _ethToDpxEth is true, we swap eth for dpxeth. However, we use getDpxEthPrice, which is dpxeth's price in eth (dpxeth/eth). Also, when we swap dpxeth for eth, we use getEthPrice which is eth's price in dpxeth.

Below is a PoC that demonstrates we get the wrong slippage when calling upperDepeg with default slippage tolerance:

// add this to _curveSwap
    amountOut = dpxEthCurvePool.exchange(
      _ethToDpxEth ? int128(int256(a)) : int128(int256(b)),
      _ethToDpxEth ? int128(int256(b)) : int128(int256(a)),
      _amount,
      minAmount > 0 ? minAmount : minOut
    );
    if (!_ethToDpxEth) {
      console.log("getDpxEthPrice = %s > 1e8, swap %s dpxEth for Eth (0.5% slippage):", getDpxEthPrice(), _amount);
      console.log("minOut (wrong slippage) = \t%s", minOut);
      uint256 ideal_Out = (_amount * getDpxEthPrice()) / 1e8;
      uint256 correct_minOut = (ideal_Out - (((_amount * getDpxEthPrice()) * slippageTolerance) / 1e16));
      console.log("minOut (correct slippage) = \t%s", correct_minOut);
      console.log("Actual got Eth amountOut = \t%s", amountOut);
      console.log("Actual slippage = \t\t%s% > 0.5%", (ideal_Out - amountOut)*1e2/ideal_Out);
    }

// tests/rdpxV2-core/Unit.t.sol
function testUpperDepeg_default_slippageTolerance() public {
    // swap weth for dpxETH (price after swap 180000000)
    dpxETH.approve(address(curvePool), 200 * 1e18);
    address coin0 = IStableSwap(curvePool).coins(0);
    if (coin0 == address(weth)) {
      IStableSwap(curvePool).exchange(0, 1, 200 * 1e18, 0);
    } else {
      IStableSwap(curvePool).exchange(1, 0, 200 * 1e18, 0);
    }
    // dpxEth : Eth = 1.8 : 1
    dpxEthPriceOracle.updateDpxEthPrice(180000000);

    // mint dpxEth and swap for Eth, using default slippage (0.5%)
    rdpxV2Core.upperDepeg(10e18, 0);
}

Output:

$ forge test -vv --match-test "testUpperDepeg_default_slippageTolerance"
Running 1 test for tests/rdpxV2-core/Unit.t.sol:Unit
[PASS] testUpperDepeg_default_slippageTolerance() (gas: 246264)
Logs:
  getDpxEthPrice = 180000000 > 1e8, swap 10000000000000000000 dpxEth for Eth (0.5% slippage):
  minOut (wrong slippage) =     5527777722500000000
  minOut (correct slippage) =   17910000000000000000
  Actual got Eth amountOut =    15885460869832720611
  Actual slippage =             11% > 0.5%

Tools Used

Manual Review.

Recommended Mitigation Steps

Reverse the order:

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

Assessed type

Context

[c4-pre-sort]

bytes032 marked the issue as duplicate of #2172

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as duplicate of #970

[GalloDaSballo]

Best POC

[c4-judge]

GalloDaSballo marked the issue as selected for report