[VPD 1241] Certik: Venus Labs - Core Feature Reaudit

contracts/DeviationBoundedOracle.sol

@@ -24,10 +24,15 @@
  * 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
@@ -45,11 +50,11 @@
 
     /// @notice Native market address
     /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address public immutable nativeMarket;
 
     /// @notice VAI address
     /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address public immutable vai;
 
     /// @notice Transient storage slot for caching final collateral prices within a transaction
     /// @dev custom:storage-location erc7201:venus-protocol/oracle/DeviationBoundedOracle/collateralCache
@@ -220,7 +225,7 @@
      * @custom:access Only authorized keeper addresses
      * @custom:event MinPriceUpdated
      */
     function updateMinPrice(address asset, uint128 newMin) external {
         _checkAccessAllowed("updateMinPrice(address,uint128)");
         _validateAndUpdateBound(asset, newMin, PriceBoundType.MIN);
     }
@@ -491,7 +496,7 @@
         MarketProtectionState storage state = assetProtectionConfig[asset];
         return
             state.currentlyUsingProtectedPrice &&
             block.timestamp >= uint256(state.lastProtectionTriggeredAt) + uint256(state.cooldownPeriod) &&
             _computePriceBoundRatio(state.minPrice, state.maxPrice) < state.resetThreshold;
     }
 
@@ -629,7 +634,7 @@
 
         if (!state.currentlyUsingProtectedPrice) revert ProtectedPriceInactive(asset);
 
         if (block.timestamp < uint256(state.lastProtectionTriggeredAt) + uint256(state.cooldownPeriod)) {
             revert CooldownNotElapsed(asset, state.lastProtectionTriggeredAt, state.cooldownPeriod);
         }
 
@@ -717,7 +722,7 @@
         if (_exceedsDeviationThreshold(spot, state.minPrice, state.maxPrice, state.triggerThreshold)) {
             bool enteringProtection = !state.currentlyUsingProtectedPrice;
             if (enteringProtection || windowExpanded) {
                 state.lastProtectionTriggeredAt = uint64(block.timestamp);
             }
             if (enteringProtection) {
                 state.currentlyUsingProtectedPrice = true;

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 {

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.

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;
 

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);
     });
   });
 });

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: