code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

When admin change fundingDuration, current funding epoch will be affected. Funding accounting will be inaccurate, future or past epochs could be mixed, and protocol could lose fund or lock fund. In some cases could temporarily DoS the functionality due to not enough fund.

Proof of Concept

Let fundingDuration be 7 days, PaymentPointer 0 points to day 0-6, PaymentPointer 1 points to day 7-13, etc. Now is day 30, latestFundingPaymentPointer updated to 5 (next epoch of day 28-34), lastUpdateTime is day 27. And let the price stay the same level. Funding for each epoch is 21 weth. So the fundingRate is 3 weth/day. And fundingRates[5] is set to 3 weth/day.

Then admin changes the fundingDuration to 10 days.

File: contracts\perp-vault\PerpetualAtlanticVault.sol
237:   function updateFundingDuration(
238:     uint256 _fundingDuration
239:   ) external onlyRole(DEFAULT_ADMIN_ROLE) {
240:     fundingDuration = _fundingDuration;
241:   }

Now the latestFundingPaymentPointer is still 5, lastUpdateTime is still day 27. However, the current implementation will treat epoch 5 as day 40-49, nextFundingPaymentTimestamp() will return day 49.

563:   function nextFundingPaymentTimestamp() {
568:     return genesis + (latestFundingPaymentPointer * fundingDuration);
569:   }

From now to day 40, latestFundingPaymentPointer won't be increased, since block.timestamp is less than nextFundingPaymentTimestamp():

462:   function updateFundingPaymentPointer() public {
463:     while (block.timestamp >= nextFundingPaymentTimestamp()) {

493:       latestFundingPaymentPointer += 1;

As a result, the funding rate is fixed until day 49. If the fixed rate is high level, the protocol will incur loss. Assume for the later epoch, the funding becomes 2 weth/day, but fundingRates[5] will be used until day 49, (49 - 34) * (3 - 2) = 15 weth in loss. Or if the contract does not have enough weth to pay, the contract will DoS until new fund is refilled. This could happen because latestFundingPaymentPointer does not increase for a long time, no more enough funding added. On the other hand, if the later epoch has lower premium than the fixed rate, those difference will be locked in the contract.

Another possibility, the fundingDuration can decrease from 10 to 7, assuming now is day 48, next epoch 5 (day 50-59) premium is transferred. When fundingDuration is decreased, updateFundingPaymentPointer() will update epoch pointer to 8 (day 49-55), fundingRates[5] is already past, it will not be used, so the transferred fund will be lost.

502:   function updateFunding() public {
503:     updateFundingPaymentPointer();
504:     uint256 currentFundingRate = fundingRates[latestFundingPaymentPointer];

Tools Used

Manual analysis.

Recommended Mitigation Steps

The way to record past epochs, need refactor. For each epoch, the start/end time and funding rate need to be stored, rather than computed by genesis + (latestFundingPaymentPointer * fundingDuration).

Also need to make sure changed fundingDuration only take effect for future epochs, not past epochs.

Assessed type

Other

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #980

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-judge commented 11 months ago

GalloDaSballo changed the severity to 2 (Med Risk)