reNFT - 0xepley's results

🛠️ Analysis - ReNFT

Collateral-free, permissionless, and highly customizable EVM NFT rentals.

Summary

a) Overview of the reNFT Project

reNFT is a blockchain protocol designed for renting Non-Fungible Tokens (NFTs). It's structured to facilitate the renting process for NFT owners and renters, offering a secure and user-friendly platform.

Key Features:

1. Modular Architecture: Separate contracts handle specific functions like payment escrow and storage.
2. Integrations: Incorporates Seaport for order processing and Gnosis Safe for wallet management.
3. Security Focus: Prioritizes the safety of rented assets and secure transaction handling.
4. Efficient Rental Process: Manages the rental lifecycle effectively.
5. Access Control: Implements role-based access for operational security.
6. Token Support: Compatible with ERC20, ERC721, and ERC1155 tokens.

b) The approach I would follow when reviewing the code

First, by examining the scope of the code, I determined my code review and analysis strategy. https://code4rena.com/audits/2024-01-renft

Accordingly, I would analyze and audit the subject in the following steps;

c) Analysis of the code base

The most important summary in analyzing the code base is the stacking of codes to be analyzed. In this way, many predictions can be made, including the difficulty levels of the contracts, which one is more important for the auditor, the features they contain that are important for security (payable functions, uses assembly, etc.), the audit cost of the project, and the time to be allocated to the audit; Uses Consensys Solidity Metrics

• Language: language in which of the source is written
• nSLOC: normalized source lines of code (only source-code lines; no comments, no blank lines)
• Comment Lines: lines containing single or block comments
• Blank Lines: Blank Lines in the source file
• Total Lines: Total number of lines in the file

Analysis of sloc of modules contracts

Analysis of sloc of Pakages contracts

Analysis of sloc of Policies contracts

Comment-to-Source Ratio:

Module contracts: On average there are 0.8 code lines per comment (lower=better).

Packages contracts: On average there are 1.27 code lines per comment (lower=better).

Policies contracts: On average there are 1.35 code lines per comment (lower=better).

Call Graph of Important Contracts

Call graph of Kernel.sol

Call graph of Stop.sol

Call graph of Guard.sol

Call graph of Create.sol

UML diagram of important Contracts

UML diagram of PaymentEscrow.sol

UML diagram of Create.sol

UML diagram of Signer.sol

UML diagram of Guard.sol

Sequence diagram of the protocol

Sequence diagrams are a good way to understand the overall interaction of the classes or files in the protocol

Sequence diagrams of the important functions of the protocol

ValidateOrder()

_settlePayment()

stopRent()

d) Test analysis

What did the project do differently? ;

• 1. It can be said that the developers of the project did a quality job, there is a test structure consisting of tests with quality content. In particular, tests have been written successfully.
• 2. Overall line coverage percentage provided by your tests : 80

What could they have done better?

• 1. In order to understand the test scenarios and develop more effective test scenarios, the following bob, alice and other roles are can be defined one by one, in this way role definitions increase the quality and readability in tests

 // Sample labels
vm.label(bob, 'bob');
vm.label(alice, 'alice');
vm.label(DEPLOYER, 'deployer');
vm.label(USD_OWNER, 'usd owner');
vm.label(POOL_PROXY, 'lending pool');

• 2. If we look at the test scope and content of the project with a systematic checklist, we can see which parts are good and which areas have room for improvement As a result of my analysis, those marked in green are the ones that the project has fully achieved. The remaining areas are the development areas of the project in terms of testing ;

Ref:https://xin-xia.github.io/publication/icse194.pdf

• 3. Test Coverage of the protocol is around 80% which is very less, the recommended test coverage of any protocol is above 90% so it is recommended to increase the coverage to at least 90%

Test-case analysis of Contracts

General Information

• Solc Version: 0.8.22
• Block Limit: 30,000,000 gas

Storage.sol: Storage contract

Deployment Cost: 1,553,625 Deployment Size: 8,094

Admin.sol: Admin contract

Deployment Cost: 921,484 Deployment Size: 4,670

Create.sol: Create contract

Deployment Cost: 2,653,662 Deployment Size: 15,175

Factory.sol: Factory contract

Deployment Cost: 692,196 Deployment Size: 3,906

Guard.sol: Guard contract

Deployment Cost: 1,030,196 Deployment Size: 5,213

Stop.sol: Stop contract

Deployment Cost: 2,167,126 Deployment Size: 12,705

e) Security Approach of the Project

Successful current security understanding of the project;

1- The project hasn't underwent any audits, this innovative assessments on Code4rena is the first, where multiple auditors are scrutinizing the code.

What the project should add in the understanding of Security;

1- By distributing the project to testnets, ensuring that the audits are carried out in onchain audit. (This will increase coverage)

2- Add On-Chain Monitoring System; If On-Chain Monitoring systems such as Forta are added to the project, its security will increase.

For example ; This bot tracks any DEFI transactions in which wrapping, unwrapping, swapping, depositing, or withdrawals occur over a threshold amount. If transactions occur with unusually high token amounts, the bot sends out an alert. https://app.forta.network/bot/0x7f9afc392329ed5a473bcf304565adf9c2588ba4bc060f7d215519005b8303e3

3- After the Code4rena audit is completed and the project is live, I recommend the audit process to continue, projects like immunefi do this. https://immunefi.com/

4- Emergency Action Plan In a high-level security approach, there should be a crisis handbook like the one below and the strategic members of the project should be trained on this subject and drills should be carried out. Naturally, this information and road plan will not be available to the public. https://docs.google.com/document/u/0/d/1DaAiuGFkMEMMiIuvqhePL5aDFGHJ9Ya6D04rdaldqC0/mobilebasic#h.27dmpkyp2k1z

5- I also recommend that you have an "Economic Audit" for projects based on such complex mathematics and economic models. An example Economic Audit is provided in the link below; Economic Audit with Three Sigma

6 - As the project team, you can consider applying the multi-stage audit model.

Read more about the MPA model; https://mpa.solodit.xyz/

7 - I recommend having a masterplan applied to project team members (This information is not included in the documents). All authorizations, including NPM passwords and authorizations, should be reserved only for current employees. I also recommend that a definitive security constitution project be found for employees to protect these passwords with rules such as 2FA. The LEDGER hack, which has made a big impact recently, is the best example in this regard;

https://twitter.com/Ledger/status/1735326240658100414?t=UAuzoir9uliXplerqP-Ing&s=19

f) Codebase Quality

Overall, I consider the quality of the ReNFT protocol codebase to be Good. The code appears to be mature and well-developed. We have noticed the implementation of various standards adhere to appropriately. Details are explained below:

g) Other Audit Reports and Automated Findings

Automated Findings: https://github.com/code-423n4/2024-01-renft/blob/main/bot-report.md

Previous Audits There isn't any Previous Audit

4naly3er report https://github.com/code-423n4/2024-01-renft/blob/main/4naly3er-report.md

h) Packages and Dependencies Analysis 📦

i) New insights and learning of project from this audit:

1. Integration with Seaport: The project's integration with Seaport for order processing demonstrates the complexity and potential challenges of interfacing with external protocols. This requires careful consideration of compatibility and security implications.

2. Emergency Measures: The project's code could benefit from implementing emergency measures like circuit breakers or pause functions, particularly in critical administrative functions, to mitigate risks in case of detected vulnerabilities.

Note: I didn't tracked the time, the time I mentioned is just an estimate

Time spent:

3 hours