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

Price Decimals Assumption in KeeperProxy

verbotenviking

Summary

The KeeperProxy contract contains hardcoded assumptions about token and price feed decimals that could lead to incorrect price validations. This issue affects price comparison logic and could potentially allow invalid transactions with incorrect pricing or block valid transactions.

Vulnerability Details

The issue is located in the _check function in KeeperProxy.sol (lines 208-209):

uint256 decimals = 30 - IERC20Meta(token).decimals();
price = price / 10 ** (decimals - 8); // Chainlink price decimals is always 8.

The code makes two specific assumptions:

  1. Chainlink price feeds always have 8 decimals (as noted in the comment)

  2. The total normalization calculation assumes token decimals are less than 30

While these assumptions might be valid for many current assets, there's no guarantee they will hold for all tokens that might be added to the system in the future. If a token with different decimal places or a price feed with non-standard decimals is used, the validation could produce incorrect results.

Additionally, there's no handle for potential errors when calling the external decimals() function on the token contract.

Impact

Incorrect decimal handling could lead to:

  1. Allowing transactions to proceed with incorrect price validation, potentially enabling trades at unfavorable prices

  2. Blocking legitimate transactions that should pass validation

  3. Integer underflow if a token with more than the expected decimals is used

  4. System malfunction if a price feed with non-standard decimals is integrated

This could impact the reliability of the price validation system, which is a critical security component of the perpetual vault system.

Tools Used

Manual code review

Recommendations

  1. Query the decimals directly from the Chainlink price feed instead of assuming it's always 8:

function _check(address token, uint256 price) internal view {
(, int chainLinkPrice, , uint256 updatedAt, ) = AggregatorV2V3Interface(dataFeed[token]).latestRoundData();
require(updatedAt > block.timestamp - maxTimeWindow[token], "stale price feed");
// Get decimals from both the token and the price feed
uint8 tokenDecimals;
uint8 feedDecimals;
try IERC20Meta(token).decimals() returns (uint8 _decimals) {
tokenDecimals = _decimals;
} catch {
revert("Failed to get token decimals");
}
try AggregatorV2V3Interface(dataFeed[token]).decimals() returns (uint8 _decimals) {
feedDecimals = _decimals;
} catch {
revert("Failed to get feed decimals");
}
// Normalize based on actual decimals
uint256 normalized = price;
if (tokenDecimals < 30) {
normalized = normalized / 10 ** (30 - tokenDecimals - feedDecimals);
} else {
normalized = normalized * 10 ** (tokenDecimals - 30 + feedDecimals);
}
require(
_absDiff(normalized, chainLinkPrice.toUint256()) * BPS / chainLinkPrice.toUint256() < priceDiffThreshold[token],
"price offset too big"
);
}

  1. Add validation for token and price feed decimals to ensure they are within expected ranges before performing calculations.

  2. Document the expected decimal ranges for both tokens and price feeds that can be safely used with the system.

  3. Consider adding configuration options to manually set decimals for special cases if necessary.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
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.

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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

Give us feedback!