Dopex - SpicyMeatball's results

Lines of code

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

Vulnerability details

Impact

It is impossible to settle an option if condition below is not met

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

value on the left is the WETH balance of the LP. This condition can easily be violated with a small WETH donation to the contract.

Proof of Concept

Test case for the tests/perp-vault/Unit.t.sol

  function testSettle() public {
    weth.mint(address(1), 1 ether);
    deposit(1 ether, address(1));

    vault.purchase(1 ether, address(this));

    uint256[] memory ids = new uint256[](1);
    ids[0] = 0;

    priceOracle.updateRdpxPrice(0.2 gwei); // initial price * 10
    uint256[] memory strikes = new uint256[](1);
    strikes[0] = 0.015 gwei;
    skip(86500); // expire

    priceOracle.updateRdpxPrice(0.010 gwei); // ITM
    // Donate 1 wei to the vault LP to DOS the settle function
    weth.transfer(address(vaultLp), 1);
    vm.expectRevert();
    vault.settle(ids);
  }

Tools Used

Foundry

Recommended Mitigation Steps

Perhaps we can refactor this block

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

like this, move the condition to the Vault contract

+   uint256 lpBalanceBefore = collateralToken.balanceOf(addresses.perpetualAtlanticVaultLP); 
    // Transfer collateral token from perpetual vault to rdpx rdpxV2Core
    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );
    // Transfer rdpx from rdpx rdpxV2Core to perpetual vault
    IERC20WithBurn(addresses.rdpx).safeTransferFrom(
      addresses.rdpxV2Core,
      addresses.perpetualAtlanticVaultLP,
      rdpxAmount
    );

+   uint256 lpBalanceAfter = collateralToken.balanceOf(addresses.perpetualAtlanticVaultLP); 
+   require(
+     lpBalanceAfter == _lpBalanceBefore - ethAmount,
+     "Not enough collateral was sent out"
+   );
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).subtractLoss(
      ethAmount
    );
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP)
      .unlockLiquidity(ethAmount);
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).addRdpx(
      rdpxAmount
    );

    emit Settle(ethAmount, rdpxAmount, optionIds);

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Admin can change the _fundingDuration value with the updateFundingDuration function, but when we change it we're not calling the updateFunding function therefore calculations with the previous _fundingDuration may be unaccounted.

Proof of Concept

In this test case in the tests/perp-vault/Unit.t.sol user purchases an option at the moment when the funding pointer is 0, the funding duration is 1 day, thus after 7 days the content of the fundingRates

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

mapping may look like this

FR:  289351851851851851851851851851851 0                                                                                                                                                                                                                        
FR:  0 1                                                                                                                                                                                                                                                      
FR:  0 2                                                                                                                                                                                                                                                       
FR:  0 3                                                                                                                                                                                                                                                       
FR:  0 4                                                                                                                                                                                                                                                        
FR:  0 5                                                                                                                                                                                                                                                        
FR:  0 6                                                                                                                                                                                                                                                        
FR:  0 7  

Suppose a user purchased another option when our pointer is 8. After that another 7 days has passed and the admin decides to change the funding duration to half a day

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

as we can see updateFunding wasn't called before the funding duration change. What if we update the funding pointer after this? Because we decreased the duration, some epochs will be skipped

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

in our example these are epochs from 8 to 15 and as a result funding for the eighth epoch won't be transferred.

  function testPurchasePremiumNotSent() public {
    vault.setLpAllowance(true);
    deposit(100 ether, address(this));
    ( ,uint256 id0) = vault.purchase(500 ether, address(this));
    vm.warp(1662951316 + 86400*7);

    uint256 vaultBalanceBefore = weth.balanceOf(address(vault));
    console.log("BAL BEFORE1: ", vaultBalanceBefore);
    vault.purchase(1 ether, address(this));
    vaultBalanceBefore = weth.balanceOf(address(vault));
    console.log("BAL BEFORE2: ", vaultBalanceBefore);
    vm.warp(block.timestamp + 86400*7);

    vault.updateFundingDuration(43200);
    
    vault.updateFundingPaymentPointer();
    uint256 vaultBalanceAfter = weth.balanceOf(address(vault));
    // we can see that there are some unspent funds at the balance which normally should be transferred
    console.log("BAL AFTER: ", vaultBalanceAfter);
  }

Tools Used

Foundry

Recommended Mitigation Steps

Call updateFunding before changing _fundingDuration

Assessed type

Timing