RdpxV2Core._curveSwap minOut calculated wrongly when swapping dpxETH for ETH

[code423n4]

Lines of code

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

Vulnerability details

This minOut calculation underestimate assets price:
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L548-L549

Impact

Swap sandwitch attack and big slippage if minAmount = 0 (not set) when calling RdpxV2Core.upperDepeg

Proof of Concept

This executes when _ethToDpxEth = false => swapping dpxETH to ETH => upper depeg (1 eth < 1 dpxETH) => getEthPrice() < 1e8 (return eth price in dpxETH)
Consider extreme case:

_amount = 1e18 (swap in 1 dpxETH)
getEthPrice() = 0.5e8 (1 dpxETH = 2 eth)
slippageTolerance = 5e5

(((_amount * getEthPrice()) / 1e8) -
(((_amount * getEthPrice()) * slippageTolerance) / 1e16))
= 0.4975e18

That is swapping 1 dpxETH to weth with miOut = 0.4975e18, when 1 dpxETH = 2 eth

Tools Used

Manual Review

Recommended Mitigation Steps

Flip price and use insted

(((_amount * 1e8) / getEthPrice()) -
(((_amount * 1e8) * slippageTolerance) / (getEthPrice() * 1e16)))

This gives minOut = 1.99e18, which is correct

Assessed type

Math

[c4-pre-sort]

bytes032 marked the issue as duplicate of #2172

[c4-pre-sort]

bytes032 marked the issue as not a duplicate

[c4-pre-sort]

bytes032 marked the issue as duplicate of #970

[c4-judge]

GalloDaSballo marked the issue as partial-50

[GalloDaSballo]

Partially valid