Juicebox V2 contest - Cheeezzyyyy's results

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBChainlinkV3PriceFeed.sol#L42-L51

Vulnerability details

Impact

Insufficient validation of data feed to check if the data returned is stale which can potentially cause cascading erroneous calculations and state updates in payment, distribution and allowances.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Inside JBChainlinkV3PriceFeed.sol, latestRoundData is being used in currentPrice to fetch the latest price of ETH/USD. However, there is insufficient validation of data feed to check if the data returned is stale.

This can result in an incorrect price data returned when priceFor inside JBPrices.sol is called, which will cause wrong calculations inside recordPaymentFrom, recordDistributionFor, recordUsedAllowanceOf, _overflowingDuring and _currentTotalOverflowOf inside JBSingleTokenPaymentTerminalStore.sol.

Code: #JBChainlinkV3PriceFeed.sol#L42-51

List of Functions that will be affected from potential stale data: #JBSingleTokenPaymentTerminalStore.sol#L387 #JBSingleTokenPaymentTerminalStore.sol#L585 #JBSingleTokenPaymentTerminalStore.sol#L661 #JBSingleTokenPaymentTerminalStore.sol#L830 #JBSingleTokenPaymentTerminalStore.sol#L868

Tools Used

• Foundry
• Hardhat

Recommended Mitigation Steps

Validate data feed appropriately.

  function currentPrice(uint256 _decimals) external view override returns (uint256) {
      // Get the latest round information. Only need the price is needed.
      (uint80 roundID, int256 _price, ,uint256 timestamp, uint80 answeredInRound ) = feed.latestRoundData();

       require(answer > 0, "Chainlink: Invalid oracle answer");
       require(answeredInRound > roundID, "Chainlink: Stale price");
       require(timestamp > 0, "Chainlink: Round not complete");

      // Get a reference to the number of decimals the feed uses.
      uint256 _feedDecimals = feed.decimals();

      // Return the price, adjusted to the target decimals.
      return uint256(_price).adjustDecimals(_feedDecimals, _decimals);
    }

Unchecked Arithmetic can be used in incrementing the looping counter so save gas

Inside JBController.sol: #JBController.sol#L913 #JBController.sol#L1014

Inside JBDirectory.sol: #JBDirectory.sol#L139 #JBDirectory.sol#L167 #JBDirectory.sol#L275-L276

Inside JBETHERERC20SplitsPayer.sol: #JBETHERC20SplitsPayer.sol#L466

Inside JBFundingCycleStore.sol: #JBFundingCycleStore.sol#L724

Inside JBOperatorStore.sol: #JBOperatorStore.sol#L85 #JBOperatorStore.sol#L135 #JBOperatorStore.sol#L165

Inside JBSingleTokenPaymentTerminalStore.sol: #JBSingleTokenPaymentTerminalStore.sol#L862

Inside JBSplitsStore.sol #JBSplitsStore.sol#L165 #JBSplitsStore.sol#L204 #JBSplitsStore.sol#L211 #JBSplitsStore.sol#L229 #JBSplitsStore.sol#L304

Mitigation Steps:

Replace

  _i++

with

  unchecked {
    _i++;
  }