Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

The price check in _validatePrice is incorrect

biakia

Summary

The KeeperProxy contract is designed to act as a proxy for executing keeper functions on a PerpetualVault. It includes a price validation mechanism to ensure that market prices are within acceptable bounds before executing actions. However, a logical error exists in the _validatePrice function where the price validation for the longToken incorrectly uses the indexTokenPrice instead of the longTokenPrice. This issue could lead to incorrect price validation, potentially allowing transactions to execute at invalid prices, thereby increasing the risk of financial loss or manipulation.

Vulnerability Details

The vulnerability is located in the _validatePrice function of the KeeperProxy contract:

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// L2 Sequencer check
(
/*uint80 roundID*/,
int256 answer,
uint256 startedAt,
/*uint256 updatedAt*/,
/*uint80 answeredInRound*/
) = AggregatorV2V3Interface(sequencerUptimeFeed).latestRoundData();
bool isSequencerUp = answer == 0;
require(isSequencerUp, "sequencer is down");
// Make sure the grace period has passed after the sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "Grace period is not over");
​
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}

The _check function is called twice for marketData.longToken, but it incorrectly uses prices.indexTokenPrice.min and prices.indexTokenPrice.max as the price parameters. This is a logical error because the longToken price should be validated against prices.longTokenPrice.min and prices.longTokenPrice.max.

_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);

Impact

  1. Incorrect Price Validation:

    • The longToken price is compared against the indexTokenPrice, which may have no correlation with the longToken price. This could result in invalid price validation, allowing transactions to proceed even when the longToken price is outside the acceptable range.

  2. Increased Risk of Financial Loss:

    • If the indexTokenPrice and longTokenPrice differ significantly, transactions could be executed at unfavorable prices, leading to financial losses for users or the protocol.

  3. Potential for Manipulation:

    • An attacker could exploit this vulnerability by manipulating the indexTokenPrice to bypass the price validation for the longToken, potentially executing malicious transactions.

The impact is Medium, the likelihood is Medium, so the severity is Medium.

Tools Used

Manual Review

Recommendations

Consider following fix:

_check(marketData.longToken, prices.longTokenPrice.min);
_check(marketData.longToken, prices.longTokenPrice.max);
Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_validatePrice_no_check_for_longTokenPrice

Likelihood: None/Very Low, everytime the keeper send a price via run/runNextAction (sent by the Gamma keeper). Impact: Medium/High, does not check the longTokenPrice, it could go out of range. Keep in mind indexToken == longToken, an error from the keeper could be considered informational.

Support

FAQs

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