CAPO Price Feed Audit

1

Summary

Timeline: From 2025-03-14 To 2025-03-17

Total Issues: 13 (0 resolved)

High Severity Issues: 1 (0 resolved)

Medium Severity Issues: 2 (0 resolved)

Low Severity Issues: 5 (0 resolved)

Notes & Additional Information: 5 (0 resolved)

Scope

OpenZeppelin reviewed pull request #972 of the compound-finance/comet repository at commit 32ce458.

In scope were the following files:

contracts/pricefeeds/ERC4626CorrelatedAssetsPriceOracle.sol
contracts/pricefeeds/utils/PriceCapAdapterBase.sol

System Overview

The Compound protocol is planning to implement the Correlated-Assets Price Oracle (CAPO) to enhance the security and reliability of price feeds for Liquid Staking Tokens (LSTs) and Liquid Restaking Tokens (LRTs). The primary motivation for this implementation is to mitigate the risks associated with using an exchange rate price feed, which can be manipulated due to fluctuating or artificially inflated values. By introducing CAPO, Compound aims to cap the maximum exchange rate growth based on predefined parameters, ensuring that the valuation of these assets remains stable and secure.

CAPO operates as an intermediary in the price feed flow. Instead of directly relying on an exchange rate price feed, CAPO verifies whether the exchange rate exceeds a dynamically calculated cap based on an initial snapshot ratio and an allowed yearly growth rate. If the exchange rate surpasses this cap, CAPO enforces the maximum preconfigured rate, preventing excessive growth within short timeframes. This mechanism safeguards the protocol from price manipulation that could lead to vulnerabilities or bad debt due to overvalued collateral.

Security Model and Trust Assumptions

The security of CAPO relies on specific assumptions regarding the roles and periodic updates of snapshots to maintain the accuracy and integrity of price caps.

  • Timelock Management: We assume that the Timelock contract will be made CAPO’s manager, enabling the Compound DAO to control and update configuration parameters through governance proposals. This ensures that parameter adjustments, such as modifying the snapshot ratio or allowable growth rate, are subject to decentralized governance and cannot be arbitrarily altered by a single entity.

  • Regular Snapshot Updates: We assume that the snapshot update process will occur regularly and will not exceed a period of three years. This is necessary to prevent potential overflows, as the contract currently limits overflow checks to a three-year timeframe.

Privileged Roles

The CAPO contract defines only one privileged role: the manager. The role is authorized to call the following functions:

  • updateSnapshot: Allows the manager to update the price cap parameters, ensuring that the oracle remains aligned with the latest exchange rate conditions.
  • setManager: Enables the transfer of the manager role to another address, which is expected to be controlled by governance via the Timelock contract.

Since the manager is anticipated to be the Timelock contract, all changes will be subject to governance approvals.

High Severity

Large Inflations Are Allowed Over a Single Block

The CAPO contract does not properly enforce a limit on inflation within a single block. While the contract sets a maximum yearly growth rate, it does not distribute this growth evenly over time and allows the maximum ratio to grow as time passes since the latest snapshot timestamp grows. As time goes by, the allowed maximum ratio returned becomes larger, which an attacker can apply all at once in a single block.

Consider a scenario where the snapshot is taken with a maximum growth rate of 24% per year (2% per month). If the underlying asset has a typical yield of 4% per year, the price ratio will increase by ~3% over the course of a year. However, since one year has passed, the maximum allowed ratio has grown by approximately 20% (124 / 103 - 100). This means that an attacker could inflate the exchange rate by 20% in a single block, potentially executing a large-scale manipulation similar to past exploits (e.g., CREAM Finance Protocol).

Consider enforcing a mechanism where growth is applied incrementally rather than allowing accumulated growth to be used all at once. A possible approach is to cap the exchange rate growth per block to a fraction of the allowed yearly rate, ensuring that inflation is applied linearly over time rather instead of permitting sudden spikes.

Medium Severity

Decrease in totalAssets() Can Enable Large Single-Block Inflation

In a standard ERC-4626 vault, the share-to-asset ratio can be inflated, but never decreased. However, LSTs behave differently, as their totalAssets() function reflects the value of staked ETH backing them rather than an internal vault balance. If a slashing event occurs, totalAssets() decreases, leading to a lower share-to-asset ratio.

Attack Scenario

  • Assume the current share-to-asset ratio is 100, and CAPO is in sync, enforcing a maximum ratio of 101.
  • A slashing event occurs, reducing totalAssets(), causing the share-to-asset ratio to drop to 75.
  • CAPO reflects this lower ratio (75), but the maximum allowable ratio remains at 101.
  • An attacker can now increase the share-to-asset ratio by 26, which translates to a 34% increase (26/75), effectively bypassing CAPO’s growth limitations.
  • If 34% inflation within a single block is sufficient, the attacker can execute an exploit similar to the CREAM Finance attack.

Normally, only LST holders would bear the financial impact of slashing. However, this example shows that lending protocols relying on CAPO for pricing could also be vulnerable if a slashing event allows an attacker to inflate prices beyond safe limits.

Consider implementing additional constraints that prevent sudden inflation spikes following a drop in totalAssets(). One approach could be to dynamically adjust the maximum allowable growth rate based on recent slashing events, ensuring that attackers cannot exploit abrupt ratio decreases to inflate the price within a single block.

Potential Precision Loss in maxRatioGrowthPerSecond Calculation

The calculation of maxRatioGrowthPerSecond may yield inconsistent results when used with tokens that have a low number of decimals, such as those with 8 decimals. This occurs because the division involves a large denominator (31104000 * 10_000), increasing the likelihood of returning a floating-point value, which Solidity does not support.

For example, if the ratio is below 259200000 and the percentage is 12, the calculation results in a floating-point approximation:

200000000 * 1200 / (100 * 1e2) / 31104000
200000000 * 1200 / 311040000000
≈ 0.77 (rounded)

Since Solidity only supports integer values, this floating-point result will be truncated to 0, meaning that maxRatioGrowthPerSecond will effectively be 0. As a consequence, the asset’s price will not increase as expected over time, defeating the purpose of the cap mechanism and potentially causing issues in the protocol’s price updates.

Consider adjusting the calculation to ensure that it always results in a non-zero integer value, possibly by restructuring the formula to minimize precision loss.

Low Severity

Immutable minimumSnapshotDelay Prevents Updates

The minimumSnapshotDelay parameter is set as an immutable variable, which means it cannot be modified after contract deployment. This limitation reduces the protocol’s flexibility, as it may be necessary to adjust this delay based on evolving risk parameters or governance decisions.

Consider defining minimumSnapshotDelay as a mutable state variable instead of an immutable one. Additionally, a setter function should be implemented that only allows the manager to update this parameter when needed.

Missing Zero-Address Check

When operations with address parameters are performed, it is crucial to ensure that the address is not set to zero. Setting an address to zero is problematic because it has special burn/renounce semantics. This action should be handled by a separate function to prevent accidental loss of access during value or ownership transfers.

Within PriceCapAdapterBase.sol, the newManager operation is missing a zero-address check.

Consider always performing a zero-address check before setting an address-type state variable.

Multiple PriceCapSnapshot Updates Within the Same Block

It is possible to call the updateSnapshot function of the PriceAdapterBase contract multiple times within the same block. This allows a malicious manager to set a snapshot timestamp to a past value, perform multiple updates within the same block, and potentially manipulate the protocol in unintended ways.

While this issue does not impact Compound directly as the manager is the Compound Timelock, other protocols that integrate CAPO may be vulnerable to this attack vector.

Attack Scenario

  • Assume block.timestamp = 150, snapshotTimestamp = 100 and minimumSnapshotDelay = 50.
  • The next block has block.timestamp = 200.
  • The manager sets snapshotTimestamp = 101 and modifies snapshotRatio to an advantageous value.
  • The manager executes a malicious action based on the new ratio.
  • The manager then sets snapshotTimestamp = 150, reverting snapshotRatio to the original value.

Consider implementing a check to ensure that the snapshot timestamp cannot be updated more than once within the same block. This could be achieved by storing the last update’s block number and preventing updates if block.number is the same as the last recorded block.

Missing Docstrings

Within PriceCapAdapterBase.sol, multiple instances of missing docstrings were identified:

Consider thoroughly documenting all functions (and their parameters) that are part of any contract’s public API. Functions implementing sensitive functionality, even if not public, should be clearly documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

Unclear maxRatioGrowthPerSecond Calculation

The maxRatioGrowthPerSecond value is calculated in the setSnapshot function of the PriceAdapterBase contract. From the code, it is clear that the priceCapParams.maxYearlyRatioGrowthPercent input parameter should be provided in basis points. However, there is no indication that the provided value should be formatted in this way and the basis points denominator is obtained by multiplying the PERCENTAGE_DECIMALS constant with 100, which makes the calculation confusing for the reader.

Consider replacing PERCENTAGE_DECIMALS with a constant named BASIS_POINTS which has the value 1e4. The new constant can then be used within the maxRatioGrowthPerSecond calculation to replace the (100 * PERCENTAGE_DECIMALS) statement. In addition, consider expanding the NatSpec to indicate that the maxYearlyRatioGrowthPercent must be provided in basis points.

Notes & Additional Information

Misleading Comment in latestRoundData Function

The comment in the latestRoundData function states: @return answer Latest price for the asset in terms of ETH. However, the price is not necessarily denominated in ETH-it could be in USD or another base asset, depending on the aggregator used.

Consider updating the comment to reflect that the returned price is in terms of the underlying base asset, which may vary. This will improve clarity and prevent misunderstanding when integrating or auditing the contract.

ratioDecimals Should Be Retrieved Dynamically

In the getRatio function, ratioDecimals represents the decimal precision of the ratio provider, which, in this case, is the ERC-4626 token. Currently, ratioDecimals is manually provided as an input in the constructor, which introduces the risk of incorrect initialization and unnecessary complexity.

Instead of manually setting ratioDecimals, consider retrieving it dynamically by calling the decimals() function of the rate provider. This would ensure that the contract always uses the correct decimal precision, reducing the likelihood of errors.

Functions Updating State Without Event Emissions

Within PriceCapAdapterBase.sol, the setManager function updates the state without emitting an event.

Consider emitting an event whenever performing a state change. This will help improve the clarity of the codebase and make it less error-prone.

Inconsistent Formatting

Throughout the PriceAdapterBase contract, multiple instances of inconsistent formatting were identified:

Consider using a consistent code style formatting throughout the contract to enhance its readability and avoid confusion.

Inconsistent Naming Convention

The internal functions within the PriceCapAdapterBase contract are inconsistently named. For example, the _getMaxRatio and the _setSnapshot internal functions are named with a leading underscore. In contrast to this, the signed256 and scalePrice internal functions do not use the leading underscore naming convention.

Consider applying a consistent naming convention throughout the codebase to enhance code clarity and readability.

Conclusion

The CAPO contract introduces a safeguard mechanism to limit excessive price inflation in Liquid Staking and Restaking Tokens by capping the growth of the exchange rate over time. While the implementation is generally sound and aligns with its intended purpose, several security concerns were identified.

Specifically, two medium-severity issues along with several low-severity ones were identified, primarily related to precision handling, governance assumptions, and optimizations. In addition, one high-severity vulnerability was discovered, related to the possibility of executing a large single-block price inflation due to the method with which the maximum ratio is calculated, which permits ratio inflation relative to the time passed since the last timestamp.

1 Like
2

Thanks for findings and hard work! We will consider your finding and update the codebase accordingly.