Inconsistence input of depositTime might lead to unexpected result

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L268 (https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L285 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L367 (https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L372

Vulnerability details

Impact

Inconsistency by relying on depositTime at positions hashmap instead of reading the value from lenderInfo()

Proof of Concept

The depositTime is being read from fromPosition.depositTime at moveLiquidity function and not from lenderInfo() L268 and L285

        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();

The lender might do another deposit into the bucket and therefor we would have a new depositTime that not updated at positions hashmap, this could lead to incorrect check for the Bucket Bankrupt

if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

The same thing is happening on reedemPositions at L367 which could block the token owner from redeeming his positions because of the check at L372

 if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) revert BucketBankrupt();

Tools Used

Manual Review

Recommended Mitigation Steps

Read the depositTime from lenderInfo() instead of positions hashmap just like memorializePositions function

    address owner = ownerOf(params_.tokenId);
( , uint256 depositTime) = pool.lenderInfo(index, owner);

Assessed type

Other

[c4-sponsor]

ith-harvey marked the issue as sponsor disputed

[ith-harvey]

The call uses the positionManager contract to store depositTimes instead of relying on the pool's record of depositTime. This is to ensure that someone who uses the position manager cannot receive an LP advantage if the bucket goes bankrupt.

[Picodes]

In the absence of detailed scenario or PoC I'll flag this report as invalid

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient proof