ERRORNEOUS minOut CALCULATION IN THE RdpxV2Core._curveSwap FUNCTION BREAKS THE EXPECTED BEHAVIOUR OF THE PROTOCOL

[code423n4]

Lines of code

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

Vulnerability details

Impact

The RdpxV2Core._curveSwap function is used to swap token A to token B based on the value of the _ethToDpxEth Boolean variable. _ethToDpxEth value denotes whether swap is from ETH to dpxETH or dpxETH to ETH.

Before the actual dpxEthCurvePool.exchange happens the minOut value is calculated as slippage protection as shown below:

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

If the _ethToDpxEth is true which means the swap is from Eth to dpxEth, the _amount is multiplied by value returned from getDpxEthPrice(). Here the _amount is in Eth and getDpxEthPrice() value is in Eth/DpxEth unit with 1e8 precision. Hence it is clear this calculation is wrong. Because we want to multiply the _amount (Eth value) by the getEthPrice() value and not by the getDpxEthPrice() value. Because getEthPrice() returns the dpxEth/Eth value in 1e8 precision which is the correct value to use to multiply the Eth _amount to obtain the minOut value in dpxEth units.

Similarly if the _ethToDpxEth is false which means the swap is from dpxEth to Eth, the _amount is multiplied by value returned from getEthPrice(). Here the _amount is in dpxEth and getEthPrice() value is in dpxEth/Eth unit with 1e8 precision. Hence it is clear this calculation is wrong. Because we want to multiply the _amount (dpxEth value) by the getDpxEthPrice() value and not by the getEthPrice() value. Because getDpxEthPrice() returns the Eth/dpxEth value in 1e8 precision which is the correct value to use to multiply the dpxEth _amount to obtain the minOut value in Eth units.

Above error in calculating the value of minOut could prompt the dpxEthCurvePool.exchange transaction to revert since we are providing the wrong slippage protection value to the actual swap happening in the exchange transaction.

This will prompt the _curveSwap transaction to behave in unexpected manner thus breaking the protocol unexpectedly.

Proof of Concept

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

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to correct the minOut value in the RdpxV2Core._curveSwap function as shown below:

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

The above change will ensure the correct minOut value is calculated based on the Boolean value of _ethToDpxEth thus ensuring successful execution of the RdpxV2Core._curveSwap transaction.

Assessed type

Other

[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

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)