diff --git a/audits/192_Core_pool_reaudit_certik_20260616.pdf b/audits/192_Core_pool_reaudit_certik_20260616.pdf new file mode 100644 index 00000000..4b9e3aec Binary files /dev/null and b/audits/192_Core_pool_reaudit_certik_20260616.pdf differ diff --git a/contracts/DeviationBoundedOracle.sol b/contracts/DeviationBoundedOracle.sol index ef7ca7cd..5a038b06 100644 --- a/contracts/DeviationBoundedOracle.sol +++ b/contracts/DeviationBoundedOracle.sol @@ -24,10 +24,15 @@ import { Transient } from "./lib/Transient.sol"; * collateral tokens. Sustained attacks beyond the window period are expected to be handled by * off-chain monitoring systems. * - * The oracle exposes both view and non-view price functions. The non-view variants update the - * price window and trigger protection. The view variants read stored state only. A transient - * price cache avoids redundant ResilientOracle calls within the same transaction when - * updateProtectionState is called before the view price reads. + * Non-view price functions update the window and trigger protection; view functions never mutate + * state and recompute from the live spot. An optional per-asset transient cache skips redundant + * ResilientOracle calls within a transaction: when enabled, the first computation populates it and + * every later read in the same transaction returns that value instead of recomputing. + * + * The cache freezes the first value for the rest of the transaction, so enable it only for assets + * whose price cannot move within a single transaction (e.g. Chainlink feeds, updated once per block). + * Leave it disabled for assets with a movable spot (e.g. AMM-derived feeds); otherwise a stale cached + * value could skip protection. */ contract DeviationBoundedOracle is AccessControlledV8, IDeviationBoundedOracle { /// @notice Minimum allowed threshold value (5%) to account for keeper deadband diff --git a/contracts/ResilientOracle.sol b/contracts/ResilientOracle.sol index 8e16b33f..61e7376a 100755 --- a/contracts/ResilientOracle.sol +++ b/contracts/ResilientOracle.sol @@ -334,7 +334,9 @@ contract ResilientOracle is PausableUpgradeable, AccessControlledV8, ResilientOr /** * @notice Updates the capped oracle snapshot. - * @dev Cache the asset price and return if already cached + * @dev Cache the asset price and return if already cached. + * `updateSnapshot()` may revert for oracles other than `CorrelatedTokenOracle`; + * the empty catch block intentionally swallows that revert and is harmless. * @param asset asset address */ function _updateAssetPrice(address asset) internal { diff --git a/contracts/oracles/OneJumpOracle.sol b/contracts/oracles/OneJumpOracle.sol index 8c3acdfb..26c38b5c 100644 --- a/contracts/oracles/OneJumpOracle.sol +++ b/contracts/oracles/OneJumpOracle.sol @@ -13,6 +13,7 @@ import { IERC20Metadata } from "@openzeppelin/contracts/token/ERC20/extensions/I */ contract OneJumpOracle is CorrelatedTokenOracle { /// @notice Address of the intermediate oracle + /// @dev Must return UNDERLYING_TOKEN per CORRELATED_TOKEN, not USD denominated price. OracleInterface public immutable INTERMEDIATE_ORACLE; /// @notice Constructor for the implementation contract. diff --git a/contracts/oracles/common/CorrelatedTokenOracle.sol b/contracts/oracles/common/CorrelatedTokenOracle.sol index accc64d1..8f12e042 100644 --- a/contracts/oracles/common/CorrelatedTokenOracle.sol +++ b/contracts/oracles/common/CorrelatedTokenOracle.sol @@ -66,6 +66,9 @@ abstract contract CorrelatedTokenOracle is OracleInterface, ICappedOracle { /// @notice Thrown if the max snapshot exchange rate is invalid error InvalidSnapshotMaxExchangeRate(); + /// @notice Thrown if the snapshot timestamp is invalid + error InvalidSnapshotTimestamp(); + /// @notice @notice Thrown when the action is prohibited by AccessControlManager error Unauthorized(address sender, address calledContract, string methodSignature); @@ -115,11 +118,20 @@ abstract contract CorrelatedTokenOracle is OracleInterface, ICappedOracle { * @notice Directly sets the snapshot exchange rate and timestamp * @param _snapshotMaxExchangeRate The exchange rate to set * @param _snapshotTimestamp The timestamp to set + * @custom:error InvalidSnapshotMaxExchangeRate error is thrown if the max snapshot exchange rate is zero while + * the snapshot interval is active (a zero cap would silently disable the growth cap) + * @custom:error InvalidSnapshotTimestamp error is thrown if the snapshot timestamp is zero or in the future while + * the snapshot interval is active (a future timestamp would underflow getMaxAllowedExchangeRate and revert pricing) * @custom:event Emits SnapshotUpdated event on successful update of the snapshot */ function setSnapshot(uint256 _snapshotMaxExchangeRate, uint256 _snapshotTimestamp) external { _checkAccessAllowed("setSnapshot(uint256,uint256)"); + if (snapshotInterval != 0) { + if (_snapshotMaxExchangeRate == 0) revert InvalidSnapshotMaxExchangeRate(); + if (_snapshotTimestamp == 0 || _snapshotTimestamp > block.timestamp) revert InvalidSnapshotTimestamp(); + } + snapshotMaxExchangeRate = _snapshotMaxExchangeRate; snapshotTimestamp = _snapshotTimestamp; diff --git a/test/oracles/CorrelatedTokenOracleTest.ts b/test/oracles/CorrelatedTokenOracleTest.ts index b1ae312e..0b6387ee 100644 --- a/test/oracles/CorrelatedTokenOracleTest.ts +++ b/test/oracles/CorrelatedTokenOracleTest.ts @@ -247,18 +247,47 @@ describe("CorrelatedTokenOracle", () => { expect(price).to.be.equal(ethers.utils.parseUnits("100.0000003170979198", 18)); }); - it("zero max allowed exchange rate", async () => { + it("reverts when setting a zero max allowed exchange rate while capping is active", async () => { await correlatedTokenOracle.updateSnapshot(); const price = await correlatedTokenOracle.getPrice(correlatedToken.address); expect(price).to.equal(ethers.utils.parseUnits("10", 18)); - // Set the max allowed exchange rate to zero + // A zero max-rate would make getMaxAllowedExchangeRate() return 0 and silently disable the cap (DS2-88) const currentBlock = await ethers.provider.getBlock("latest"); const currentTimestamp = currentBlock.timestamp; - await correlatedTokenOracle.setSnapshot(0, currentTimestamp); + await expect(correlatedTokenOracle.setSnapshot(0, currentTimestamp)).to.be.revertedWithCustomError( + correlatedTokenOracle, + "InvalidSnapshotMaxExchangeRate", + ); + }); - // eslint-disable-next-line no-unused-expressions - expect(await correlatedTokenOracle.isCapped()).to.be.false; + it("reverts when setting a future snapshot timestamp while capping is active", async () => { + // A future timestamp would underflow block.timestamp - snapshotTimestamp and DoS pricing (DS2-87) + const currentBlock = await ethers.provider.getBlock("latest"); + const currentTimestamp = currentBlock.timestamp; + await expect( + correlatedTokenOracle.setSnapshot(exchangeRate, currentTimestamp + 1000), + ).to.be.revertedWithCustomError(correlatedTokenOracle, "InvalidSnapshotTimestamp"); + }); + + it("reverts when setting a zero snapshot timestamp while capping is active", async () => { + // A zero timestamp would make timeElapsed equal block.timestamp, inflating the cap to never bind + await expect(correlatedTokenOracle.setSnapshot(exchangeRate, 0)).to.be.revertedWithCustomError( + correlatedTokenOracle, + "InvalidSnapshotTimestamp", + ); + }); + + it("allows a zero snapshot when capping is disabled", async () => { + // Disable capping (snapshotInterval = 0); snapshot fields are then irrelevant and unvalidated + await correlatedTokenOracle.setGrowthRate(0, 0); + + await expect(correlatedTokenOracle.setSnapshot(0, 0)) + .to.emit(correlatedTokenOracle, "SnapshotUpdated") + .withArgs(0, 0); + + expect(await correlatedTokenOracle.snapshotMaxExchangeRate()).to.equal(0); + expect(await correlatedTokenOracle.snapshotTimestamp()).to.equal(0); }); }); }); diff --git a/yarn.lock b/yarn.lock index e2039ac2..c5c4c74d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4131,7 +4131,29 @@ __metadata: languageName: node linkType: hard -"@venusprotocol/oracle@^2.7.0, @venusprotocol/oracle@workspace:.": +"@venusprotocol/oracle@npm:^2.7.0": + version: 2.15.0 + resolution: "@venusprotocol/oracle@npm:2.15.0" + dependencies: + "@chainlink/contracts": ^0.5.1 + "@defi-wonderland/smock": 2.4.0 + "@nomicfoundation/hardhat-network-helpers": ^1.0.8 + "@openzeppelin/contracts": ^4.6.0 + "@openzeppelin/contracts-upgradeable": ^4.7.3 + "@venusprotocol/governance-contracts": ^2.13.0 + "@venusprotocol/solidity-utilities": ^2.0.0 + "@venusprotocol/venus-protocol": ^9.7.0 + ethers: ^5.6.8 + hardhat: 2.22.18 + hardhat-deploy: ^0.12.4 + module-alias: ^2.2.2 + patch-package: ^8.0.0 + solidity-docgen: ^0.6.0-beta.29 + checksum: c1d6a6eacedc9f6883a5a2cc85530fe5e9390ecb9cfcc0f526f09cf059ea25098c1c9e208767e74f73cd8e13ae04bb4acd6f6a43431db8686a0e75bd72794af9 + languageName: node + linkType: hard + +"@venusprotocol/oracle@workspace:.": version: 0.0.0-use.local resolution: "@venusprotocol/oracle@workspace:." dependencies: