Revert Lend - invitedtea's results

A lending protocol specifically designed for liquidity providers on Uniswap v3.

General Information

Platform: Code4rena

Start Date: 04/03/2024

Pot Size: $88,500 USDC

Total HM: 31

Participants: 105

Period: 11 days

Judge: ronnyx2017

Total Solo HM: 7

Id: 342

League: ETH

Revert

Findings Distribution

Researcher Performance

Rank: 27/105

Findings: 1

Award: $527.39

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: 14si2o_Flint

Also found by: Bauchibred, K42, Sathish9098, hunter_w3b, invitedtea, popeye, yongskiws

Labels

analysis-advanced
grade-a
sufficient quality report
A-03

Awards

527.3923 USDC - $527.39

External Links

Revert Lend - Audit Analysis

Project Description

Revert Lend Protocol

Revert Lend is a decentralized lending platform designed specifically for Uniswap v3 Automated Market Maker Liquidity Providers. It enables these providers to take out ERC-20 token loans by using their liquidity provider positions as collateral, all while maintaining control over their funds within Uniswap v3 pools.

Key Components:

  • Lending and Borrowing Mechanism: Facilitates the supply of assets to a lending pool in exchange for rTokens and allows Uniswap v3 Liquidity Providers to borrow by collateralizing their positions.
  • Protocol Architecture: Employs a non-upgradable contract design for security, with Vaults for AMM LPs, unified liquidity, rTokens, and adherence to the ERC-4626 tokenized vault standard.
  • Position Transformers and Liquidation Framework: Implements specialized contracts for flexible position management and a detailed liquidation mechanism to handle undercollateralized positions.

Key Contracts:

AutoCompound

The AutoCompound smart contract allows the operator (a Revert controlled bot) to compound positions for users in the form of Uniswap v3 Non-Fungible Tokens (NFTs). Users must approve the contract to manage their assets when outside the vault, and when assets are inside a Vault, the owner must give approval for the position to be transformed by the contract.

  • Reentrancy Risk in withdrawLeftoverBalances: The withdrawLeftoverBalances function is marked as nonReentrant, but reentrancy through other functions or interactions with ERC20 tokens is still plausible due to external calls like SafeERC20.safeTransfer. Additional checks should be implemented to ensure atomicity where needed.

  • Unchecked Return Values: The _checkApprovals makes external calls to IERC20(token).allowance and SafeERC20.safeApprove, but the return values are not checked. Although OpenZeppelin's SafeERC20 library should revert on failure, it is a best practice to also check the return value of critical function calls.

  • Unbounded Loop in withdrawBalances: Function withdrawBalances uses a loop, which could become an issue if provided with a large array and, result in out of gas errors or denial of service, when the loop runs out of gas during execution.

  • Potential Price Manipulation in the execute Function: The execute function uses an oracle to protect against price manipulation. However, the absence of slippage protection when swapping could be exploited by a sophisticated attacker. Slippage protection, in addition to TWAP, would enhance security.

  • Missing Input Validation: The executeWithVault and execute functions do not validate the validity of the ExecuteParams. Proper validation should be implemented to avoid unexpected behaviors.

  • Implicit Dependency on External State: The contract expects the nonfungiblePositionManager's ownerOf to return the correct owner, even following an NFT transfer. Any changes in the NFT implementation could potentially affect the functioning of the AutoCompound feature.

  • Inefficient Storage Access: Multiple reads and writes to storage variables like positionBalances could be optimized for gas efficiency by using the memory keyword or caching values in local variables during computations and transitions.

  • Use of Type(uint256).max in Approvals: Using type(uint256).max when setting approvals results in higher gas fees. It is recommended to only approve the necessary amount.

  • Code Duplication in _withdrawBalanceInternal and withdrawLeftoverBalances: The _withdrawBalanceInternal and withdrawLeftoverBalances functions have similar checks and effects. This could be refactored to reduce code duplication and improve maintainability.

  • Events Not Indexed: Some events such as AutoCompounded and BalanceWithdrawn do not have indexed parameters, which would make filtering events costlier and challenging.

AutoExit

AutoExit is designed for automatic handling of Uniswap v3 positions allowing either their removal or swapping to the opposite token when a certain tick is reached. The contract exhibits the expected functionality, enabling users to automate limit order and stop loss order strategies, while delegating execution to an authorized operator.

  • Lack of Zero Address Validation for the Constructor The contract constructor takes in several addresses as parameters, but it lacks explicit checks to ensure these addresses are not zero addresses. Supplying a zero address for important roles like _operator or _withdrawer could lead to loss of funds or malfunction of the contract. Implement input validation checks within the constructor to ensure that none of the essential addresses are zero.

  • Reentrancy Concerns in execute Function The execute function interacts with external contracts for token transfers and swaps and does not use reentrancy guards. This could potentially allow reentrancy attacks in scenarios where the called tokens have non-standard behaviors. Employ a reentrancy guard modifier to prevent reentrancy attacks.

  • Possible Unchecked External Call in _routerSwap The _routerSwap function performs an external call to swap tokens and does not check the return value. If the swap fails silently (returns false instead of reverting), it may lead to unexpected behavior. Always check the return values of external calls and handle potential failure cases appropriately.

  • Use of Magic Numbers Several magic numbers or constants, such as Q64, are used without documentation explaining their meaning. This hinders code readability.Define these values as named constants with clear documentation.

  • Implicit Trust on Operator Role The contract relies on an operator address passed in the constructor but offers no on-chain mechanisms to validate or restrict this operator's behavior.Introduce on-chain checks or a governance process to set and validate the operator's address.

  • Struct Packing in PositionConfig The PositionConfig struct can be reordered to optimize storage and reduce gas costs. Reorder struct properties according to their datatype sizes to optimize storage utilization.

  • Excessive State Variable Writes The execute function frequently writes to state variables which can be gas costly.Review the usage of state variable updates within functions and leverage memory variables where appropriate.

  • Event Emit Consistency The Executed event is emitted with a msg.sender argument which corresponds to the operator, not the original caller. This can be confusing. Clarify variable naming for event arguments or modify the event to include the initiator's address.

Automator

The Automator contract inherits functionality from the Swapper and Ownable contracts and is designed to interact with Uniswap V3 pools, manage Uniswap position tokens, and handle WETH wrapping/unwrapping logic. It is part of the Revert Lend protocol's codebase, used for handling automation of tasks in a decentralized finance (DeFi) context.

  • Implement a reentrancy guard to protect against attacks in sensitive functions.
  • Add a check for the slippage rate in the _validateSwap function.
  • Gas optimizations should be addressed for economic efficiency of contract interactions.
  • Refactor and standardize event names to accurately represent the contract's logic and intent.
  • Replace magic numbers with named constants for better code clarity and maintainability.

The Automator contract plays a vital role in the Revert Lend protocol ecosystem, facilitating automation for liquidity management on Uniswap V3. While it displays a well-thought-out design and intended functionality, addressing the identified issues will bolster its robustness and trustworthiness within the DeFi community. Implementing the recommendations provided will significantly enhance the maintainability, security, and efficiency of the Automator contract.

AutoRange

AutoRange, a part of the Revert Lend protocol, is meant to allow operators to adjust Uniswap v3 LP positions' ranges with the intention of optimizing the positions' performance. The contract includes functionality to withdraw liquidity, execute swaps, add liquidity to adjusted range positions, and transfer position tokens, with rebalancing strategies defined by parameters set for each position.

  • Unbounded Loops and Gas Usage

  • Location: There appear to be no unbounded loops detected in the AutoRange contract.

  • Risk: Unbounded loops could render functions unusable if they consume too much gas or hit the block gas limit.

  • Recommendation: Ensure functions with loops are capped or designed to prevent such conditions.

  • Public Functions Without Checks

  • Location: Functions like configToken do not have explicit checks for zero addresses or other potential misuse.

  • Risk: Public functions without thorough precondition checks could be misused or exploited, risking contract integrity.

  • Recommendation: Add precondition checks to public functions to validate function arguments.

  • External Call Risks

  • Location: Functions such as execute make external calls to Uniswap contracts and potential calls to arbitrary swap functions via swapData.

  • Risk: External calls could introduce reentrancy risks or rely on potentially untrusted external contract code.

  • Recommendation: Use checks-effects-interactions to mitigate reentrancy risks and validate external contracts when possible.

  • Price Oracle Manipulation

  • Location: Price validation relies on Uniswap's TWAP, which could be manipulated in the period between fetch and use.

  • Risk: Relying on a potentially manipulable oracle exposes the contract to flash loan attacks or other price manipulation tactics.

  • Recommendation: Use multiple oracles or add circuit breakers to pause contract functions if oracle manipulation is detected.

InterestRateModel

InterestRateModel is intended for calculating interest rates within the Revert Lend protocol. The protocol offers lending services tailored to Uniswap v3 Liquidity Providers. The contract in question determines interest rates based on a kinked model that adjusts rates dynamically with the utilization rate of pooled funds.

  • Lack of Input Validation for setValues Parameters While the contract ensures that the new interest rate parameters are within a permissible range, there is no check to ensure that the _kinkX96 parameter is below Q96, corresponding to a 100% utilization rate. Not capping this could potentially cause erroneous interest rate calculations. Introduce a require statement to validate that _kinkX96 is less than Q96.

  • Potential Arithmetic Precision Loss The contract heavily depends on fixed-point arithmetic computations. However, some operations could result in a loss of precision, particularly the divisions to calculate per-second rates from per-year rates.Consider implementing a more robust fixed-point arithmetic library or adding a precision compensation mechanism where necessary.

  • Cache Q96 in Memory in Functions - The global constant Q96 is used multiple times in the getRatesPerSecondX96 and getUtilizationRateX96 functions. Caching it to memory at the beginning of these functions could provide minor gas savings.

  • Event Emitting Semantic Clarity - The SetValues event uses parameters that mirror the internal variable names rather than more descriptive terms, which might confuse off-chain subscribers. Rename event parameters for better clarity, e.g., baseRatePerYearX96 to newBaseRatePerYearX96.

Approach

The audit focused on a comprehensive review of the project's smart contracts to ensure security and efficiency. Specific areas of focus included potential vulnerabilities in:

  1. Reentrancy: Due to interactions between contracts and external calls.
  2. Share Calculation: Ensuring fair and accurate representation of lenders' shares.
  3. Price Manipulation: Assessing the risk posed by external dependencies on price oracles.
  4. Crafted ERC-20 Tokens: Evaluating the protocol's resilience to tokens designed to exploit the lending mechanics.

Audit Practices for this Project

  • Automated Analysis: Utilized tools like Slither and other static analysis tools to inspect the codebase for common vulnerabilities and code smells.
  • Manual Review: Conducted thorough manual inspection of the logic, focusing on the system's economics, incentive structures, and potential edge cases.
  • Gas Optimization: Analyzed transactions for inefficient gas usage, identifying opportunities for optimization.
Codebase Quality

The codebase demonstrates high quality, with comprehensive documentation and adherence to best practices in Solidity development. The use of Foundry as a development toolchain aids in testing and ensures a robust codebase. Modifications to external libraries, such as the POOL_INIT_CODE_HASH, are clearly documented, ensuring the correct integration with Uniswap v3.

Systemic & Centralization Risks

  • Liquidation Mechanisms: While designed to be resolved by market arbitrage, undercollateralized positions may expose systemic risks if market conditions prevent timely liquidation.
  • Oracle Dependence: Reliance on Chainlink and Uniswap V3 TWAP oracles embeds external risk factors, especially with price manipulation or deviation.
  • Governance Centralization: Initial governance by a multi-sig team may pose centralization risks until transitioned to community governance.

Recommendations

  1. Enhance Liquidation Incentives: To mitigate systemic risks associated with undercollateralized positions, consider introducing more aggressive liquidation premiums or alternative incentives for liquidators.
  2. Oracle Security Improvements: Implement additional safeguards against oracle manipulation, including more frequent updates and utilizing multiple sources for critical price feeds.
  3. Decentralize Governance Sooner: Accelerate the transition towards a more decentralized governance model to reduce centralization risks and enhance community trust.

Gas Efficiency

While the protocol demonstrates awareness of gas costs, particularly in its choice of non-upgradable contracts for security, there are areas where gas efficiency can be improved. Optimizing internal function calls and state updates, as well as leveraging gas-saving patterns for smart contract interactions, could further enhance efficiency.

Final Thoughts

Revert Lend presents a novel approach to leveraging Uniswap v3 positions for decentralized lending, with thoughtful mechanisms and robust architecture. While the audit identified areas for improvement, particularly around liquidation mechanisms, oracle security, and governance, the project demonstrates strong potential for enhancing capital efficiency in the DeFi ecosystem. With adherence to recommendations and continuous testing, Revert Lend is well-positioned to offer a secure and efficient lending platform for liquidity providers.

Time spent:

10 hours

#0 - c4-pre-sort

2024-03-24T08:23:20Z

0xEVom marked the issue as sufficient quality report

#1 - c4-judge

2024-04-01T11:42:28Z

jhsagd76 marked the issue as grade-b

#2 - c4-judge

2024-04-01T11:43:05Z

jhsagd76 marked the issue as grade-a

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter