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

Chainlink Price Feed Validation Bypass in KeeperProxy Compromises Vault Price Safety

Summary and Impact

The KeeperProxy contract, which serves as the core price validation mechanism for the Perpetual Vault Protocol, has incomplete validation of Chainlink price feed data. The _check() function fails to properly validate the returned price data, potentially allowing stale or incorrect prices to be used in critical vault operations.

This directly violates the protocol's core invariant of "Depositor Share Value Preservation" as stated in the documentation: "The value of a depositor's shares should never decrease due to the actions of other depositors. Any losses or gains should be distributed proportionally based on share ownership."

When price feeds return unexpected values, the system may incorrectly value positions, leading to:

  1. Improper position valuations affecting depositor share values

  2. Incorrect execution of leveraged trades

  3. Unfair distribution of funding fees

Vulnerability Details

The vulnerability exists in the _check() function of KeeperProxy.sol:

function _check(address token, uint256 price) internal view {
// https://github.com/code-423n4/2021-06-tracer-findings/issues/145
(, 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"
);
}

The function:

  1. Doesn't validate answeredInRound vs roundId

  2. Ignores potential negative prices

  3. Uses unsafe casting via toUint256()

Here's a proof of concept test demonstrating the issue:

function test_PriceFeedValidationBypass() public {
// Setup mock price feed returning stale round data
mockPriceFeed.setPrice(1000e8);
mockPriceFeed.setStaleRoundData(true);
MarketPrices memory prices;
prices.indexTokenPrice.min = 1000e8;
prices.indexTokenPrice.max = 1000e8;
// This should revert but doesn't due to incomplete validation
keeper.run(
address(vault),
true,
true,
prices,
new bytes[](0)
);
}

This is particularly concerning because the protocol documentation states: "After all actions completed, nextAction, swapProgressData should be empty. PositionKey is 0 when no position". Invalid prices could lead to incomplete actions or incorrect position keys.

Tools Used

  • Manual Review

  • Foundry

Recommendations

Update the _check() function to include comprehensive price feed validation:

function _check(address token, uint256 price) internal view {
(
uint80 roundId,
int256 chainLinkPrice,
,
uint256 updatedAt,
uint80 answeredInRound
) = AggregatorV2V3Interface(dataFeed[token]).latestRoundData();
require(chainLinkPrice > 0, "negative price");
require(answeredInRound >= roundId, "stale price round");
require(updatedAt > block.timestamp - maxTimeWindow[token], "stale price feed");
uint256 decimals = 30 - IERC20Meta(token).decimals();
price = price / 10 ** (decimals - 8);
uint256 chainlinkPriceUint = uint256(chainLinkPrice);
require(
_absDiff(price, chainlinkPriceUint) * BPS / chainlinkPriceUint < priceDiffThreshold[token],
"price offset too big"
);
}
```.
Updates

Lead Judging Commences

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

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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