Core Contracts

Table of Contents

1. Introduction

2. Scope

3. Methodology

4. Findings

   • Critical

   • High

   • Medium

   • Low

5. Recommendations

6. Conclusion

---

Introduction

The LendingPool contract is a core component of a decentralized lending protocol developed by Cyfrin. It enables users to deposit NFTs (specifically, RAACNFT) as collateral to borrow assets, such as crvUSD. The contract integrates with several other contracts, including RTokens for lending, DebtTokens for borrowing, and a ReserveLibrary for managing interest rates and liquidity.

This audit aims to identify potential security vulnerabilities, logic errors, and optimization opportunities within the LendingPool.sol contract. Given its critical role in managing user funds and collateral, ensuring its security and reliability is paramount.

---

Scope

This audit focuses on the LendingPool.sol contract, located in the contracts/core/pools/LendingPool/ directory of the repository https://github.com/Cyfrin/2025-02-raac.git. The analysis is based on the code available as of February 23, 2025. While other contracts in the repository (e.g., ReserveLibrary.sol, RAACNFT.sol) were not directly audited, their interactions with LendingPool.sol were considered where relevant.

---

Methodology

The audit was conducted through a manual review of the Solidity code, emphasizing common smart contract vulnerabilities, best practices, and DeFi-specific risks. The review process included:

• Analysis of the contract’s logic and functionality.

• Identification of potential security flaws, such as reentrancy, access control issues, and oracle manipulation.

• Assessment of gas efficiency and optimization opportunities.

• Verification of adherence to relevant standards (e.g., ERC-20, ERC-721).

This audit is a static analysis; no dynamic testing or deployment was performed. The findings are informed by expertise in blockchain security and knowledge of historical exploits.

---

Findings

The findings are categorized by severity: Critical, High, Medium, and Low. Each issue includes a description, evidence from the code, and a recommended remediation.

Severity: Critical

1. Oracle Manipulation Risk in getNFTPrice

• Description: The contract relies on an external oracle (IRAACHousePrices) for NFT pricing without validating the freshness or integrity of the data. This exposes the protocol to oracle manipulation or stale data risks, potentially leading to incorrect collateral valuations.

• Evidence:

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

  return priceOracle.getPrice(tokenId);

  }

• Impact: Attackers could exploit manipulated or outdated prices to borrow excessive amounts or avoid liquidation, risking protocol insolvency.

• Recommendation: Implement timestamp checks to ensure price data is recent (e.g., within the last hour) and consider using multiple oracles or a decentralized oracle network like Chainlink.

Severity: High

1. Reentrancy Risk in withdrawNFT

• Description: The withdrawNFT function transfers the NFT before updating the contract’s state, and the transfer direction appears incorrect. Although nonReentrant is used, improper state management could still allow reentrancy if the NFT contract triggers a callback.

• Evidence:

  raacNFT.safeTransferFrom(msg.sender, address(this), tokenId);

  This likely contains a typo; it should transfer from the contract to the user. State updates should precede external calls.

• Impact: Incorrect transfer direction prevents users from receiving their NFTs. If a callback is triggered, it could enable reentrancy attacks.

• Recommendation: Correct the transfer to raacNFT.safeTransferFrom(address(this), msg.sender, tokenId) and ensure all state updates occur before external calls.

2. Collateral Valuation Logic Incomplete

• Description: The collateral check in withdrawNFT lacks a liquidation threshold or buffer, which is essential to maintain overcollateralization in lending protocols.

• Evidence:

  require(collateralValue - nftValue >= userDebt, "Insufficient collateral");

• Impact: Without a safety margin (e.g., 150% collateralization), the protocol risks undercollateralization if NFT prices drop slightly after withdrawal.

• Recommendation: Introduce a health factor or require collateral to exceed debt by a predefined ratio (e.g., 150%).

Severity: Medium

1. Array Management Oversight in withdrawNFT

• Description: The user.nftTokenIds array is not updated when an NFT is withdrawn, leaving stale data that could cause logic errors in dependent functions.

• Evidence: No logic exists to remove tokenId from user.nftTokenIds after withdrawal.

• Impact: Stale data could lead to incorrect collateral valuations or other miscalculations, potentially causing financial discrepancies.

• Recommendation: Implement array removal logic (e.g., swap-and-pop) to maintain accurate state.

2. Missing Access Control on Sensitive Functions

• Description: Critical functions like updateOwnerParameter are protected only by onlyOwner, lacking additional safeguards such as timelocks or multisigs.

• Evidence:

  function updateOwnerParameter(OwnerParameter param, uint256 newValue) external onlyOwner { ... }

• Impact: A compromised owner key could lead to unauthorized parameter changes, disrupting the protocol or causing financial loss.

• Recommendation: Integrate a timelock or multisig wallet for ownership to enhance security.

3. Unchecked External Calls in Constructor

• Description: The constructor assigns external contract addresses without verifying their validity or interface compatibility, risking runtime errors.

• Evidence: No checks are performed on addresses like _reserveAssetAddress or _raacNFTAddress.

• Impact: Invalid addresses could cause functions to fail, locking funds or breaking protocol functionality.

• Recommendation: Add checks to ensure addresses are contracts and implement expected interfaces (e.g., by calling a view function).

Serverity: Low

1. Gas Optimization Opportunity

• Description: Frequent state updates and inefficient data structures (e.g., arrays for tracking NFTs) increase gas costs for users.

• Evidence: Multiple state reads and writes in functions like withdrawNFT, with growing arrays like nftTokenIds.

• Impact: High gas costs may deter users, especially during network congestion.

• Recommendation: Optimize state updates by batching operations and consider more gas-efficient data structures or off-chain tracking.

---

Recommendations

• Oracle Manipulation Risk: Add timestamp validation for oracle prices and explore decentralized oracle solutions.

• Reentrancy Risk: Correct the NFT transfer direction and reorder state updates to precede external calls.

• Collateral Valuation: Implement a liquidation threshold to ensure overcollateralization.

• Array Management: Remove withdrawn NFTs from the nftTokenIds array.

• Access Control: Use a timelock or multisig for critical owner functions.

• Unchecked External Calls: Validate external contract addresses in the constructor.

• Gas Optimization: Review and optimize state management and data structures.

---

Conclusion

The LendingPool contract provides a solid foundation for a lending protocol but contains critical and high-severity issues that must be addressed prior to deployment. The most pressing concerns—oracle manipulation and reentrancy risks—could lead to significant financial losses if exploited. Additionally, enhancements in access control and gas efficiency would improve the contract’s security and usability.

The development team is advised to prioritize fixing the critical and high-severity issues and to consider a professional audit from a reputable firm for further validation. Comprehensive testing, including dynamic analysis and attack simulations, is also recommended.

Disclaimer: This audit is based on a static analysis and should be considered a preliminary review. The auditor disclaims liability for any undetected issues or future vulnerabilities. This report is for educational and informational purposes only and does not constitute financial or legal advice.