reNFT - hunter_w3b's results

Analysis - reNFT Contest

Description overview of The reNFT Contest

The reNFT protocol facilitates collateral-free NFT rentals through the use of modular contracts, Gnosis Safe and integration with the Seaport protocol. It allows NFT owners to list their assets for rent through Seaport, after which renters can access those listings and initiate rentals without needing to provide collateral, as the rented NFTs are transferred to Gnosis Safe controlled by contracts that escrow rental payments and restrict transfers until the rental period ends. Core components include payment escrow, storage and administration modules that manage the protocol's data and functionality, along with policies that handle rental creation, guarding rented assets, and completion of rentals, all built on a permissionless, trustless framework enabled through decentralized applications.

The key contracts of reNFT protocol for this Audit are:

• PaymentEscrow.sol - Handles the core payment escrow functionality which is crucial for the functioning of the protocol. Needs to ensure payments are properly handled.

• Storage.sol - Manages critical storage and data for the protocol, including tracking rentals, safes, hooks etc. Data integrity needs to be verified.

• Create.sol - Implements the core rental creation process, so validation of orders, signature checks etc.

• Factory.sol - Deploys rental safes which are core to rentals, so safes need proper permissions and restrictions.

• Guard.sol - Enforces rental transaction restrictions on safes, so permissions and exploit checks are important.

• Kernel.sol - Acts as the registry managing all other contracts. Key to verify its modular design, permissions and roles.

As the audit of these contracts, I checked for potential security vulnerabilities, such as reentrancy, access control issues, and logic flaws. Additionally, thoroughly test the functions and roles defined in these contracts to make sure they behave as expected.

System Overview

Scope

• PaymentEscrow.sol: The PaymentEscrow contract is a module within the reNFT protocol. It is responsible for escrowing rental payments while rentals are active and managing the settlement process when rentals are stopped. The contract handles various functions, including calculating and applying fees on payments, settling payments through pro-rata or in full schemes, and managing the deposit balances of different tokens. Additionally, it provides external functions for settling payments for single or multiple rental orders, increasing deposit balances, setting fees, skimming excess funds, and facilitating contract upgrades. The contract employs a modular architecture, utilizing the Gnosis Safe and Seaport platforms, and is designed to be upgradeable while ensuring compatibility with ERC-1822 standards.

• Storage.sol: This contract is designed for managing various storage functionalities, including tracking active rentals, deployed rental safes, hooks, and whitelists. It utilizes a modular architecture and ensures compatibility with ERC-1822 standards, allowing for upgradeability through the Gnosis Safe and Seaport platforms, with features such as adding/removing rental orders, updating hook paths and statuses, adding deployed rental safes, toggling whitelist statuses, and facilitating contract upgrades and freezing.

• Accumulator.sol: The Accumulator is an abstract contract provides functionality for managing dynamically allocated data struct arrays directly in memory, specifically implementing the accumulation of an intermediary representation of a dynamic array of RentalAssetUpdate structs and converting this representation into a conventional Solidity array. This is designed to address the need for an array of structs where the total size is not known at instantiation, utilizing low-level assembly operations for efficient memory management.

• Reclaimer.sol: The Reclaimer abstract contract facilitates the retrieval of rented assets from a wallet contract once a rental has been stopped, and it transfers them to the proper recipient, preventing potential exploits through delegate calls by ensuring that calls are made only from whitelisted addresses within the context of the rental safe.

• Signer.sol: The Signer abstract contract contains logic related to signed payloads and signature verification when creating rentals, implementing EIP-712 domain separation for cryptographic verification of order-related data, and ensuring fulfillment and expiration conditions are met.

• Admin.sol: Admin.sol contract acts as an interface for all behavior in the protocol related to admin logic, including fee management, proxy management, and whitelist management, with specific functions allowing the administrator to toggle whitelists, upgrade and freeze storage and payment escrow modules, skim protocol fees, and set the protocol fee numerator.

• Create.sol: This contract is an interface for all behavior related to creating a rental includes functions for configuring dependencies, requesting permissions, retrieving domain separator, deriving EIP-712 compliant hashes for rental order, rent payload, and order metadata, processing different types of rental orders (BASE, PAY, PAYEE) converting offer and consideration items into an array of Item, adding hooks associated with a rental order, and initiating a rental order using a rental payload received by the fulfiller. The contract also is a valid Seaport zone with the external function validateOrder, which is called by Seaport after each order in a batch is processed. This function validates the order, checks the expiration of the signature, verifies the fulfiller, recovers the signer from the payload, and initiates the rental using the rental manager.

• Factory.sol: An interface for deploying rental safes with specific policies and guards, utilizing the Gnosis Safe framework. It includes functionalities for configuring dependencies, requesting permissions, initializing rental safes with predefined policies, and deploying rental safes with specified owners and signature thresholds.

• Guard.sol: The contract is an interface for managing and enforcing transaction restrictions, such as preventing token transfers, module modifications, or guard changes, for transactions originating from a rental wallet within the Gnosis Safe framework. The Guard interacts with the storage contract to retrieve and validate information about rental status, whitelisted extensions, and hook contracts for specific target contracts

• Stop.sol: Stop contrac is an interface for managing the termination of rental orders within the Gnosis Safe framework. It includes functions to stop individual rentals or batches of rentals interacting with other modules, such as the storage contract, payment escrow, and hooks for additional processing. The policy enforces conditions for stopping rental orders based on order types, expiration times, and the roles of involved parties.

• Create2Deployer.sol: This contract utilizing CREATE2 opcode to deploy contracts with a given salt and init code while preventing frontrunning by asserting that the first 20 bytes of the salt match the sender's address, ensuring cross-chain safety and uniqueness in deployment addresses.

• Kernel.sol: The contract defines a modular and permissioned kernel system, including abstract contracts for kernel adapters, modules, and policies. It manages the installation, upgrade, activation, and deactivation of modules and policies, maintains dependencies, handles permissions, and allows the migration of the kernel to a new contract. The kernel is a registry that manages roles, module keycodes, and their interactions.

Privileged Roles

Some privileged roles exercise powers over the reNFT contracts:

• Admin

    modifier onlyAdmin() {
        if (msg.sender != admin) revert Errors.Kernel_OnlyAdmin(msg.sender);
        _;
    }

• Executor

    modifier onlyExecutor() {
        if (msg.sender != executor) revert Errors.Kernel_OnlyExecutor(msg.sender);
        _;
    }

• Role

    modifier onlyRole(bytes32 role_) {
        Role role = toRole(role_);
        if (!kernel.hasRole(msg.sender, role)) {
            revert Errors.Policy_OnlyRole(role);
        }
        _;
    }

• Kernel

    modifier onlyKernel() {
        if (msg.sender != address(kernel))
            revert Errors.KernelAdapter_OnlyKernel(msg.sender);
        _;
    }

• permission

    modifier permissioned() {
        if (!kernel.modulePermissions(KEYCODE(), Policy(msg.sender), msg.sig)) {
            revert Errors.Module_PolicyNotAuthorized(msg.sender);
        }
        _;
    }

Approach Taken-in Evaluating The reNFT Protocol

Accordingly, I analyzed and audited the subject in the following steps;

1. Core Protocol Contract Overview:

   I focused on thoroughly understanding the codebase and providing recommendations to improve its functionality. The main goal was to take a close look at the important contracts and how they work together in the reNFT Protocol.

   I start with the following contracts, which play crucial roles in the reNFT protocol:

   Main Contracts I Looked At

   I start with the following contracts, which play crucial roles in the reNFT protocol:

           PaymentEscrow.sol
           Storage.sol
           Create.sol
           Accumulator.sol
           Reclaimer.sol
           Kernel.sol
   

   I started my analysis by examining the PaymentEscrow.sol contract, as it serves a pivotal role in the reNFT protocol. This module is dedicated to escrowing rental payments while rentals are active, and it plays a crucial role in the settlement process when rentals are stopped. The contract manages the deposit balances of various tokens, calculates and applies fees on payments, and handles the external functions for settling payments for single or multiple rental orders.

   Then I turned my attention to the Storage.sol contract, a crucial component within the reNFT protocol. This contract is specifically designed for managing various storage functionalities, including tracking active rentals, deployed rental safes, hooks, and whitelists. The modular architecture of Storage.sol ensures compatibility with ERC-1822 standards, allowing for upgradeability through the Gnosis Safe and Seaport platforms.

   Then went to that Create.sol contract to check how implementing a Seaport zone, validating and processing rental orders, configuring dependencies on storage and payment escrow modules, and handling various order types such as Base, Pay, and Payee orders, including processing offer items, consideration items, and hooks associated with the rental orders.

   Then check vulnerabilities in Accumulator.sol contract by reviewing for potential issues such as unchecked external calls, array bounds checking, and reentrancy vulnerabilities. Additionally, ensure that the contract follows best practices for visibility settings, gas usage, and proper handling of edge cases to mitigate potential security risks specific to its implementation.

2. Documentation Review:

   There is no documentation available when I stack ask questions from sponser's [Alec1017]

3. Compiling code and running provided tests:

4. Manuel Code Review In this phase, I initially conducted a line-by-line analysis, following that, I engaged in a comparison mode.

   • Line by Line Analysis: Pay close attention to the contract's intended functionality and compare it with its actual behavior on a line-by-line basis.

   • Comparison Mode: Compare the implementation of each function with established standards or existing implementations, focusing on the function names to identify any deviations.

Architecture Description and Diagram

Architecture of the key contracts that are part of the reNFT protocol:

Architecture Description:

The "reNFT" protocol facilitates generalized collateral-free NFT rentals built on Gnosis Safe and Seaport, utilizing the Default Framework as its main architecture. The contracts in scope can be categorized into four main groups:

Modules:

1. Payment Escrow: A module dedicated to escrowing rental payments while rentals are active. It determines payouts to all parties when rentals are stopped, with a fee reserved for withdrawal by a protocol admin.

2. Storage: This module maintains all the storage for the protocol, including storage for active rentals, deployed rental safes, hooks, and whitelists.

Policies:

3. Admin: Acts as an interface for admin logic, managing fee, proxy, and whitelist management.

4. Create: Provides an interface for creating a rental. It serves as an entry point for creating rentals through the protocol, accessible only by Seaport contracts.

5. Factory: Manages behavior related to deploying rental safes using Gnosis Safe factory contracts.

6. Guard: Acts as an interface for behavior related to guarding transactions originating from a rental wallet. It prevents transfers of ERC721 and ERC1155 tokens, token approvals, and enabling of non-whitelisted Gnosis Safe modules.

7. Stop: Serves as an interface for behavior related to stopping a rental. This policy, also a module on rental safe wallets, has the authority to pull funds out of a rental wallet during a stop.

Packages:

8. Accumulator: Implements functionality for managing dynamically allocated data struct arrays directly in memory, addressing the need for an array of structs with an unknown total size at instantiation.

9. Reclaimer: Retrieves rented assets from a wallet contract once a rental is stopped and transfers them to the proper recipient through a delegate call.

10. Signer: Contains logic related to signed payloads and signature verification when creating rentals.

General:

11. Create2 Deployer: A deployment contract using the init code and a salt for deployment, adding cross-chain safety.

12. Kernel: A registry contract managing policy and module contracts, permissions, and roles for role-granting and executing kernel functionality.

Architecture Feedback

Formal Verification: Consider a professional formal verification of the contract to identify and mitigate any security risks.

Documentation: Providing thorough natural language documentation of concepts, approaches, functions via external resources like a Knowledge Base is important for developers and users.

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:

Systemic & Centralization Risks

The analysis provided highlights several significant systemic and centralization risks present in the reNFT protocol. These risks encompass concentration risk in PaymentEscrow, Accumulator risk and more, third-party dependency risk, and centralization risks arising.

Here's an analysis of potential systemic and centralization risks in the contracts:

Systemic Risks:

Accumulator.sol

1. Out-of-Gas Risk:

   • The _insert function uses inline assembly to manipulate memory, and if the number of elements becomes large, it could lead to high gas costs. Gas estimation and testing with various input sizes are crucial to ensure that the function remains gas-efficient.

2. Memory Layout Risks:

   • The use of inline assembly introduces low-level memory manipulation. Any error in calculating offsets or updating memory could result in unexpected behavior, memory corruption, or vulnerabilities. Careful auditing and testing are necessary to ensure correct memory layout and handling.

Reclaimer.sol

1. Delegate Call Risks:

   • The reclaimRentalOrder function relies on delegate calls and the correct context (the rental wallet) to prevent unauthorized access. Any vulnerabilities in the delegate call mechanism or incorrect context validation could lead to unauthorized reclaiming of assets. Careful testing and auditing are essential to ensure the security of this approach.

Signer.sol

1. Signature Verification Vulnerabilities:

   • The contract heavily relies on signature verification for various components, such as rental orders and order fulfillments. If there are vulnerabilities in the signature verification logic or if the underlying ECDSA library has issues, it could lead to unauthorized actions, including unauthorized rentals or fulfillments.

2. Expiration Validation Reliance:

   • The contract relies on the block.timestamp for signature expiration validation. Depending solely on block timestamps can introduce risks due to potential manipulation or miner discrepancies. It's essential to consider potential time-related vulnerabilities and ensure that the expiration logic is robust.

3. Domain Separator Dependency:

   • The contract uses an EIP-712 domain separator for signature verification. The correctness of the domain separator is crucial for security. If there are issues with how the domain separator is derived or if it is not properly updated in the future, it could lead to incorrect signature verification.

Admin.sol

1. Freezing modules is irreversible: A bug could permanently disable protocol functions.

2. Single admin role controls all permissions: compromise breaks security model.

Create2Deployer.sol

1. Gas Limit and Deployment Cost:

   • The deploy function uses assembly to perform contract deployment, and the gas cost of deployment is not predictable. If the deployment gas cost exceeds the block gas limit, deployment might fail, and funds could be lost.

2. Limited Error Information:

   • Reverting with custom error messages from the Errors library is good for readability, but it might limit the information provided to users or developers in case of failures. Additional context or debugging information could enhance the user experience.

3. Front-Running Risk:

   • While the contract attempts to prevent front-running by checking if the first 20 bytes of the salt match the sender's address, front-running attacks are intricate, and additional measures may be needed to mitigate such risks. It's essential to consider the specifics of the use case and potential attack vectors.

4. CREATE2 Byte Collision:

   • The contract uses the byte 0xff as a constant (create2_ff) to prevent collisions with the CREATE2 opcode. While this is generally safe, it assumes that no other contract in the Ethereum ecosystem uses the same byte for other purposes.

Centralization Risks:

• PaymentEscrow.sol

  1. Single Administrator: The contract appears to have an upgrade mechanism (upgrade function) that allows changing the contract's implementation. However, the upgrade capability is controlled by a single administrator, and if compromised, it poses a centralization risk. Consider implementing a multisig or decentralized governance mechanism for upgrades to mitigate this risk.

  2. Fee Management Centralization: The setFee function allows a single administrator to set the fee for the contract. Centralized control over fee management might lead to potential misuse or lack of transparency. Consider implementing a decentralized governance model for fee adjustments.

  3. Skimming Centralization: The skim function allows the administrator to skim funds from the contract. The ability to skim funds is centralized and should be carefully managed to prevent abuse. Consider implementing a decentralized or multisig control mechanism for fund skimming.

  4. Freeze Function Centralization: The freeze function allows the contract to be frozen, preventing further upgrades. This capability is also centralized. It might be beneficial to implement a decentralized mechanism for freezing to avoid single points of control.

• Storage.sol

  1. Single Administrator: The contract has an upgrade mechanism (upgrade function) that allows changing the contract's implementation. However, the upgrade capability is controlled by a single administrator, and if compromised, it poses a centralization risk. Consider implementing a multisig or decentralized governance mechanism for upgrades to mitigate this risk.

  2. Storage Base Centralization: Storage management is centralized within the StorageBase contract. Any changes or upgrades to the storage structure would require modifying this base contract, which is a centralized point of control. Consider designing storage in a modular and upgradable manner to allow for flexibility and decentralization.

  3. Rental Safe Deployment Centralization: The addition of rental safe addresses (addRentalSafe function) is controlled by a single administrator. Centralized control over rental safe deployment might lead to potential misuse or lack of transparency. Consider implementing a decentralized governance model for rental safe registration.

  4. Hook Path and Status Centralization: The management of hook paths (updateHookPath function) and their status (updateHookStatus function) is centralized. The choice of hooks and their functionality is controlled by a single administrator. Consider implementing decentralized or multisig control mechanisms for hook-related operations.

  5. Whitelist Management Centralization: The toggling of whitelist entries for delegates and extensions (toggleWhitelistDelegate and toggleWhitelistExtension functions) is centralized. Centralized control over whitelist management might pose risks. Consider implementing decentralized governance for whitelist operations.

  6. Freeze Function Centralization: The freeze function allows the contract to be frozen, preventing further upgrades. This capability is also centralized. It might be beneficial to implement a decentralized mechanism for freezing to avoid single points of control.

• Accumulator.sol

  1. Single Owner Control:The contract does not incorporate any access control or ownership mechanisms. This means that any contract inheriting from Accumulator can freely call and modify the internal functions. Consider implementing an access control mechanism to restrict who can call these functions.

  2. No Governance Mechanism: There is no governance mechanism or multisig control over critical functions like _insert or _convertToStatic. A single owner controls these operations, which poses centralization risks. Implementing a decentralized governance model or requiring multisig approval for critical operations could mitigate this risk.

• Reclaimer.sol

  1. Single Owner Control:The contract lacks a decentralized governance model or multisig control over critical functions like reclaimRentalOrder. A single owner controls these operations, which poses centralization risks. Implementing a decentralized governance model or requiring multisig approval for critical operations could mitigate this risk.

• Signer.sol

  1. Single Owner Control: The contract lacks a decentralized governance model or multisig control over critical functions. A single owner controls these operations, which poses centralization risks. Implementing a decentralized governance model or requiring multisig approval for critical operations could mitigate this risk.

  2. Potential Code Duplication:There is some redundancy in type hash derivation where similar strings are encoded multiple times. Consider refactoring the code to reduce duplication and improve maintainability.

• Admin.sol

  1. Modules are upgraded by admin: introducing single point of control over code.

  2. Governance is not decentralized: risks poor policy decisions without community input.

  3. Fee parameters set centrally by admin: lack of validation/oversight.

• Create.sol

  1. Single Point of Failure: The contract has a dependency on the Kernel contract for configuration and permissions. If the Kernel contract becomes inaccessible or has issues, it could affect the entire system.

  2. Role-Based Access Control: The contract uses role-based access control (onlyKernel and onlyRole("SEAPORT")). Ensure that the assignment of roles is secure, and there are no vulnerabilities in the role management logic.

  3. Signature Validation: The contract relies on a signature for validation (_recoverSignerFromPayload). If the signing mechanism is compromised, it could lead to unauthorized actions.

• Guard.sol

  1. Single Point of Failure:The contract relies on a single kernel contract (Kernel). If this kernel contract becomes compromised, it may have significant consequences for the entire system.

  2. Admin-Only Functions: The functions updateHookPath and updateHookStatus are restricted to the "GUARD_ADMIN" role. If the role is centralized or easily manipulated, it could pose a centralization risk.

  3. Dynamic Hook Configuration: The contract allows dynamic updates to the hook paths and statuses. Depending on the context, this flexibility can be a centralization risk if not carefully controlled and monitored.

  4. Whitelisting of Delegates and Extensions:The contract relies on whitelisting for certain operations, such as delegates and extensions. Centralized control over these whitelists could be a centralization risk.

• Create2Deployer.sol

  1. Admin Privileges: There are no mechanisms to change or upgrade the logic of the contract. If any updates or changes are needed, they would require deploying a new contract. This lack of upgradability might be intentional but limits flexibility.

  2. Single Deployment Authority:The contract's deploy function relies on the sender's authority to deploy contracts. If the sender's private key is compromised, it could lead to unauthorized deployments.

  3. Limited Salt Generation: The generateSaltWithSender function provides a simple method for generating salts based on the sender's address and additional data. While this ties the salt to the sender, it might limit flexibility in certain deployment scenarios.

  4. Limited Deployment Constraints: The contract assumes that deployments can only be performed by the address specified in the salt. This might be suitable for specific use cases, but it restricts the flexibility of contract deployments and might not be suitable for decentralized or permissionless scenarios.

  5. Deployment Address Ownership:The deployed mapping is used to prevent redeploys at the same address. Ownership and control over this mapping are centralized within the contract. Consideration should be given to potential risks if the deployment address ownership needs to be transferred or managed by multiple entities.

• Kernel.sol

  1. Administrator Role: The kernel contract has an admin role, and certain functions can only be called by the admin. The admin role introduces centralization, and if the admin's private key is compromised or misused, it could lead to unauthorized changes and disruptions in the system.

  2. Executor Role: The executor role is responsible for handling kernel executions. The onlyExecutor modifier restricts certain functions to be called only by the executor. The centralization of execution rights should be carefully managed to prevent potential misuse.

  3. Policy Activation and Deactivation: The activation and deactivation of policies rely on the onlyAdmin modifier. Centralized control over policy activation introduces risks, and a compromised admin could manipulate the system by activating or deactivating policies without proper authorization.

  4. Role Management: The functions grantRole and revokeRole are restricted by the onlyAdmin modifier, allowing only the admin to manage roles. This centralization of role management should be carefully considered, especially in scenarios where multiple entities need to participate in the administration.

  5. Permissions Management: The executeAction function allows the executor to execute actions like installing modules, upgrading modules, and activating policies. The centralization of these actions may introduce risks if not properly managed and monitored.

  6. Dependency Pruning: The _pruneFromDependents function is responsible for removing a policy from the array of dependents when deactivated. The correct operation of this function relies on proper implementation by policies, and failure to do so could lead to inconsistent state and unexpected behavior.

Properly managing these risks and implementing best practices in security and decentralization will contribute to the sustainability and long-term success of the reNFT protocol.

Conclusion

Here are the key points I would include in a conclusion:

• Overall the code quality and architecture of the reNFT protocol contracts are good. The code is well structured, tested and follows solidity best practices.

• Some opportunities for improvement include adding documentation, more error handling, formal verification.

• Following best practices in security, decentralization and ongoing maintenance/audits will help ensure the long-term sustainability and success of the protocol.

• The analysis has identified several systemic and centralization risks present in the reNFT protocol. These risks span across different contracts, each with its own set of vulnerabilities that could potentially impact the security and decentralization of the overall system.

• 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.

Time spent:

30 hours