c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/PriceFeed.sol#L98

Vulnerability details

Impact

When there are multiple requests(that has price request of fetchPrice) in the same block, there is possibility of different prices. It is expected to return the same price within a block.

fetchPrice is indirectly accessed in the following public functions: addColl, withdrawColl, withdrawDebt, repayDebt, adjustCdp, adjustCdpWithColl, liquidate, partiallyLiquidate, syncGlobalAccountingAndGracePeriod, setGracePeriod, redeemCollateral, redeemCollateral. When there are multiple requests within a block, it is possible the fetchPrice can send a different price based on CASE 1 to CASE 5 inside fetchPrice method.

Gas saving in subsequent calls to fetchPrice within a block

Proof of Concept

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/PriceFeed.sol#L98

Tools Used

Manual review

Recommended Mitigation Steps

File: contracts/PriceFeed.sol

45:    uint256 public lastGoodPrice;

       // @audit: The last good price's timestamp
       uint256 public lastGoodPriceTimestamp;

98:    function fetchPrice() external override returns (uint256) {
           // audit: early return when requests is within same block
           if(block.timestamp == lastGoodPriceTimestamp) {
               return lastGoodPrice;
           }

553:    function _storePrice(uint256 _currentPrice) internal {
554:        lastGoodPrice = _currentPrice;
            // @audit: update lastGoodPriceTimestamp 
            lastGoodPriceTimestamp = block.timestamp;