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

Safety mechanism in KeeperProxy to validate Price is broken

Summary

There is a mechanism in KeeperProxy::_validatePrice which validates the market prices against the Chainlink data feed to make sure the price difference is under threshold. But that is broken which makes this safeguard ineffective.

Vulnerability Details

So Under KeeperProxy::``validatePrice it calling different function _check to validate the price. But it's comparing the longTokenprices with indexTokenPircewhich is actual issue. Because not all market have the same index and long token.
For e.g: As per docs the available market would going to be (WETH, WBTC, LINK). But the market of WBTC is not available in which longTokenand indexTokenis same in GMX perpetual Arbitrum.
Can be validated by `GMX::Reader::getMarkets()` (0x0537C767cDAC0726c76Bb89e92904fe28fd02fE1)

// KeeperProxy::_validatePrice
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
// https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/KeeperProxy.sol#L174C1-L179C63

Impact

There is multiple Impact it can caused.

  • It would lead the call to revert making it DOS.

    • Becuase we could be making call at 0 address dataFeed[token]).latestRoundData()

    • By being the price diff too big, As we will be comparing Token A price with Token B

  • Ineffective price comparison putting the protocol integrity at risk

// KeeperProxy::_check
(, int chainLinkPrice, , uint256 updatedAt, ) = AggregatorV2V3Interface(dataFeed[token]).latestRoundData();
require(updatedAt > block.timestamp - maxTimeWindow[token], "stale price feed");
uint256 decimals = 30 - IERC20Meta(token).decimals();
price = price / 10 ** (decimals - 8); // Chainlink price decimals is always 8.
require(
_absDiff(price, chainLinkPrice.toUint256()) * BPS / chainLinkPrice.toUint256() < priceDiffThreshold[token],
"price offset too big"
);

Tools Used

Manual Review

Recommendations

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

Lead Judging Commences

n0kto Lead Judge 9 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.

Give us feedback!