Platform: Code4rena
Start Date: 08/01/2024
Pot Size: $83,600 USDC
Total HM: 23
Participants: 116
Period: 10 days
Judge: 0xean
Total Solo HM: 1
Id: 317
League: ETH
Rank: 38/116
Findings: 1
Award: $269.86
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: fouzantanveer
Also found by: 0xA5DF, 0xepley, SAQ, SBSecurity, albahaca, catellatech, clara, hals, hunter_w3b, kaveyjoe, ladboy233, pavankv, peanuts, tala7985, yongskiws
269.8643 USDC - $269.86
The reNFT protocol is a collateral-free, permissionless, and highly customizable EVM (Ethereum Virtual Machine) NFT rental platform. Built on top of Gnosis Safe and Seaport, it facilitates generalized NFT rentals. Users can lend their gaming NFTs through Seaport orders, allowing others to rent and use them. The protocol ensures the secure transfer and management of NFTs during rentals, enhancing the overall user experience.
The protocol's architecture is based on the Default Framework, with contracts categorized into Modules, Policies, Packages, and General components. Modules handle internal states, Policies manage external calls, Packages offer helper functionalities, and General contracts provide agnostic support.
It supports ERC20, ERC721, and ERC1155 tokens, with deployments planned on Ethereum Mainnet, Polygon, and Avalanche, expanding later to all chains supported by Gnosis Safe. The protocol defines trusted roles, specifying addresses with specific roles like SEAPORT, CREATE_SIGNER, ADMIN_ADMIN, and GUARD_ADMIN.
Publicly known issues include considerations about the security of rental wallets, proper rental creation and stopping, and maintaining specific invariants during the rental lifecycle. The protocol also addresses potential attack vectors related to rental wallet security and emphasizes the importance of proper rental creation and stopping procedures.
The reNFT protocol demonstrates innovation in the NFT space, providing a decentralized and flexible solution for NFT rentals. The integration with Gnosis Safe and Seaport enhances security and usability. However, careful attention is required to address potential vulnerabilities related to rental wallet security and lifecycle management.
The protocol's reliance on external contracts, such as Gnosis Safe and Seaport, introduces additional layers of complexity that should be thoroughly audited and monitored. Additionally, the use of hook contracts introduces a potential attack surface that needs robust mitigation strategies.
Overall, the protocol presents a promising solution, but a comprehensive security audit, particularly focused on rental wallet interactions and external contract dependencies, is crucial for ensuring a secure and reliable ecosystem.
The evaluation of the codebase involved a systematic review of key contracts, including Create2Deployer, Kernel, PaymentEscrow, Storage, Accumulator, Reclaimer, Signer, Admin, Create, Factory, Guard, and Stop. The analysis focused on understanding each contract's functionality, identifying security concerns, and providing recommendations for improvement.
The evaluation considered various aspects, including role-based access control, upgradeability, external interactions, error handling, and gas optimization. Security issues, potential attack vectors, and best practices were scrutinized to provide a comprehensive assessment.
The recommendations provided aim to enhance the security, efficiency, and robustness of the protocol by addressing identified issues and suggesting proactive measures.
Note: Due to the character limit, the Approach and Analysis for the codebase evaluation have been summarized. Further details can be provided upon request.
Role Management in Admin Contract:
Gas Efficiency in Contract Loops:
Access Control Validation Across Contracts:
External Contract Audits:
Upgrade Path Verification in Admin Contract:
Enhanced Role Management:
Optimization of Gas Efficiency:
Thorough Access Control Validation:
Continuous External Contract Audits:
Upgrade Path Verification in Admin Contract:
These recommendations aim to strengthen the protocol's architecture by addressing identified issues. Implementing these measures will contribute to a more secure, efficient, and resilient NFT rental protocol.
| Contract | Maintainability | Readability | Security | Gas Efficiency | Best Practices Adherence |
|---|---|---|---|---|---|
| Create2Deployer.sol | High | High | High | High | High |
| Kernel.sol | Medium | Medium | Medium | Medium | Medium |
| PaymentEscrow.sol | Medium | Medium | Medium | Medium | Medium |
| Storage.sol | Medium | Medium | Medium | Medium | Medium |
| Accumulator.sol | Medium | Medium | Medium | Medium | Medium |
| Reclaimer.sol | Medium | Medium | Medium | Medium | Medium |
| Signer.sol | Medium | Medium | Medium | Medium | Medium |
| Admin.sol | Medium | Medium | Medium | Medium | Medium |
| Create.sol | Medium | Medium | Medium | Medium | Medium |
| Factory.sol | Medium | Medium | Medium | Medium | Medium |
| Guard.sol | Medium | Medium | Medium | Medium | Medium |
| Stop.sol | Medium | Medium | Medium | Medium | Medium |
This table provides a brief overview of codebase quality metrics for each contract in terms of maintainability, readability, security, gas efficiency, and adherence to best practices. Each category is rated as High, Medium, or Low based on an analysis of the contract's characteristics. The ratings are subjective and may vary based on individual interpretations and project requirements.
The Create2Deployer contract serves as a deployment contract using the CREATE2 opcode, allowing for deterministic contract addresses. It deploys contracts with the help of a salt and initCode, ensuring a calculated and secure target deployment address. To mitigate front-running, the contract incorporates the sender's address in the salt. The use of inline assembly is carefully implemented for the specific context of calculating deployment addresses.
The Kernel contract manages various aspects of the protocol, including modules, policies, permissions, roles, and the ability to migrate to a new kernel contract. Modules provide functionalities, policies define rules, and roles restrict access. The contract employs several mappings to track relationships and actions. However, centralization risks arise due to the Kernel's high control over the system, and proper access control and upgrade mechanisms are crucial.
The PaymentEscrow contract is responsible for escrowing rental payments within the broader DeFi platform. It supports upgradability, permissioned functions, and pro-rata settlement logic. Security concerns include the risk of malicious changes during upgrades, secure access control for permissioned functions, careful validation of fee handling for fairness, and transparent pro-rata settlement logic.
The Storage contract manages various components within the protocol, including rentals, safes, hooks, and whitelists. It supports upgradability and employs custom modifiers for access control. Security considerations involve the risk of malicious upgrades, proper access control validation, and gas optimization concerns with loops iterating over large datasets.
The Accumulator contract is designed to manage dynamic arrays in memory using inline assembly. It provides functionality for handling dynamically allocated data struct arrays. Security concerns include potential issues with inline assembly, the need for bounds checking in the _insert function for memory manipulation, and the lack of validation on inputs and unchecked return values.
The Reclaimer contract handles the reclamation of rented assets from a wallet contract via delegate call, transferring ERC721 and ERC1155 tokens to the lender based on a rental order. Security considerations involve reliance on external calls to token contracts, security dependence on admin policy and whitelisting integrity, and the absence of events for transparency in asset transfers.
The Signer contract manages the creation and verification of signed payloads for rental orders using EIP-712. It relies on server-side signatures, introducing centralization risks. Security concerns include the lack of apparent nonce management for preventing replay attacks and the need for careful setting of expiration times to avoid vulnerabilities with old or quickly invalidated signatures.
The Admin contract plays a crucial role in protocol governance and access control. It handles initialization, module configuration, permission requests, whitelist management, module upgrades, freezing modules, fee management, and more. Security issues involve critical reliance on secure role management, centralization risks with the "ADMIN_ADMIN" role, upgradeability concerns, permission system audit, and external contract dependencies.
The Create contract serves as an interface for creating rentals through the protocol. It inherits from various contracts, including Policy, Signer, Zone, and Accumulator, and depends on external contracts like Storage and PaymentEscrow. The contract includes functions for hash derivation, domain separator retrieval, processing rental orders, and entry points like validateOrder. Security considerations involve potential reentrancy issues, robust signature verification, secure access control, proper handling of external calls, and efficient error handling.
The Factory contract is responsible for configuring dependencies, requesting permissions, initializing and deploying rental safes. It depends on various modules and contracts, including Storage, Stop, Guard policies, TokenCallbackHandler, SafeProxyFactory, and SafeL2. Security considerations involve access control using onlyKernel modifiers, warnings about delegate calls, validation of thresholds during safe deployment, ensuring unique safe addresses, potential reentrancy risks, custom errors for clarity, and emitting events for transparency.
The Guard contract acts as an interface for transaction guarding, hook processing, and validation. It inherits from Policy and BaseGuard, depends on Storage, and includes internal and external functions for these purposes. Security considerations include the use of modifiers for access control, inline assembly for calldata reading, reliance on external calls to Storage and hook contracts, and proper error handling. It assumes correct implementation and trustworthiness of external contracts.
The Stop contract handles the stopping of rentals, including validation, hook processing, item reclamation, payment settlement, and storage updates. It is initialized with dependencies on the Kernel contract and requests permissions for specific functions. Security concerns involve access control using modifiers like onlyKernel, potential reentrancy risks, comprehensive error handling, robust validation for stopping rentals, and reliance on external contracts and libraries.
| Contract Name | Security Issues | Recommendations |
|---|---|---|
| Create2Deployer.sol | - Front-running mitigation with sender's address in salt. <br> - No apparent reentrancy concerns.<br> - Inline assembly used correctly. | - Thorough testing and audit before deployment. <br> - Confirm security measures alignment with use case. <br> - Consider potential gas costs and adjust initCode accordingly. |
| Kernel.sol | - Centralization risks due to Kernel's high control.<br>- Access control concerns with modifiers.<br>- Upgrade mechanism validation needed.<br>- Permission management correctness crucial. | - Carefully review and audit the entire codebase, including imports.<br>- Ensure proper access control and upgrade mechanisms.<br>- Thoroughly test the system for role management and permissions.<br>- Consider potential denial-of-service vectors with large datasets. |
| PaymentEscrow.sol | - Upgradability risk during upgrades.<br>- Secure access control needed for permissioned functions.<br>- Fee handling validation for fairness and transparency.<br>- Pro-rata settlement logic review. | - Ensure secure access control for permissioned functions.<br>- Thoroughly review fee calculation logic for fairness and transparency.<br>- Carefully validate and test pro-rata settlement logic.<br>- Consider potential reentrancy concerns. |
| Storage.sol | - Upgradability risk during upgrades.<br>- Access control relies on custom modifiers, potential for unauthorized access.<br>- Gas optimization concerns with loops iterating over large datasets. | - Thoroughly review access control logic and upgrade mechanisms.<br>- Evaluate gas costs for functions with loops over large datasets.<br>- Ensure correct initialization and validation of critical logic. |
| Accumulator.sol | - Inline assembly usage prone to errors.<br>- Memory manipulation in _insert function needs bounds checking.<br>- Lack of validation on inputs and unchecked return values. | - Carefully review inline assembly for correct memory operations.<br>- Implement bounds checking to prevent memory overwrites.<br>- Validate inputs and check return values for safety. |
| Reclaimer.sol | - Relies on external calls to token contracts, assuming correct implementation.<br>- Security heavily depends on admin policy and whitelisting integrity.<br>- Lack of events for transparency in asset transfers. | - Thoroughly review whitelisting mechanisms and admin policy.<br>- Consider adding events for critical actions like asset transfers.<br>- Validate assumptions about rental order data and lender address. |
| Signer.sol | - Relies on server-side signatures, introducing centralization risks.<br>- No apparent nonce management for preventing replay attacks.<br>- Expiration time setting crucial for preventing invalid signatures. | - Review server-side signature generation for security.<br>- Consider implementing nonce management to prevent replay attacks.<br>- Carefully set expiration times for signatures.<br>- Ensure immutability of constants aligns with future needs. |
| Admin.sol | - Critical role-based access control reliance; improper assignments may lead to vulnerabilities.<br>- Centralization risks with "ADMIN_ADMIN" role.<br>- Upgradeability and freezing assumptions.<br>- Permission system and external contract dependencies vulnerabilities. | - Rigorous auditing and monitoring of role assignments.<br>- Consider multisig or timelock mechanisms for critical role operations.<br>- Thoroughly verify ERC-1822 compliance during upgrades; additional checks may be considered.<br>- Verify and audit the kernel's permission system for robustness.<br>- Ensure secure behavior of external contracts; consider thorough audits. |
| Create.sol | - Reentrancy risks in interactions with external contracts.<br>- Robustness of EIP-712 signature verification.<br>- Role-based access control management and validation critical.<br>- Graceful handling of failed external calls for better security.<br>- Comprehensive error handling for all scenarios. | - Implement and thoroughly test reentrancy guards.<br>- Validate and audit the signature verification process.<br>- Rigorously manage and validate role-based access control.<br>- Implement robust handling of failed external calls.<br>- Comprehensive error handling for all scenarios.<br>- Optimize gas usage where possible. |
| Factory.sol | - Uses onlyKernel modifiers for access control; kernel security is crucial.<br>- Warns about delegate calls; proper restrictions by the guard policy important.<br>- Validation of threshold during safe deployment.<br>- Cross-chain uniqueness consideration.<br>- Potential reentrancy risks in external calls.<br>- Custom errors usage for efficiency and clarity. | - Ensure secure implementation and management of the kernel.<br>- Confirm delegate call restrictions by the guard policy.<br>- Validate threshold properly during safe deployment.<br>- Confirm cross-chain uniqueness.<br>- Thoroughly review external calls for potential reentrancy risks.<br>- Ensure all possible errors are handled correctly.<br>- Continue emitting events for transparency. |
| Guard.sol | - Role validation and management crucial.<br>- Inline assembly usage prone to errors.<br>- External call security assurance needed.<br>- Consider implementing post-execution checks for additional security.<br>- Trustworthiness and correctness of external contracts assumed. | - Rigorously validate and manage roles.<br>- Carefully review inline assembly for correctness.<br>- Ensure secure behavior of external contracts; consider thorough audits.<br>- Consider implementing post-execution checks for additional security.<br>- Confirm trustworthiness and correctness of external contracts.<br>- Handle potential ABI changes and ensure continued compatibility. |
| Stop.sol | - Uses modifiers like onlyKernel for access control; proper management of roles crucial.<br>- Order of operations appears to follow checks-effects-interactions pattern for reentrancy mitigation.<br>- Comprehensive error handling for all scenarios.<br>- Thorough validation of conditions for stopping rentals.<br>- External contracts' security impacts the Stop contract. | - Rigorous management and validation of roles.<br>- Confirm adherence to checks-effects-interactions pattern for external interactions.<br>- Handle errors for all scenarios comprehensively.<br>- Thoroughly validate conditions for stopping rentals.<br>- Ensure external contracts are secure through audits.<br>- Conduct a comprehensive audit of the entire system, including all dependencies. |
Centralization risks in the reNFT protocol arise from the concentration of power and control within specific roles or contracts. The "ADMIN_ADMIN" role in the Admin contract is a critical point of concern. This role holds significant power over fee management, proxy management, and whitelist management. The compromise of the private key associated with this role could result in severe damage to the protocol.
Additionally, the use of an upgradeable design in contracts like Admin and PaymentEscrow introduces risks during upgrades. While upgradeability can be a powerful feature, it also creates a potential centralization point if not managed carefully. The assumption of compliance with ERC-1822 during upgrades must be thoroughly validated to prevent the introduction of vulnerabilities.
To mitigate centralization risks, a more decentralized approach to critical operations should be considered. Implementing multi-signature or timelock mechanisms for critical roles, especially in the Admin contract, can distribute decision-making authority and enhance the overall security of the protocol.
The mechanisms implemented in the reNFT protocol for rental creation, stopping, and guarding transactions are well-defined but require careful scrutiny.
Rental Wallet Security: The protocol aims to allow users to safely rent out their assets to rental wallets. The mechanism involves creating a Gnosis Safe for the renter, which has restrictions on moving assets freely. However, potential attack surfaces, such as the usage of delegate call or prohibited functions, should be thoroughly reviewed to ensure the security of rental wallets.
Rental Creation and Stopping: The rental creation and stopping mechanisms involve transferring assets, validating rentals, and handling payments. Proper storage of rental identifiers is crucial for tracking rentals, and potential attack vectors, such as preventing storage of rental identifiers, should be addressed to maintain the integrity of the rental system.
Hook Contracts: The protocol uses hook contracts as middleware for executing logic before transaction payloads. While a whitelist is in place to permit only permissioned hook contracts, the protocol is exposed to risks if a malicious or faulty hook contract is used. Ensuring the security and reliability of hook contracts is paramount to prevent unintended behavior.
External Contract Trust: The protocol relies on the correct implementation and trustworthiness of external contracts, including Gnosis Safe and Seaport. The security of the reNFT protocol is closely tied to the security of these external components. Rigorous audits of these external contracts are necessary to ensure their secure interaction with the reNFT protocol.
In summary, while the mechanisms are well-conceptualized, it is essential to conduct thorough testing and validation to address potential security loopholes and ensure the robustness of the protocol.
Systemic risks in the reNFT protocol are closely tied to the interactions with external contracts, especially Gnosis Safe and Seaport. These external components play a pivotal role in the overall functionality and security of the protocol.
Gnosis Safe Interaction: The protocol relies on Gnosis Safe for creating secure wallets and managing assets. Any vulnerabilities or exploits in the Gnosis Safe implementation could have cascading effects on the security of the reNFT protocol. Therefore, comprehensive audits of Gnosis Safe are imperative to identify and address potential risks.
Seaport Integration: Seaport is utilized for signing seaport order data, enabling the lending and renting of NFTs. The correct integration and secure usage of Seaport are critical for the proper functioning of the reNFT protocol. Audits of Seaport should be conducted to ensure its reliability and security.
Cross-Chain Considerations: The protocol plans to launch on multiple blockchains, including Ethereum Mainnet, Polygon, and Avalanche. The expansion to other chains supported by Gnosis Safe introduces cross-chain considerations. Ensuring the uniqueness and security of addresses across different chains is essential to prevent potential cross-chain attacks.
External Contract Dependencies: The reliance on external contracts introduces dependencies that can pose risks if not thoroughly examined. A comprehensive audit of Gnosis Safe, Seaport, and any other external dependencies is essential to identify and mitigate systemic risks.
In conclusion, the systemic risks in the reNFT protocol emphasize the importance of external contract audits and careful consideration of cross-chain interactions to ensure the overall security and reliability of the protocol.
19 hours
#0 - c4-judge
2024-01-27T20:26:46Z
0xean marked the issue as grade-a