2024-01-reNFT.md

📝 Analysis - reNFT

Overview

reNFT protocol is focused on lending and collateral-free rentals of ERC721 and ERC1155 tokens in integration with OpenSea’s engine - Seaport and GnosisSafe to prevent theft of rented assets. The protocol allows collateral-free in-house renting, lending, and reward sharing, focusing on scholarship automation.

The key contracts of reNFT protocol for this audit are:

• Create: Contract serving as an entry point for renters, this is their entry point for renting assets (ERC721, ERC1155). There OpenSea orders are fulfilled, rentals are registered as rentals to the wallets of the users, and payments are escrowed to PaymentEscrow.
• Stop: Contract that allows all users to stop rent, distribute the payments either pro rata (PayOrder) or in full (BaseOrder and expired PayOrder), and transfer back the rental assets to the lender.
• PaymentEscrow: Contract that is used for internal ERC20 accounting, payment calculation, and distribution between involved parties.
• Storage: Contract used as storage - keeping, and maintaining information about hooks, rentals, and safes. Additions and removals are routed to functions in this contract.
• Factory: Contract used to deploy rental safes (GnosisSafe) on behalf of renters with hardcoded module and guard policy which prevent users from stealing the NFTs.
• Guard: Contract which serves as SafeWallet guard, all the transactions (except the ones from Stop policy) are routed and checked in the checkTransaction function.

System Overview

Scope

1. Core

• Kernel - core contract used as a registry, manages permissions, upgrades, activates, and deactivates policies and modules.
• Create2Deploy - contract used for a deterministic salt-based deployment with added cross-chain safety.

2. Modules

• PaymentEscrow - internal ERC20 accounting proxy contract, also calculates involved parties’ payments.
• Storage - module proxy contract used as storage - keeping, and maintaining information about hooks, rentals, and safes. Additions and removals are routed to functions in this contract.

3. Policies

• Admin - contract responsible for fee management, whitelisting delegates and extensions, and upgrading modules.
• Create - contract serving as a main entry point for renters after the order is being fulfilled, used to convert Order and Consideration structs to reNFT native and save it into memory allocated mapping.
• Factory - contract used as deployment agent for Gnosis safe wallets and setting predefined wallet module (Stop) and guard (Guard.sol) to prevent malicious transactions from being executed.
• Guard - contract inheriting from Gnosis Safe’s Guard interface, used to verify all the transactions originated from a wallet that involves rented assets.
• Stop - contract used to reclaim rental assets and release the funds from the escrow contract.

4. Packages

• Accumulator - contract used to handle the manipulation of dynamically allocated arrays of data structures directly in memory.
• Reclaimer - contract used to transfer back rental assets to the proper recipient when the period has expired or the lender terminated the rent.
• Signer - contract used to create type hashes and signature verification when creating rentals.

Privileged Roles

The approach that reNFT team has taken is unique, they are configuring permissions to policies based on functions that need to be called.

There are standard trusted roles assigned to real people:

**ADMIN** - role used to grant and revoke other roles.
**SEAPORT** - singleton role granted to Seaport, which will make it possible to call **validateOrder** function in **Create** policy.
**CREATE_SIGNER** - privileged EOA which will live on the ReNFT backend, owned by ReNFT.
**ADMIN_ADMIN** - admin of the protocol, can skim funds, and manage critical params through **Admin** policy
**GUARD_ADMIN** - role given to EOA, can update hook status and path.
**EXECUTOR** - role used only for action execution in the **Kernel**

Approach Taken-in Evaluating The reNFT

Stage	Action	Details	Information
1	Compile and Run Test	Installation	Easy setup and commands provided for testing and deploying
2	Documentation review	Docs	Provides full flow for creating, fulfilling, and stopping rentals, as well as detailed info about whitelists and hooks.
3	Contest details	Audit Details	Thorough details for the contracts and the idea of the protocol were provided. Known issues and possible attack scenarios are helpful.
4	Diagramming	Excalidraw	Drawing diagrams through the entire process of codebase evaluation.
5	Test Suits	Tests	In this section, the scope and content of the tests of the project are analyzed.
6	Manual Code Review	Scope	Reviewed all the contracts in scope
7	Special focus on Areas of Concern	Areas of Concern	Observing the known issues and bot report

Codebase Explanation & Examples Scenarios of Intended Protocol Flow

All possible Actions and Flows in reNFT:

1. Safe wallet deployment

Rental process cannot be finalized without a valid safe gnosis wallet deployed by the renter.

This can be done through Factory policy as it contains all the needed configurations for a safe deployment and transaction validation. Pre deployed Gnosis contracts, such as FallbackHandler, SafeProxyFactory and SafeSingleton, are being used. Additionally, Guard policy is set as guard, that is used to prevent malicious transactions with rental assets involved, such as transfers and approvals, Stop policy is set as wallet module as it is used to reclaim the rented assets without routing the transaction through the Guard’s checkTransaction. TokenCallbackHandler is added as a wallet fallback handler, to be able to handle rental assets as this adds callback functionality.

2. Rental initiation

When a user wants to rent an NFT he interacts only with the backend of the protocol by choosing the token that he want to rent and the rent period. Then fulfillAdvancedOrder in Seaport is called which does various validations and actions, one of them to transfer the token to the recipient specified in the ZoneParameters which is passed to the reNFT’s Create policy through the validateOrder function. This function is the entry point for the reNFT rental creation as it does all the needed actions to ensure information is saved and can be tracked properly in the system:

• Check that the payload is not expired.

• Validates if the intended fulfiller by the lender is the same as the actual fulfiller.

• Recovers the RentPayload signer, who is a reNFT person who is granted the CREATE_SIGNER role.

• Valid safe wallet is required (should be deployed through Factory policy).

• SpentItem and ReceivedItem, aka offer and consideration items from a fulfiller’s perspective are converted to protocol native struct: Item. Then rental assets (ERC721, ERC1155) which are hashed with the help of Accumulator (package used for in memory structure, holding unknown size of elements) added as rentals to the Storage module. (This is valid only for Base and Pay order types. You can read more about different order types here).

• ERC20 items are used to increase the internal balance of the PaymentEscrow module.

  PaymentEscrow module is passed as a conduit recipient and receives all the token payments when rent is initiated.

• If renter has specified hooks, mostly used for Pay order types, their onStart function is being executed (execution can be disabled by an admin).

• Event is emitted and order is saved off-chain.

• Lastly, valid order magic value is returned in order Seaport to consider the execution successful.

Note about why seaport was forked:

reNFT uses validateOrder with modified ZoneParameters struct that contains totalExecutions used to validate the recipient for both ERC721 and ERC20 token which has to be Gnosis Safe wallet of the renter and PaymentEscrow module respectively. This information is missing in the original OpenSea code.

3. Safe transaction execution

First we need to understand how gnosis safes generally works - their architecture is modular, which allows owners to specify various add-ons on top of a wallet.

Some of them are guards and modules:

• Guard - this is a special EIP165 compliant contract that is executed for all normal transactions, initiated from a wallet and does various checks and validations according to the needs of the owners.
• Module - contract that adds additional logic to an existing wallet - in reNFT Stop policy is configured as a wallet module. Note that modules can execute transactions without validating them through the Guard which is crucial for the seamless rental reclaiming.

Transaction is initiated from a wallet and if it’s not originated from a module, Guard’s checkTransaction is executed. This function is used to prevent renters from executing transactions that involve approvals or transfers of rented assets.

On top of that, if renter has specified hooks on rental initiation process and they are activated by an admin, their onExecution function is called giving an additional extendability to the rental process.

Note: delegate call functionality offered by Gnosis is disabled by default, unless explicitly enabled by an admin for specific transaction recipients.

4. Rental termination

Process of stopping a rent can be initiated by everyone, but there are caveats - in case orderType is Base order it will only succeed if rent period has passed, not matter who the caller is. For Pay order if lender initiated the reclaim action, rent period is not taken into account, for non-lender users the rule above applies.

Then for rental items (ERC721 and ERC1155) same dynamic memory structure is created as the one in Rental initiation is created. In case there are hooks specified their onStop function is being executed (execution can be disabled by an admin).

Transaction from the Gnosis safe module (Stop) is executed. This is done to prevent the Guard::checkTransaction from being called (that’s how modules in gnosis safe work) and assets are successfully returned to the renter.

PaymentEscrow settles the payments in either full or pro rata, depending if the rent period is passed and the order type:

• For non-expired Pay orders:
  • Payment is split between lender and renter equally(slightly in favor of the renter because of rounding protection) based on the elapsed time.
• For expired Pay orders:
  • Full payment is settled to the renter due to the nature of this orderType.
• For Base orders:
  • Full payment is settled to the lender.

Rental references are removed from the Storage module which prevents from claiming the tokens twice.

Lastly, event is emitted to notify the off-chain that the rental order has stopped.

Architecture Recommendations

Architecture of the protocol is simple and robust, there is no big complexity for the main tasks that will be performed - renting and executing transactions. There are other parts of the code that are requiring more attention such as upgrades and dynamically allocated data structs kept in memory.

Contracts are split by context which makes the understanding easier.

The idea of hooks is essential as it will extend significantly the use case of the protocol.

Create2Deployer is gas efficient due to the amount of assembly used, would be great if same effect is applied to Factory policy, because it will be used quite often.

Important notes regarding the codebase and its sustainability:

• Guard policy does not prevent important functions from being executed such as burn, it is important to note that all the rent request will be approved by reNFT signer, but handling such function will definitely prevent from malicious actions.
• Most of the hashes are constructed wrong, which will DoS the rentals.

Codebase Quality Analysis

We consider the codebase of the reNFT simple and versatile, hooks greatly fit the main idea of the team but also does not limit the working capabilities of the protocol as not being mandatory and having the go-to checkTranscation function giving the same or even bigger sense of security. Safe wallets deployment is utilised to automatically deploy one on behalf of renter. The rent stop process is not as good as the other parts of the code, as it heavily depends on the hooks’ (if order uses any) onStop not being disabled, otherwise NFTs will be stuck in rental safes.

Tests have high abstraction which is a double-edged sword as they require more time to be completely understood, but due to Seaport integration this can’t be mitigated.

Codebase Quality Categories	Comments	Score (1-10)
Code Maintainability and Reliability	The codebase demonstrates moderate maintainability with well-structured functions and shared modularity across various contracts constructing all the calls in the system. Well-designed system for the main purpose and extendability for handling various NFT use cases by using Hooks. Packages are used to differentiate logic, lowering the code complexity and reducing the time needed to understand the reNFT codebase.	8
Code Comments	All the contracts in scope have comments explaining the purpose and functionality of their functions, making it easier for developers and auditors to understand and maintain the code. Comments describe methods, variables, and the overall structure of the contract. Contracts have good amounts of comments that make understanding an easy process.	9
Documentation	Mostly non-technical documentation is provided on the reNFT, but the team did a great job by providing an overview on the contest page explaining the idea, invariants, attack scenarios, and the main actions that various users will perform in the system. Seaport part was a bit confusing, reNFT team can push themselves further and create documentation about the functions used and what needed to change to be able to handle reNFT as a zone.	8
Code Structure and Formatting	Most of the contracts are constructed from 1 main function which utilises all the others as this approach makes it easier to trace the flow and understand the system, default formatting is used which splits the arguments on a new line. There aren’t big and chunky code lines and in our opinion placing all the arguments on one line will result in better looking code.	6

Test analysis

The audit scope of the contracts to be audited is 80% most of them cover the whole flow of the actions that are available in the system without testing in isolation.

However, development team did a great job by not creating mocks for most of the contracts but fixtures with the real logic of the contracts were used.

This approach deviates a little from the initial idea of unit testing - to test only small modules, but gives context what happens in the both protocols that are integrated in the system - GnosisSafe and Seaport.

Thing to consider about the tests is how the hashes in fixtures are constructed. Same function is used to hash both structs and this gives false sense of security and most of our reported vulnerabilities are from these parts of the code. Currently their representation is something like this: _deriveOrderMetadataHash(arg) == _deriveOrderMetadataHash(arg), which will always return true.

Systemic & Centralization Risks

reNFT utilises contract based authorization which gives the policy contracts (contracts where users interact with the system) certain function selectors from the module contracts that can be called from these contracts. This approach lowers the centralization risk of the protocol.

There are other roles as well, such as EXECUTOR and ADMIN that will be given to EOAs and they poses serious centralisation risk, especially the EXECUTOR.

If it turns out to be given to a malicious user, this can lead to catastrophic consequences as he can change the Kernel, Policies and Modules implementation which will directly affect both lenders and renters.

Team Findings

Severity	Findings
High	3
Medium	2
Low/NC	8

Findings were identified in the Signer and Guard contracts.

They cause significant impact because:

• Signer policy malfunction which leads to wrongly hashed structs.
• Burn function selector is not handled and can result in 100%+ loss for lender and in some cases profit for renter who burned because NFT refunds the floor price.

New insights and learning from this audit

Learned about:

• Seaport
  • Zones
  • Order fulfillment
  • Order creation
  • Conduits
  • TokenTransferrer from Seaport fulfillment
• GnosisSafe
  • Guard
  • Modules
  • Safe transaction execution
• EIP 712 in depth

Conclusion

In general, the reNFT project exhibits an standard and well-developed architecture we believe the team has done a good job regarding the code, but the identified risks need to be addressed, and measures should be implemented to protect the protocol from potential malicious use cases. Additionally, it is recommended to improve the documentation as there is a need of proper understanding of the underlying architecture interacting with the integrated protocols. It is also highly recommended that the team continues to invest in security measures such as mitigation reviews, audits, and bug bounty programs to maintain the security and reliability of the project.