Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

equity check after deposit/withdraw may fail due to the equityCache uses a different price than the price during the callback

Citris

Summary

equity check after deposit/withdraw may fail due to the equityCache uses a different price than the price during the callback.

take deposit as an example,

During processDeposit, equityAfter is calculated based the latest oracle price from GMXOracle. And the sharesToUser is calculated based on the equity difference. However, the equityBefore, is calculated during deposit call. Since these are two steps processes. This is possible that the price used for calculating the equityBefore and equityAfter become different.

In this case, deposit can be reverted, if price decreases and the equityAfter becomes smaller even after an increase in LpAmt. On the contract, withdrawal can be reverted, if price increases, and the equityAfter becomes larger even after an decrease in LpAmt.

equityAfter

function processDeposit(
GMXTypes.Store storage self
) external {
self.depositCache.healthParams.equityAfter = GMXReader.equityValue(self);
​
self.depositCache.sharesToUser = GMXReader.valueToShares(
self,
self.depositCache.healthParams.equityAfter - self.depositCache.healthParams.equityBefore,
self.depositCache.healthParams.equityBefore
);
​
GMXChecks.afterDepositChecks(self);
}

equityBefore

function deposit(
GMXTypes.Store storage self,
GMXTypes.DepositParams memory dp,
bool isNative
) external {
// Sweep any tokenA/B in vault to the temporary trove for future compouding and to prevent
// it from being considered as part of depositor's assets
if (self.tokenA.balanceOf(address(this)) > 0) {
self.tokenA.safeTransfer(self.trove, self.tokenA.balanceOf(address(this)));
}
if (self.tokenB.balanceOf(address(this)) > 0) {
self.tokenB.safeTransfer(self.trove, self.tokenB.balanceOf(address(this)));
}
​
self.refundee = payable(msg.sender);
​
GMXTypes.HealthParams memory _hp;
​
_hp.equityBefore = GMXReader.equityValue(self);
...

equity calculation in processDeposit, would underflow in processDeposit

function processDeposit(
GMXTypes.Store storage self
) external {
self.depositCache.healthParams.equityAfter = GMXReader.equityValue(self);
​
self.depositCache.sharesToUser = GMXReader.valueToShares(
self,
self.depositCache.healthParams.equityAfter - self.depositCache.healthParams.equityBefore,
self.depositCache.healthParams.equityBefore
);
​
GMXChecks.afterDepositChecks(self);
}

equity check in afterWithdrawChecks, would revert if equityAfter does not decrease

function afterWithdrawChecks(
GMXTypes.Store storage self
) external view {
// Guards: revert if lpAmt did not decrease at all
if (GMXReader.lpAmt(self) >= self.withdrawCache.healthParams.lpAmtBefore)
revert Errors.InsufficientLPTokensBurned();
​
// Guards: revert if equity did not decrease at all
if (
self.withdrawCache.healthParams.equityAfter >=
self.withdrawCache.healthParams.equityBefore
) revert Errors.InvalidEquityAfterWithdraw();

Vulnerability Details

Impact

revert of normal user deposit/withdraw

Tools Used

Recommendations

use a cached price from the equityBefore to calculate the equityAfter so that they can be compared apple to apple.

Updates

Lead Judging Commences

hans Auditor
over 1 year ago
hans Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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