Dopex - wintermute's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L145-L175

Vulnerability details

Rogue PerpetualAtlanticVault LPs can avoid loss of funds by sandwiching PerpetualAtlanticVault PUT options settlement by redeem and then deposit. Problem is - LPs can immediately redeem assets, deposited into risk vault. By doing so, they can avoid that risk.

Impact

Rogue LPs get risk-free income from funding and options purchase premium. Other LPs suffer greater loss.

Proof of Concept

If attacker see that asset price dropped and options can now be settled, he simply redeems his funds from vault and not incurs any loss. He cat easely do it: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L145-L175 He can use advanced tehnique like bot that monitoring tx pool and sandwitches every option settlement tx: redeem => settle => deposit.

Tools Used

Manual Review

Recommended Mitigation Steps

Add withdraw timelock

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L200-L203

Vulnerability details

Attacker can break APP options settlement by sending 1 wei of WETH to PerpetualAtlanticVaultLP => break require statement in subtractLoss function. It will break PerpetualAtlanticVault.settle function because it internally calls PerpetualAtlanticVaultLP.subtractLoss.

Impact

PerpetualAtlanticVault options settlement will always revert

Proof of Concept

PerpetualAtlanticVaultLP.subtractLoss has the following requirement: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L200-L203 If attacker sends 1 wei to PerpetualAtlanticVaultLP, this require will fail and PerpetualAtlanticVaultLP.subtractLoss will always revert because now contract have one more wei bigger than internal accounting _totalCollateral varialbe and there is no way to sync it. PerpetualAtlanticVaultLP.subtractLoss always called in PerpetualAtlanticVault.settle, breaking it functionality.

Tools Used

Manual Review

Recommended Mitigation Steps

Remove require statement from subtractLoss function

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L964 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L975-L990 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1001-L1003

Vulnerability details

In RdpxV2Core.addToDelegate amount delegated added to total amount delegated: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L964 But if delegate decides to withdraw provided eth, that amount won't be decremented: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L975-L990

Impact

Broken RdpxV2Core reserves accounting: will always revert on RdpxV2Core.sync function, which actively used in reserves accounting.

Proof of Concept

Attacker can repeatedly call addToDelegate and then withdraw => greatly increase totalWethDelegated - making it greater than weth balance. It leads to always revert in sync function here due to underflow: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1001-L1003

Tools Used

Manual Review

Recommended Mitigation Steps

Decrement totalWethDelegated in withdraw function

Assessed type

Under/Overflow

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

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L160-L178

Vulnerability details

AMO functions don't sync RdpxV2Core reserves internal accounting when perform operations.

Impact

RdpxV2Core broken reserves accounting after Uniswap v3 AMO operations. Its can be fixed with RdpxV2Core.sync function call in another tx, but it's not practical and convenient. And still it can affect the protocol if there are txs between AMO operations and RdpxV2Core.sync txs.

Proof of Concept

Uniswap v2 AMO don't call RdpxV2Core.sync at all: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L160-L178 Where as Uniswap v3 AMO do it after every operation: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L361 And in Uniswap v3 AMO when collecting fee: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L119-L133

Tools Used

Manual Review

Recommended Mitigation Steps

Add RdpxV2Core.sync to UniV2LiquidityAMO._sendTokensToRdpxV2Core and UniV3LiquidityAMO.collectFees functions

Assessed type

Uniswap

Lines of code

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

Vulnerability details

ReLPContract when performing relp does not send unused WETH back to rdpxV2Core.

Impact

Unused WETH not transferred back to reserves and stack in ReLPContract contract.

Proof of Concept

Amount add min set to 0 on both tokens: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L286-L295 It means that not all desired A and B tokens can be used. Remainning tokensA are sent back to reserves, but tokenB don't - they are stuck in contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Send back both unused tokens. Add this snippet:

IERC20WithBurn(addresses.tokenB).safeTransfer(
  addresses.rdpxV2Core,
  IERC20WithBurn(addresses.tokenB).balanceOf(address(this))
);

Assessed type

Token-Transfer