Core Contracts

Finding Description and Impact

The getNFTPrice function in the LendingPool contract relies on the RAACHousePrices contract to retrieve the latest price and timestamp for a given NFT. However, the RAACHousePrices contract uses a global lastUpdateTimestamp for all tokens instead of maintaining token-specific timestamps. This design flaw leads to the following issues:

1. Outdated Price Data:

   • The lastUpdateTimestamp returned by RAACHousePrices does not reflect the actual last update time for the specific token being queried. Instead, it reflects the latest update time for any token in the system.

   • As a result, the price returned by getNFTPrice could be outdated, even if the timestamp appears recent.

2. Incorrect Collateral Calculations:

   • The withdrawNFT function uses getNFTPrice to determine the value of the NFT being withdrawn. If the price is outdated, the collateral check (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) could be bypassed or unfairly enforced.

   • This could allow users to withdraw NFTs even if it leaves them undercollateralized, or prevent users from withdrawing NFTs even if they are sufficiently collateralized.

Proof of Concept

1. getNFTPrice in LendingPool:

   function getNFTPrice(uint256 tokenId) public view returns (uint256) {

   (uint256 price, uint256 lastUpdateTimestamp) = priceOracle.getLatestPrice(tokenId);

   if (price == 0) revert InvalidNFTPrice();

   return price;

   }

2. getLatestPrice in RAACHousePrices:

   function getLatestPrice(uint256 _tokenId) external view returns (uint256, uint256) {

   return (tokenToHousePrice[_tokenId], lastUpdateTimestamp);

   }

3. setHousePrice in RAACHousePrices:

   function setHousePrice(uint256 _tokenId, uint256 _amount) external onlyOracle {

   tokenToHousePrice[_tokenId] = _amount;

   lastUpdateTimestamp = block.timestamp; // Updates global timestamp, not token-specific

   emit PriceUpdated(_tokenId, _amount);

   }

Scenario

1. Suppose tokenA was last updated 30 days ago with a price of 100 ETH.

2. tokenB is updated today with a price of 200 ETH.

3. When querying the price of tokenA, the lastUpdateTimestamp returned will be today's timestamp (from tokenB's update), even though tokenA's price is outdated.

4. The getNFTPrice function will return the outdated price of tokenA (100 ETH) without any indication that the price is stale.

Impact

• If a user tries to withdraw tokenA, the protocol will use the outdated price (100 ETH) to calculate collateralization. If the actual market value of tokenA has dropped to 50 ETH, the user could withdraw the NFT even though it leaves them undercollateralized, putting the protocol at risk of bad debt.

Recommended Mitigation Steps

1. Implement Token-Specific Timestamps:

   • Modify the RAACHousePrices contract to maintain a separate lastUpdateTimestamp for each token.

   mapping(uint256 => uint256) public tokenToLastUpdateTimestamp;

   • Update the setHousePrice function to set the token-specific timestamp:

   function setHousePrice(uint256 _tokenId, uint256 _amount) external onlyOracle {

   tokenToHousePrice[_tokenId] = _amount;

   tokenToLastUpdateTimestamp[_tokenId] = block.timestamp;

   emit PriceUpdated(_tokenId, _amount);

   }

   • Update the getLatestPrice function to return the token-specific timestamp:

   function getLatestPrice(uint256 _tokenId) external view returns (uint256, uint256) {

   return (tokenToHousePrice[_tokenId], tokenToLastUpdateTimestamp[_tokenId]);

   }

2. Add Stale Price Check in getNFTPrice:

   • Add a check to ensure the price is not outdated before returning it.

   function getNFTPrice(uint256 tokenId) public view returns (uint256) {

   (uint256 price, uint256 lastUpdateTimestamp) = priceOracle.getLatestPrice(tokenId);

   if (price == 0) revert InvalidNFTPrice();

   if (block.timestamp - lastUpdateTimestamp > MAX_PRICE_AGE) revert PriceTooOld();

   return price;

   }

By implementing these changes, the protocol can ensure that NFT prices are accurate and up-to-date, reducing the risk of financial losses and improving overall security.