Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L296

Vulnerability details

Impact

The updateInterest() function is called before retrieving the fromPosition value from positions[params_.tokenId][params_.fromIndex] in the moveLiquidity() function. This means that the bucketDeposit value may not reflect the current accrued interest, which can result in an incorrect amount of liquidity being available in the position and lead to unexpected behavior.

If the bucketDeposit value does not accurately reflect the current accrued interest, the maxQuote calculation may not accurately represent the maximum amount of quote tokens that can be moved. This can result in a user being able to move more liquidity than they should.

Proof of Concept

In the moveLiquidity() function

function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

In the code, the updateInterest() function is called before retrieving the fromPosition value from positions[params_.tokenId][params_.fromIndex]. This means that the bucketDeposit value may not reflect the current accrued interest. Therefore, when calculating the maximum amount of quote tokens that can be moved, given the tracked LP, vars.maxQuote may not accurately represent the maximum amount of quote tokens that can be moved.

For Example:

Alice deposits liquidity into a position and waits for some time to accrue interest.
Alice calls the moveLiquidity() function, specifying a larger amount of quote tokens than what is actually available in the position. Let's say Alice specifies 1000 quote tokens, but the actual available amount is only 800.
Due to the incorrect calculation of maxQuote, the smart contract allows Alice to move more liquidity than she should. In this case, let's say Alice is able to move 1200 LP tokens from the position.
Alice sells the LP tokens for collateral tokens, effectively withdrawing more funds from the position than she should be able to.
As a result, the position now has less liquidity than expected, potentially causing it to underperform or even result in a loss for Bob, the victim.

Tools Used

vscode

Recommended Mitigation Steps

The updateInterest() function should be called after retrieving the fromPosition value from positions[params_.tokenId][params_.fromIndex]. This will ensure that the bucketDeposit value reflects the current accrued interest.

function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();
}

Now the bucketDeposit value will reflect the current accrued interest, and the maxQuote calculation will accurately represent the maximum amount of quote tokens that can be moved.

Assessed type

Error

code-423n4 / 2023-05-ajna-findings

Incorrect Calculation of Max Amount of Quote Tokens in moveLiquidity() Function in PositionManager.sol. #438