Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Lack of staleness check in getNFTPrice() can lead to protocol loss

t0x1c

Summary

getNFTPrice() does not check for price staleness even though it claims so:

File: contracts/core/pools/LendingPool/LendingPool.sol
​
584: /**
585: * @notice Gets the current price of an NFT from the oracle
586: * @param tokenId The token ID of the NFT
587: * @return The price of the NFT
588: *
589:@---> * Checks if the price is stale
590: */
591: function getNFTPrice(uint256 tokenId) public view returns (uint256) {
592:@---> (uint256 price, uint256 lastUpdateTimestamp) = priceOracle.getLatestPrice(tokenId);
593: if (price == 0) revert InvalidNFTPrice();
594: return price;
595: }

The flow of fetching price is:

  1. Chainlink Oracle -> RAACHousePriceOracle

  2. RAACHousePriceOracle -> RAACHousePrices

  3. RAACHousePrices -> LendingPool::getNFTPrice()

Description & Impact

Since getNFTPrice() is internally called by critical functions withdrawNFT() and getUserCollateralValue() (which is internally called by borrow() etc.) an outdated price can result in the user borrowing more than allowed, thus profiting and putting the protocol underwater.

Attack Path:

  • Monitor old price: 100 ETH --> Wait for market drop --> Real value now 60 ETH (suppose that price drop happened over 24 hours)

  • Buy NFT for 60 ETH from the open market

  • Deposit NFT in LendingPool

  • Borrow 80 ETH against NFT, since price reported by getNFTPrice() is still 100 ETH

  • Default on loan

  • Keep 20 ETH profit

Mitigation

Add a check which verifies the age. Something along these lines:

function getNFTPrice(uint256 tokenId) public view returns (uint256) {
(uint256 price, uint256 lastUpdateTimestamp) = priceOracle.getLatestPrice(tokenId);
if (price == 0) revert InvalidNFTPrice();
+ uint256 MAX_PRICE_AGE = 24 hours; // Or other appropriate duration which could be based on the type of NFT and derived from a mapping
+ if (block.timestamp - lastUpdateTimestamp > MAX_PRICE_AGE) {
+ revert StalePrice();
+ }
return price;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::getNFTPrice or getPrimeRate doesn't validate timestamp staleness despite claiming to, allowing users to exploit outdated collateral values during price drops

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::getNFTPrice or getPrimeRate doesn't validate timestamp staleness despite claiming to, allowing users to exploit outdated collateral values during price drops

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.