Lines of code

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

Vulnerability details

Impact

The moveLiquidity function as described. If the positionIndex.remove(params_.fromIndex) function call returns false, it means that the specified index was not present in the positionIndex set, and the RemovePositionFailed() error is not actually applicable in this case. However, the code does not handle this scenario, and it will still revert with RemovePositionFailed() error, even though the position was not in the index.

This can cause confusion for developers who are calling this function, as they may assume that the error occurred because the specified index was present in the index and couldn't be removed, when in reality, the index was not present in the first place.

Proof of Concept

The vulnerable code can be found in the moveLiquidity function of the smart contract. Specifically, the issue occurs in the line where the remove function of the positionIndex variable is called. The line of code in question is:

        if (!positionIndex.remove(params_.fromIndex)) revert RemovePositionFailed();

The function called "moveLiquidity" that allows users to move their liquidity from one bucket to another within the contract. However, there is a problem with how the function checks if the position being moved actually exists in the index before removing it. If the position does not exist in the index, the function throws an error and reverts, even if the position was never in the index in the first place. https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L333

When a user wants to move their liquidity from one bucket to another, the function first checks if the user has already moved liquidity from that bucket. If they have not, it updates the interest for the bucket and retrieves information about the bucket, such as the amount of liquidity pool tokens (LP) and collateral deposited in the bucket. It then checks if the bucket has gone bankrupt since the user deposited their liquidity. If it has, the function throws an error and reverts.

Next, the function calculates the maximum amount of quote tokens (QT) that can be moved based on the tracked LP and collateral in the bucket, and the current price of the LP token. It then retrieves the index of positions for the token being moved, and attempts to remove the current position index from the tracked positions. If the index is successfully removed, the function adds the new position index to the tracked positions.

Tools Used

Recommended Mitigation Steps

A check can be added to ensure that the position exists in the index before attempting to remove it. One possible solution is to use the contains function of the EnumerableSet library to check if the index exists in the positionIndex variable before removing it.

Assessed type

Error

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

Failure to Check for Existence Before Removal #417