joestakey - Incorrect check in `claimCollateral` leads to the function always reverting

[sherlock-admin]

joestakey

high

Incorrect check in claimCollateral leads to the function always reverting

Summary

Due to a logic error,claimCollateral always reverts, making it impossible for users to claim collateral.

Vulnerability Detail

MarketUtils.claimCollateral computes the amount of collateral a user can claim before transferring that amount.

The issue is that the call mistakenly reverts when the claimableAmount is greater than the already claimed amount line 632.

File: contracts/market/MarketUtils.sol
631:          uint256 adjustedClaimableAmount = Precision.applyFactor(claimableAmount, claimableFactor);
632:         if (adjustedClaimableAmount >= claimedAmount) {//@audit error
633:             revert CollateralAlreadyClaimed(adjustedClaimableAmount, claimedAmount);
634:         }
635:
636:        uint256 remainingClaimableAmount = adjustedClaimableAmount - claimedAmount;

Impact

The call will always revert, either line 633, or with an underflow error line 636. Users cannot claim their collateral.

Code Snippet

https://github.com/gmx-io/gmx-synthetics/blob/7be3ef2d119d9e84473e1a49f346bcdc06fd57a3/contracts/market/MarketUtils.sol#L631-L636

Tool used

Manual Review

Recommendation

The call should revert if the claimableAmount is lower than the already claimed amount

631:          uint256 adjustedClaimableAmount = Precision.applyFactor(claimableAmount, claimableFactor);
-632:         if (adjustedClaimableAmount >= claimedAmount) {
+632:         if (adjustedClaimableAmount <= claimedAmount) {
633:             revert CollateralAlreadyClaimed(adjustedClaimableAmount, claimedAmount);
634:         }
635:
636:        uint256 remainingClaimableAmount = adjustedClaimableAmount - claimedAmount;

Duplicate of #136