Description

The depositWithDiscountAndLock and mintWithDiscountAndLock functions allow an LP to choose to lock their vault shares upon deposit, trading away the ability to redeem those shares for a lockDuration in exchange for a discount on the share price.

When the deposit is made, the discount is noted in _executeDiscountAndLock:

function _executeDiscountAndLock(
    uint256 assets,
    uint256 assetsDeposited,
    uint256 shares,
    uint32 lockDuration,
    address receiver
) private returns (uint256) {
    // [...]
    uint256 assetsDiscount = assets - assetsDeposited;

    LockedDeposit storage d = lockedDeposits[depositId];
    d.owner = receiver;
    d.shares = shares;
    d.assetsDeposited = assetsDeposited;
    d.assetsDiscount = assetsDiscount;
    d.atTimestamp = uint32(block.timestamp);
    d.lockDuration = lockDuration;

And then later, when lockDuration time has passed and the discount is unlocked, the discount is accounted for into accPnlPerToken:

function unlockDeposit(uint256 depositId, address receiver) external {
    // [...]
    int256 accPnlDelta = int256(d.assetsDiscount.mulDiv(PRECISION_6, totalSupply(), MathUpgradeable.Rounding.Up));

    accPnlPerToken += accPnlDelta;
    if (accPnlPerToken > int256(maxAccPnlPerToken())) {
        revert NotEnoughAssets();
    }

    lockedDepositNft.burn(depositId);

    accPnlPerTokenUsed += accPnlDelta;
    updateShareToAssetsPrice();

However, the unlock process changes accPnlPerTokenUsed, which directly affects the share price. Additionally, the unlocking itself can fail with NotEnoughAssets if it increases the accPnlPerToken by too much, despite the amount already being promised to the LP at lock time.

Impact

If the LPs suffer net losses, owners of large locked deposits cannot unlock their deposits because of NotEnoughAssets, so they cannot even liquidate their deposit at a loss.

Additionally, because the unlock process directly affects the share price, both front-runners and the unlocker themselves can take advantage of this expected change in the share price without exposing themselves to any risk. For instance, someone with both a large locked position and a large amount of unlocked shares can first queue the withdrawal of all of those shares and then later, in a transaction, complete the withdrawal at a particular share price, unlock the deposit (which decreases the share price), and then rebuy all of the shares for less money, pocketing the difference.

Recommendations

We recommend increasing accPnlPerToken at the time the lock is created, instead of when the lock is unlocked. That would prevent both of the issues from happening and better represent the fact that a discount was promised to the LP at the time they agreed to the lock.

Remediation