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

3 stars 3 forks source link

`PerpetualAtlanticVaultLP.deposit()` should be made pausable #843

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/PerpetualAtlanticVaultLP.sol#L118-L135

Vulnerability details

PerpetualAtlanticVaultLP.deposit() allows users to deposit WETH to write put options and receive funding payments.

However, when RdpxV2Core and PerpetualAltanticVault are paused, new premium and funding payments will be paused as well.

Therefore, deposit() should be paused in that situation, to prevent new users from depositing WETH that will not receive funding payments.

  function deposit(
    uint256 assets,
    address receiver
  ) public virtual returns (uint256 shares) {
    //@audit whenNotPaused() should be added to prevent new deposits during pause state as they will not receive funding payments

    // Check for rounding error since we round down in previewDeposit.
    require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

    perpetualAtlanticVault.updateFunding();

    // Need to transfer before minting or ERC777s could reenter.
    collateral.transferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

    _totalCollateral += assets;

    emit Deposit(msg.sender, receiver, assets, shares);
  }

Impact

During the paused state, the protocol continues to allow new deposits into PerpetualAtlanticVaultLP but fails to pay any funding.

Recommended Mitigation Steps

Add whenNotPaused() to PerpetualAtlanticVaultLP.deposit()

Assessed type

Other

c4-pre-sort commented 11 months ago

bytes032 marked the issue as primary issue

c4-pre-sort commented 11 months ago

bytes032 marked the issue as sufficient quality report

c4-sponsor commented 11 months ago

witherblock (sponsor) acknowledged

c4-sponsor commented 11 months ago

witherblock (sponsor) confirmed

GalloDaSballo commented 11 months ago

Imo the Warden should have spent more time explaining the finding

This seems to boil down to the ability of depositing into a deprecated vault, no funds are lost, nor denied, and people can redeem

c4-judge commented 11 months ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 11 months ago

Downgrading to QA due to lack of impat