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

3 stars 3 forks source link

`mintokenAAmount` calculation uses the wrong oracle price resulting in no slippage protection in `IUniswapV2Router::swapExactTokensForTokens` #1400

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L273

Vulnerability details

Impact

In line 273 of ReLPContract, minTokenBAmount is calculated as:

    mintokenAAmount =
      (((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
      (((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);

In the above, tokenAInfo.tokenAPrice is calculated in Line 221 as:

    tokenAInfo.tokenAPrice = IRdpxEthOracle(addresses.rdpxOracle)
      .getRdpxPriceInEth();

To convert tokenB to tokenA, the inverse of tokenAPrice needs to be used. Formula above would give a negligible value for mintokenAAmount causing a huge slippage loss on tokenA (rDPX).

Proof of Concept

In the above case, tokenA corresponds to RDPX and tokenB corresponds to WETH. If we assume RDPX as $20 and ETH as $2000, rdpx:ETH = 0.01, ie 1 RDPX = 0.01 ETH. If amountB/2 is 1 ETH, then minTokenAAmount gives a value of 0.01 instead of 100 (for simplicity, I assume slippageTolerance=0). This would mean that this swap effectively has no slippage protection and bots can frontrun this transaction to cause significant slippage on RDPX.

Tools Used

Manual

Recommended Mitigation Steps

Recommend the following changes:

   uint256 tokenBPrice = IRdpxEthOracle(addresses.rdpxOracle)
      .getEthPriceInRdpx();

    mintokenAAmount =
      (((amountB / 2) * tokenBPrice) / 1e8) -
      (((amountB / 2) * tokenBPrice * slippageTolerance) / 1e16);

Assessed type

Uniswap

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #1805

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-judge commented 1 year ago

GalloDaSballo changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory