code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

Incorrect slippage calculation in `_curveswap` function #2164

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

swaps will fail or execute with higher slippage than intended.

Proof of Concept

In _curveSwap() function of RdpxV2Core contract , getEthPrice() should be used in place of getDpxEthPrice(), and getDpxEthPrice() should be used in place of getEthPrice().


/**
   * @notice Returns the price of dpxETH against ETH
   * @dev    Price is in 1e8 Precision
   * @return dpxEthPrice dpxETH price
   **/
  function getDpxEthPrice() public view returns (uint256 dpxEthPrice) {
    return
      IDpxEthOracle(pricingOracleAddresses.dpxEthPriceOracle)
        .getDpxEthPriceInEth();
  }

getDpxEthPrice function returns the price of 1 dpxETH in ETH.


/**
   * @notice Returns the price of ETH token against the DpxEth
   * @dev    Price is in 1e8 Precision
   * @return ethPrice ethToken price
   **/
  function getEthPrice() public view returns (uint256 ethPrice) {
    return
      IDpxEthOracle(pricingOracleAddresses.dpxEthPriceOracle)
        .getEthPriceInDpxEth();
  }

getEthPrice returns the price of 1 ETH in dpxETH.

lets say 1 ETH = 2 dpxETH so 1 dpxETH = 0.5 ETH

So getEthPrice function returns 2 because 1 ETH = 2 dpxETH. And getDpxEthPrice function returns 0.5 because 1 dpxETH = 0.5 ETH.

If we try to swap 1 ETH into dpxETH the minAmount should represent how much dpxETH we should get so it should be around 2 with a slippage of 0.5 percent.


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

But if we calculate minOut according above code

 minOut = 1 * getDpxEthPrice - 1 * getDpxEthPrice * (0.5/100) 
        = (1 * 0.5)  - (1 * 0.5 * 0.5/100)
        = 0.4975

avoiding all the decimals hell and considering slippage as 0.5

Here we are getting minOut as 0.4975 instead of getting a value around 2. If the slippage calculation is correct we should get a value around 2 because current market value of 1 ETH is 2 dpxETH.So if we are swapping from 1 ETH to dpxETH minOut should be around 2 with 0.5% slippage.

Tools Used

Manual Review

Recommended Mitigation Steps


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

Assessed type

Math

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #2172

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #970

c4-judge commented 11 months ago

GalloDaSballo marked the issue as satisfactory