Dopex - mert_eren's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L359-L360 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L199-L205

Vulnerability details

Impact

Settle function used for sell options(take weth and give rdpx) in strike price to PerpetualAtlanticVaultLP.sol when strike price is higher than current price. Settle function call substractLoss after take colleteralToken from PerpetualAtlanticVaultLP.sol.

  function settle(
    uint256[] memory optionIds
  )
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
  {
    _whenNotPaused();
    _isEligibleSender();


    updateFunding();


    for (uint256 i = 0; i < optionIds.length; i++) {
      uint256 strike = optionPositions[optionIds[i]].strike;
      uint256 amount = optionPositions[optionIds[i]].amount;


      // check if strike is ITM
      _validate(strike >= getUnderlyingPrice(), 7);


      ethAmount += (amount * strike) / 1e8;
      rdpxAmount += amount;
      optionsPerStrike[strike] -= amount;
      totalActiveOptions -= amount;


      // Burn option tokens from user
      _burn(optionIds[i]);


      optionPositions[optionIds[i]].strike = 0;
    }


    // Transfer collateral token from perpetual vault to rdpx rdpxV2Core
    collateralToken.safeTransferFrom(//@audit take collateral from perpetualAtlanticVaultLP ethAmount
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );
    // Transfer rdpx from rdpx rdpxV2Core to perpetual vault
    IERC20WithBurn(addresses.rdpx).safeTransferFrom(
      addresses.rdpxV2Core,
      addresses.perpetualAtlanticVaultLP,
      rdpxAmount
    );


    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).subtractLoss(//@audit call substractLoss with ethAmount parameter.
      ethAmount
    );
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP)
      .unlockLiquidity(ethAmount);
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).addRdpx(
      rdpxAmount
    );


    emit Settle(ethAmount, rdpxAmount, optionIds);
  }

  function subtractLoss(uint256 loss) public onlyPerpVault {
    require(
      collateral.balanceOf(address(this)) == _totalCollateral - loss,
      "Not enough collateral was sent out"
    );
    _totalCollateral -= loss;
  }

As can be seen above collateral balance should be equal to _totalCollateral-loss. However due to strict equality, a malicious user can send 1 wei of WETH(collateral token) to PerpetualAtlanticVaultLP.sol to revert substractLoss so settle function of PerpetualAtlanticVault.sol.

Proof of Concept

After protocol operations, _totalCollateral and weth.balanceOf(PerpetualAtlanticVaultLP) change same amount. Moreover, it's essential to note that there's no built-in mechanism within PerpetualAtlanticVaultLP.sol that would allow the administrator to fix this issue.Admin cannot fix this issue because there is no permission for admin to tranfer token from PerpetualAtlanticVaultLP.sol without change _totalCollateral.

In simpler terms, even if 1 wei is transferred to the contract without using any function in PerpetualAtlanticVaultLP.sol, the condition weth.balanceOf - _totalCollateral != 0 can occur, and even the admin cannot rectify this situation, causing subtractLoss to revert.So settle function DOSed forever.

Tools Used

Manuel review

Recommended Mitigation Steps

Dont use strict equality in substractLoss like addProceed function.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

reLp function use uniswap rdpx-weth pool swap() function for take rdpx and give weth. Before call swap calculate minimum rdpx token for slippage protection however it calculate wrongly and slippage can be too high if rdpx/weth price too big than 1e8 or too low if rdpx/weth price is too less than 1e8.

Proof of Concept

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


    uint256 tokenAAmountOut = IUniswapV2Router(addresses.ammRouter)
      .swapExactTokensForTokens(
        amountB / 2,
        mintokenAAmount,
        path,
        address(this),
        block.timestamp + 10
      )[path.length - 1];

TokenA used for rdpx and token b used for weth. This code piece in the reLp() function and tokenAprice is rdpx price denomianted in ethereum with 8 decimal.

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

The problem is price used wrong when calculate minTokenAAmount.Becuase tokenAAmount.tokenAprice gives tokenB value.However tokenBAmount.tokenAprice will not give tokenAAmount. It can be understood better with an example: 1)Assume 1 rdpx's price equals to 2 weth so tokenAPrice will be equal to 2e8. 2)Assume slipageTolerance will be 1e7(%10). 3)Assume in reLp function amountB/2 equals to 0.1 ether. 4)mintokenAAmount will be equal to 18e16(0.18 ether) when calculation made with this parameters. 5)However when we give 0.1 ether weth to this pool we expect to take approximately 0.005 ether rdpx with this rdpx/weth price but slippage protection expect to take at least 0.18 ether so reLp will revert due to too high slippage. In reverse scenario if rdpx/weth price less than 1e8 than slippage will be low than expected and mev bots can make sandwich attack.

Tools Used

manuel review

Recommended Mitigation Steps

use

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

when calculate minTokenAAmount.

Assessed type

Uniswap

Lines of code

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

Vulnerability details

Impact

In the reLp function, following the usage of Uniswap functions swap and addLiquidity, a small amount of WETH (Wrapped Ethereum) remains trapped within the reLP contract. This stranded WETH cannot be moved anywhere else and remains locked indefinitely. Although this may seem insignificant for a single transaction, the contract will be used extensively, potentially accumulating a substantial amount of stuck WETH over time, possibly becoming a significant issue after some time passed from the protocol's launch.

Proof of Concept

  function testReLpContract() public {
    testV2Amo();

    // set address in reLP contract and grant role
    reLpContract.setAddresses(
      address(rdpx),
      address(weth),
      address(pair),
      address(rdpxV2Core),
      address(rdpxReserveContract),
      address(uniV2LiquidityAMO),
      address(rdpxPriceOracle),
      address(factory),
      address(router)
    );
    reLpContract.grantRole(reLpContract.RDPXV2CORE_ROLE(), address(rdpxV2Core));

    reLpContract.setreLpFactor(9e4);

    // add liquidity
    uniV2LiquidityAMO.addLiquidity(5e18, 1e18, 0, 0);
    uniV2LiquidityAMO.approveContractToSpend(
      address(pair),
      address(reLpContract),
      type(uint256).max
    );

    rdpxV2Core.setIsreLP(true);
    assertEq(weth.balanceOf(address(reLpContract)),0);// added by auditor no weth before bond used(so reLP)
    rdpxV2Core.bond(1 * 1e18, 0, address(this));
    uint256 lpBalance2 = pair.balanceOf(address(uniV2LiquidityAMO));
    uint256 rdpxBalance2 = rdpx.balanceOf(address(rdpxV2Core));
    assertEq(weth.balanceOf(address(reLpContract)),1022407518327480);// added by auditor remain weth after bond used(so reLP)
    uint256 reLpRdpxBalance = rdpx.balanceOf(address(reLpContract));
    uint256 reLpLpBalance = pair.balanceOf(address(reLpContract));
    assertEq(lpBalance2, 1422170988183415261);
    assertEq(rdpxBalance2, 53886041379169834724);
    assertEq(reLpRdpxBalance, 0);
    assertEq(reLpLpBalance, 0);

  }

this was actually original test which in tests/rdpxV2-core/Periphery.t.sol. I just added commented lines. This shows that weth stuck in reLp token after rdpxV2Core.bond used(this function calls reLPContract.reLP function.). Weth stucked because in reLP take amountB WETH with calling uniswap.removeLiqudity.And use half of them for taking token with use uniswap.swap and use other half for addLiqudity.

    uint256 tokenAAmountOut = IUniswapV2Router(addresses.ammRouter)
      .swapExactTokensForTokens(
        amountB / 2,
        mintokenAAmount,
        path,
        address(this),
        block.timestamp + 10
      )[path.length - 1];


    (, , uint256 lp) = IUniswapV2Router(addresses.ammRouter).addLiquidity(
      addresses.tokenA,
      addresses.tokenB,
      tokenAAmountOut,
      amountB / 2,
      0,
      0,
      address(this),
      block.timestamp + 10
    );

The issue arises from the fact that the addLiquidity function consumes less than amountB/2 WETH, while swapExactTokensForTokens consumes exactly amountB/2 WETH. The protocol takes this action to obtain tokens. Due to the enforcement of balanced amounts in the addLiquidity function, one of the amounts (WETH or the token) is adjusted to match the desired ratio. If tokenAAmountOut / (amountB / 2) is less than reserveA / reserveB, the protocol takes less than amountB/2, leaving WETH stranded. This situation occurs because tokenAAmountOut / (amountB / 2) is approximately equal to (0.97) * reserveA / ReserveB, which is clearly less than reserveA / ReserveB. The ratio of reserveA / ReserveB decreases after the swap, but due to the large liquidity in the pool compared to the swap amount, this decrease is much smaller than 0.03.

Tools Used

Recommended Mitigation Steps

Consider adding extra functionality for administrators to recover the stuck WETH from the contract. This would allow for the resolution of the issue by authorized parties.

Assessed type

Invalid Validation