Dopex - KrisApostolov's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L315 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L201

Vulnerability details

Impact

Any settle call can be reverted, thus disallowing the protocol from settling any options and from releasing the locked tokens.

Proof of Concept

The protocol admin calls settle() on an option when its price gets bellow the strike price, at which the option is bought at. The issue arises due to a flawed check in PerpetualAtlanticVaultLP.subtractLoss().

require(
	collateral.balanceOf(address(this)) == _totalCollateral - loss,
	"Not enough collateral was sent out"
);

This check makes front-running the transaction by donating 1 WEI for making it revert possible.

Here is a Foundry PoC demonstrating the issue:

https://gist.github.com/CrisCodesCrap/d1e7af6ffec3abfe500949340c418577

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Consider altering the check in the following way:

require(
      collateral.balanceOf(address(this)) >= _totalCollateral - loss,
      "Some revert message here"
    );

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L123-L125

Vulnerability details

Impact

Users can get more shares than they should.

Proof of Concept

Users can get more shares than they paid for because vaultLp.deposit()'s share calculation is done before the contract updates the funding token balances. This makes it so that users can deposit into the vault as if there isn't any new funding, but then update the funding in the line bellow.

// @audit shares get calculated here:
require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

// @audit Funding gets updated here, effectively allowing users to mint shares at old rates, but instantly get more value than they deposited.
perpetualAtlanticVault.updateFunding();

Consider the following PoC demonstrating the issue at hand:

https://gist.github.com/CrisCodesCrap/ab356eb14667dfe83b5ba1155b2ded4c

Tools Used

Manual review + Foundry

Recommended Mitigation Steps

Consider calling vault.updateFunding() before doing the share calculation to fully mitigate the issue.

Here is how it may look like when implemented:

// @audit First: the funding gets updated
perpetualAtlanticVault.updateFunding();

// @audit shares get calculated afterwards to ensure the bug is mitigated
require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L941-L968 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L975-L990 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L995-L1008

Vulnerability details

Impact

The contract's WETH amount gets permanently bricked.

Proof of Concept

A user can call addToDelegate() and give WETH, that other people can use for bonding with their rDPX in exchange for a certain percentage appointed by the delegatee.

Delegate memory delegatePosition = Delegate({
      amount: _amount,
      fee: _fee,
      owner: msg.sender,
      activeCollateral: 0
    });
    delegates.push(delegatePosition);

    // add amount to total weth delegated
    totalWethDelegated += _amount;

    emit LogAddToDelegate(_amount, _fee, delegates.length - 1);
    return (delegates.length - 1);

There the user gets added to the delegates array and also the delegated WETH amount gets added to a variable called totalWethDelegated, which is used for keeping track of the part of WETH in the contract, which is owned by delegates. That variable is also used in sync() for setting the virtual balance of WETH in the contract.

function sync() external {
    for (uint256 i = 1; i < reserveAsset.length; i++) {
      uint256 balance = IERC20WithBurn(reserveAsset[i].tokenAddress).balanceOf(
        address(this)
      );

      if (weth == reserveAsset[i].tokenAddress) {
        balance = balance - totalWethDelegated;
      }
      reserveAsset[i].tokenBalance = balance;
    }

    emit LogSync();
 }

The issue arises due to the WETH amount not being removed from totalWethDelegated upon withdrawal.

function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

		// @audit-issue The delegate entry's amount gets removed, but the amount doesn't get subtracted from totalWethDelegated

    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

Consider the following PoC demonstrating the issue:

https://gist.github.com/CrisCodesCrap/fb5ad3b5a5c95670d2ae44c895b42ab5

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Consider removing the amount upon withdrawing the delegated WETH from the protocol.

function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

		totalWethDelegated -= amountWithdrawn;
    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L286-L295

Vulnerability details

Impact

The amount being LPd into Uniswap can get stolen trough MEV.

Proof of Concept

The reLP contract re-LPs a certain amount of the tokens, that enter after a bond gets bought. The issue arises due to there not being proper minimum liquidity amounts passed when calling addLiquidity() on a Uniswap V2 pool.

(, , uint256 lp) = IUniswapV2Router(addresses.ammRouter).addLiquidity(
      addresses.tokenA,
      addresses.tokenB,
      tokenAAmountOut,
      amountB / 2,
      // @audit-issue lack of slippage checks
      0,
      0,
      address(this),
      block.timestamp + 10
);

Tools Used

Manual review

Recommended Mitigation Steps

Consider passing appropriate minimum liquidity amounts to addLiquidity().

Assessed type

Uniswap

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L289-L299

Vulnerability details

Impact

If the transaction gets stalled in the mempool it can can get executed at an inappropriate moment.

Proof of Concept

The Uniswap V3 AMO integrates Uniswap to provide liquidity and to execute swaps. The issue arises due to the protocol using an arbitrary timetamp in the future instead of an actual one that will protect the protocol from getting damaged due to precision loss.

ISwapRouter.ExactInputSingleParams memory swap_params = ISwapRouter
      .ExactInputSingleParams(
        _tokenA,
        _tokenB,
        _fee_tier,
        address(this),
				// @audit-issue inappropriate timestamp:
        2105300114, // Expiration: a long time from now
        _amountAtoB,
        _amountOutMinimum,
        _sqrtPriceLimitX96
      );

Tools Used

Manual review

Recommended Mitigation Steps

Consider swapping the timestamp with a value passed as params.

Assessed type

Uniswap