Lines of code

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

Vulnerability details

Proof of Concept

The epoch in the protocol is used as the "expiry date" of options and the frequency which fundings are payed to depositors in PerpetualAtlanticVaultLP.sol. The standard epoch duration is set to 7 days in the fundingDuration state variable at PerpetualAtlanticVault.sol.

uint256 public fundingDuration = 7 days;

It can also be changed via updateFundingDuration method in PerpetualAtlanticVault.sol#L237.

  function updateFundingDuration(
    uint256 _fundingDuration
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    fundingDuration = _fundingDuration;
  }

The function nextFundingPaymentTimestamp() calculates the ending of an epoch using the state variable fundingDuration.

  function nextFundingPaymentTimestamp()
    public
    view
    returns (uint256 timestamp)
  {
    return genesis + (latestFundingPaymentPointer * fundingDuration);
  }

The calculation is genesis + (latestFundingPaymentPointer * fundingDuration, which can be translated to genesis + (Counter of Epochs * Duration of an Epoch). It assumes that all epochs have the same duration, which is a mistake:
1. We are in epoch 10 and fundingDuration == 7 days, so nextFundingPaymentDuration() returns genesis + 70 days.
2. Admin changes the fundingDuration to 10 days.
Now nextFundingPaymentDuration() returns genesis + 100 days. However, as an epoch now lasts 10 days, the 30 days difference from one fundingDuration value to another isn't correct.
1. This can cause a range of different impacts accordingly to the new fundingDuration value since important methods updateFundings, updateFundingPaymentPointer and purchase (all from PerpetualAtlanticVault.sol) uses nextFundingPaymentTimestamp in some way.

Impact

First, we need to understand the function that uses the vulnerable nextFundingPaymentTimestamp and is linked to purchase and updateFundings. It's updateFundingPaymentPointer and it's purpose is to pay an epoch when it ends and also update the latestFundingPaymentPointer, which is the counter of epochs. To do that, first it checks if block.timestamp (current time) is higher or equal to nextFundingPaymentTimestamp, so it only updates if the current epoch ended. That's done in a loop way to update even if the counter is 2 epochs outdated.

  function updateFundingPaymentPointer() public {
    // 1 - LOOPS UNTIL CURRENT TIME IS LOWER THAN EPOCH ENDING
    while (block.timestamp >= nextFundingPaymentTimestamp()) {
    /// some code

    // 2 - PAY THE LAST EPOCH BEFORE STARTING A NEW ONE
            collateralToken.safeTransfer(
          addresses.perpetualAtlanticVaultLP,
          (currentFundingRate * (nextFundingPaymentTimestamp() - startTime)) /
            1e18
        );
    /// some code
    // 3 - UPDATE THE CODE TO HANDLE THE CURRENT EPOCH 
    latestFundingPaymentPointer += 1;

Impacts are scenarios of denial of service, wrong funding payments and breaking the correct calculation of nextFundingPaymentTimestamp for an unlimited amount of time:

Scenario of updating fundingDuration to a lower value:
1. fundingDuration = 7 days and latestFundingPaymentPointer - 7 (Counter of Epochs = 7, so we are in epoch 7). With this state variables, nextFundingPaymentTimestamp == genesis + 49 days
2. Admin updates fundingDuration to 5 days via updateFundingDuration.
3. Now, nextFundingPaymentTimestamp == genesis + 35 days. So, even that fundingDuration changed to 5 days, which should only produce a difference of 2 days to the original output, the function is now returning a difference of 14 days, effectively acting like the contract is 2-3 epochs away from the actual epoch.
4. updateFundingPaymentPointer will loop and increment lastFundingPaymentPointer a few times to get updated to make nextFundingPaymentTimestamp get updated again
```
while (block.timestamp >= nextFundingPaymentTimestamp() { 
// ...
        collateralToken.safeTransfer(
          addresses.perpetualAtlanticVaultLP,
          (currentFundingRate * (nextFundingPaymentTimestamp() - startTime)) /
            1e18
        );
// ... 
lastFundingPaymentPointer += 1; 
}
```
5. Now that lastFundingPaymentPointer was incremented unduly, two things happen with the protocol:
currentFundingRate of the epoch, which is a value based on sold option's premiums is lost since it's based in the current epoch and the protocol skipped the current epoch
lastUpdatePaymentPointer, which is the counter of epochs now is forever wrong. If before the output of nextFundingPaymentTimestamp was genesis + 49 days, even if admin changes back to 7 days the fundingDuration, nextFundingPaymentTimestamp will output a bigger value since the calculation relies on lastFundingPaymentPointer. This will make every epoch lasts more than it should, since all epoch-related functions relies on nextFundingPaymentTimestamp.
Scenario of updating fundingDuration to a higher value.
1. Let's suppose fundingDuration = 7 days and latestFundingPaymentPointer - 7 (Counter of Epochs = 7, so we are in epoch 7). With this state variables, nextFundingPaymentTimestamp == genesis + 49 days
2. Admin changes fundingDuration to 9 days.
3. Now nextFundingPaymentTimestamp == genesis + (7 * 9), which is genesis + 63 days.
4. updateFundingPaymentPointer isn't affected anymore, since it relies that block.timestamp >= nextFundingPaymentTimestamp. However, updateFundings(), which is the function used in PerpetualAtlanticVault.sol and called in important functions, like purchase (used to buy Options), redeem and deposit (both used in PerpetualAtlanticVaultLP.sol for ERC-4626-Like Yield Farm) is now affected:
```
  function updateFunding() public {
```

updateFundingPaymentPointer();

uint256 currentFundingRate = fundingRates[latestFundingPaymentPointer];

uint256 startTime = lastUpdateTime == 0 ? (nextFundingPaymentTimestamp() - fundingDuration) : lastUpdateTime; lastUpdateTime = block.timestamp;

collateralToken.safeTransfer( addresses.perpetualAtlanticVaultLP, (currentFundingRate * (block.timestamp - startTime)) / 1e18 ); // ... }

- The function pays depositors of `PerpetualAtlanticVaultLP.sol` in a drip-maner way since it's called by a lot of methods and transfers to VaultLP a quantity of funds based on how much time of the epoch has already passed `currentFundingRate * (block.timestamp - startTime)`. 
- However, now that `nextFundingPaymentTimestamp` output is bigger than it should, `updateFundingPaymentPointer` will take longer to update the epoch, so the epoch will efectively last longer.

5. The calculation `currentFundingRate * (block.timestamp - startTime)` will start to get unexpected values, since the epoch duration wasn't expected to be so big.

6. Because of (5), more funds than it should will be transfered to `VaultLP` depositors. Also, `safeTransfer` (using `SafeERC20.sol` library) will revert if `PerpetualAtlanticVault.sol` doesn't have enough funds, which probably will happen since the protocol was expecting to drip-maner pay a `8 days` epoch, not a bigger one. 

7. So, denial of service will happen and even if admin updates `updateFunding` to the standard value `7 days` again, the `nextFundingPaymentTimestamp` can have unexpected actions since his calculation is wrong and can't handle how much days passed correctly.

## Tools Used

Manual Review

## Recommended Mitigation Steps

1. Add an array to list all `fundingDuration` values used.
2. Add a mapping to map how many epochs passed for each `fundingDuration` used 
3. Make `updateFundingPaymentPointer`, which is the function that updates epochs to interact with this mapping
4. Now, `nextFundingPaymentTimestamp` can correctly calculate the end of an epoch

function nextFundingPaymentTimestamp() public view returns (uint256 timestamp) { uint256 value; for (uint x; x < fundingDurationValues.length - 1; ++ x) { uint256 fundingDurationValue = fundingDurationValues[x]; value += epochsForEachFundingDuration[fundingDurationValue] * fundingDurationValue; return genesis + value; }



## Assessed type

Math

code-423n4 / 2023-08-dopex-findings

Epoch ending timestamps are misscalculated if the duration is changed #1244

Lines of code

Vulnerability details

Proof of Concept

Impact