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
Rank: 27/105
Findings: 1
Award: $527.39
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 14si2o_Flint
Also found by: Bauchibred, K42, Sathish9098, hunter_w3b, invitedtea, popeye, yongskiws
527.3923 USDC - $527.39
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.
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 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.
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.
_validateSwap
function.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, 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 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
.
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:
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.
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.
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.
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