DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Incorrect Handling of `positionIsClosed` for Liquidations in `afterLiquidationExecution` Function

Summary

The afterLiquidationExecution function is responsible for updating the contract's state after a position is liquidated or auto-deleveraged (ADL) on GMX. However, the function does not explicitly set the positionIsClosed state variable to true when a position is fully liquidated, despite a comment in the code stating that the positionIsClosed value should be kept to allow the keeper to create new orders with the liquidated funds. This inconsistency between the comment and the implementation can lead to incorrect contract state and prevent the keeper from creating new orders with the liquidated funds.

Vulnerability Details

The comment in the afterLiquidationExecution function states:

// keep the positionIsClosed value so that let the keeper be able to create an order again with the liquidated fund

However, the function does not explicitly set positionIsClosed = true when a position is fully liquidated.

The function only deletes curPositionKey if the position size is zero (sizeInTokens == 0):

if (sizeInTokens == 0) {
delete curPositionKey; // Clear the position key if the position is fully liquidated
}

It does not update positionIsClosed, which remains false even after the position is liquidated.

Impact

If positionIsClosed is not set to true after a liquidation, the contract may incorrectly assume that a position is still open. This can lead to errors in subsequent operations, such as creating new orders or handling withdrawals.
The keeper relies on the positionIsClosed state variable to determine whether new orders can be created using the liquidated funds. If positionIsClosed remains false, the keeper may be unable to create new orders, even though the funds are available.

Tools

Manual Review

Recommendations

Explicitly set positionIsClosed = true when a position is fully liquidated.

function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
+ positionIsClosed = true;
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_liquidation_do_not_reset_positionIsClosed

"// keep the positionIsClosed value so that let the keeper be able to create an order again with the liquidated fund" Liquidation can send some remaining tokens. No real impact here.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!