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

3 stars 3 forks source link

`CalculateFunding()` and `provideFunding()` shoud not be disabled when paused #1580

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

CalculateFunding() and provideFunding() are used to calculate the funding rate and transfer the funding payments to PerpetualAtlanticVault for rewarding the PerpetualAtlanticVaultLP option writers.

However, they are disabled when RdpxV2Core is paused(), which causes the option writers to be shortchanged as they are not able to fully redeem their LP shares if there are still OTM options.

There could be legitimate reasons where the protocol is paused for a short period of time, and the protocol still wish to keep the options active (for hedging). In that case, the protocol should still pay the funding to the option writers.

Impact

PerpetualAtlanticVaultLP option writers will lose funding payments when paused despite having their WETH locked in PerpetualAtlanticVaultLP.

Proof of Concept

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

  function calculateFunding(
    uint256[] memory strikes
  ) external nonReentrant returns (uint256 fundingAmount) {
    //@audit this prevent computation of funding when paused
    _whenNotPaused();
    _isEligibleSender();

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L795

  function provideFunding()
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
    returns (uint256 fundingAmount)
  {
    //@audit this prevent funding payment when paused
    _whenNotPaused();

Recommended Mitigation Steps

Remove _whenNotPaused() from CalculateFunding() and provideFunding().

Assessed type

Other

c4-pre-sort commented 12 months ago

bytes032 marked the issue as sufficient quality report

c4-sponsor commented 11 months ago

psytama marked the issue as disagree with severity

psytama commented 11 months ago

This should be a medium severity as if such a case arises there are ways we can compensate the users.

c4-judge commented 11 months ago

GalloDaSballo changed the severity to 2 (Med Risk)

GalloDaSballo commented 11 months ago

The argument here is that when the contracts are paused yield would be lost, I think this has historically been awarded as Medium

c4-judge commented 11 months ago

GalloDaSballo marked the issue as duplicate of #1496

c4-judge commented 10 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)