Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[H-03] PerpetualAtlanticVault.settle(): If APP puts become OTM and price does not recover, it can cause option writers collateral to be locked forever #625

Closed
code423n4 opened this issue Sep 1, 2023 · 9 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1956 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

In PerpetualAtlanticVault.settle(), users that have provided liquidity in vaultLP for APP options can redeem their rDPX and wETH collateral once options are settled. However, once price of rDPX increases above 75% of strike price (which is very likely), these APP options are never settled due to this check in settle():

PerpetualAtlanticVault.sol#L333

function settle(
    uint256[] memory optionIds
)
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
{
    _whenNotPaused();
    _isEligibleSender();

    updateFunding();

    for (uint256 i = 0; i < optionIds.length; i++) {
        uint256 strike = optionPositions[optionIds[i]].st
        uint256 amount = optionPositions[optionIds[i]].amount;

        // check if strike is ITM
        // @audit if strike price (75% of rDPX price at time of minting)
        // is smaller than current price, reverts
->      _validate(strike >= getUnderlyingPrice(), 7);

        ethAmount += (amount * strike) / 1e8;
        rdpxAmount += amount;
        optionsPerStrike[strike] -= amount;
        totalActiveOptions -= amount;

        // Burn option tokens from user
        _burn(optionIds[i]);

        optionPositions[optionIds[i]].strike = 0;
        ...
        ...
}

Consider the following flow:

  1. If strike price is not in the money when settle() is called, some weth and rdpx will never be unlocked for liquidity providers to redeem their shares (This can happen for a sudden increase in rDPX price within a week and price remains high without decreasing), since settle() will revert

  1. Redemption of collateral for current epoch will not be allowed, since rdpx and wETH collateral is not unlocked yet via unlockLiquidity() and addRdpx() while liquidity providers still retain shares. Here _rdpxCollateral remains at 0, signifiying no rdpx collateral unlocked, and _activeCollateral remains constant since it is not unlocked yet.

  1. The next epoch starts, LP deposits to provide liquidity using deposit().

  1. More shares are minted, now assume strike price of options minted in current epoch is ITM, then settle() will not revert. Consequently, _rdpxCollateral and _activeCollateral will be increased and decreased respectively.

  1. Now, liquidity provider will be able to redeem shares minted for current epoch liquidity providers before _rdpxCollateral underflows, assuming previously non-settled options still cannot be settled as prices remain high. They will also compete with other liquidity providers where previously minted shares that are stuck attempts to redeem their shares.

Unless price of rDPX decrease till strike price is smaller again, previous APP options can never be settled leaving collateral to be potentially forever stuck for some liquidity providers, depending on who redeems their shares first whenever APP options are settled by treasury

Proof of Concept

underflow in _rdpxCollateral, user cannot redeem shares, of 1.89e18 wETH and 1e18 rDPX.

function testCollateralStuck() external {
    // Actors:
    // Liquidity provider 1: address(1)

    // 1. Setup token balances for LPs
    rdpx.mint(address(1), 10 ether);
    weth.mint(address(1), 3 ether);

    // 2. Provide Liquidity
    deposit(1 ether, address(1));

    // 3. Purchase APP at strike price of 0.0075 gwei (75% * 0.01 gwei)
    priceOracle.updateRdpxPrice(0.010 gwei);
    (, uint256 id) = vault.purchase(1 ether, address(this)); // purchases for epoch 1; send 0.05 weth premium
    uint256[] memory optionIds = new uint256[](1);
    optionIds[0] = id;

    // 4. Settle APP at current price of 0.02 gwei
    // Reverts as strike price is smaller than current price (0.0075 gwei  < 0.02 gwei)
    skip(86400); // expire
    priceOracle.updateRdpxPrice(0.02 gwei);
    vm.expectRevert(
        abi.encodeWithSelector(
        PerpetualAtlanticVault.PerpetualAtlanticVaultError.selector,
        7
        )
    );
    (uint256 ethAmount, uint256 rdpxAmount) = vault.settle(optionIds);

    // 5. Check current available wETH and rDPX balances for redemption in vaultLP for epoch 1
    uint collateralBalanceinLPBefore = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPBefore = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralBefore = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPBefore, rdpxCollateralBalanceinLPBefore, totalAvailableCollateralBefore);

    // 6. Next epoch starts, LP provides liquidity 
    deposit(1 ether, address(1));

    // 7. Purchase APP at strike price of 0.015 gwei (75% * 0.02 gwei), assuming rDPX price has increased for epoch 2
    priceOracle.updateRdpxPrice(0.02 gwei);
    (, uint256 id2) = vault.purchase(1 ether, address(this)); // purchases for epoch 2; send 0.05 weth premium

    // 8. Settle APP at current price of 0.015 gwei, assuming rDPX price has dropped
    uint256[] memory optionIds2 = new uint256[](1);
    optionIds2[0] = id2;
    priceOracle.updateRdpxPrice(0.015 gwei);
    (uint256 ethAmount2, uint256 rdpxAmount2) = vault.settle(optionIds2);

    // 9. Check current available wETH and rDPX balances for redemption in vaultLP for epoch 2
    (uint collateral1, uint rdpxPerShare1) = vaultLp.redeemPreview(2 ether);
    console.log(collateral1, rdpxPerShare1);
    uint collateralBalanceinLPAfter = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPAfter = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralAfter = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPAfter, rdpxCollateralBalanceinLPAfter, totalAvailableCollateralAfter);

    // 10. Attempt to redeem all shares (2e18) for unlocked rDPX and wETH collateral but fails
    uint256 balance = vaultLp.balanceOf(address(1));
    console.log(balance);
    vm.expectRevert();
    (uint256 ethAmount3, uint256 rdpxAmount3) = vaultLp.redeem(balance, address(1), address(1));

    // 11. Simulate strike price decreases to 0.0075 gwei, now previous epoch options can be settled
    // This is the only way liquidity provider can get back his collateral
    priceOracle.updateRdpxPrice(0.0075 gwei);
    uint256 id3 = id;
    uint256[] memory optionIds3 = new uint256[](1);
    optionIds3[0] = id3;
    (uint256 ethAmount4, uint256 rdpxAmount4) = vault.settle(optionIds3);
    uint collateralBalanceinLPAfter1 = vaultLp._totalCollateral();
    uint rdpxCollateralBalanceinLPAfter1 = vaultLp._rdpxCollateral();
    uint totalAvailableCollateralAfter1 = vaultLp.totalAvailableCollateral();
    console.log(collateralBalanceinLPAfter1, rdpxCollateralBalanceinLPAfter1, totalAvailableCollateralAfter1);

    // 12. Liquidity provider still cannot redeem shares as `_totalCollateral` is now
    // lower then expected due to loss incurred from APP
    (uint collateral2, uint rdpxPerShare2) = vaultLp.redeemPreview(2 ether);
    console.log(collateral2, rdpxPerShare2);
    // (uint256 ethAmount5, uint256 rdpxAmount5) = vaultLp.redeem(balance, address(1), address(1));
}

Tools Used

Manual Analysis, Foundry

Recommendation

It is crucial to rewrite options for optionIds where it cannot be settled when the new premium is calculated based on current rDPX mark price to prevent price changes/stability from locking collateral provide to write APP.

This can be done by exposing another admin function where associated optionIds from previous epoch not settled has its strike price rewrittened. This should be performed before calculating funding via calculateFunding() which is then followed by payFunding().

function rewrite (
    uint256[] memory optionIds
)
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
{

    uint256 currentPrice = getUnderlyingPrice(); // price of underlying wrt collateralToken
    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price

    uint256 amount;
    uint256 strikeBefore;
    for (uint256 i = 0; i < optionIds.length; i++) {
      strikeBefore = optionPositions[optionIds[i]].strike; 
      optionPositions[optionIds[i]].strike = strike;
      uint256 amount += optionPositions[optionIds[i]].amount;
      optionsPerStrike[strikeBefore] -= amount
    }

    optionsPerStrike[strike] += amount;
}

Assessed type

Context

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Sep 1, 2023
code423n4 added a commit that referenced this issue Sep 1, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as duplicate of #1012

@c4-pre-sort
Copy link

bytes032 marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Sep 15, 2023
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 12, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo changed the severity to QA (Quality Assurance)

@nevillehuang
Copy link

nevillehuang commented Oct 21, 2023

Hi @GalloDaSballo, I think this should be a duplicate of #1956, as it is also describing about the same root cause of not being able to forfeit OTM options

@c4-judge c4-judge reopened this Oct 25, 2023
@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Oct 25, 2023
@c4-judge
Copy link
Contributor

This previously downgraded issue has been upgraded by GalloDaSballo

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as not a duplicate

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as duplicate of #1956

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Oct 30, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as satisfactory

@GalloDaSballo
Copy link

Leaving #1956 as primary due to using the lingo of forfeiture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-1956 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

5 participants