Revert Lend - Bauchibred'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: 9/105

Findings: 4

Award: $1,317.98

🌟 Selected for report: 2

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: Bauchibred

Also found by: Giorgio, grearlake, kodyvim

Labels

bug
3 (High Risk)
primary issue
satisfactory
selected for report
sponsor confirmed
sufficient quality report
:robot:_127_group
H-05

Awards

958.2005 USDC - $958.20

External Links

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L357-L374

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L357-L374

    function _getReferencePoolPriceX96(IUniswapV3Pool pool, uint32 twapSeconds) internal view returns (uint256) {
        uint160 sqrtPriceX96;
        // if twap seconds set to 0 just use pool price
        if (twapSeconds == 0) {
            (sqrtPriceX96,,,,,,) = pool.slot0();
        } else {
            uint32[] memory secondsAgos = new uint32[](2);
            secondsAgos[0] = 0; // from (before)
            secondsAgos[1] = twapSeconds; // from (before)
            (int56[] memory tickCumulatives,) = pool.observe(secondsAgos); // pool observe may fail when there is not enough history available (only use pool with enough history!)
            //@audit
            int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds)));
            sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick);
        }

        return FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, Q96);
    }

This function is used to calculate the reference pool price, It uses either the latest slot price or TWAP based on twapSeconds.

Now note that unlike the original uniswap implementation, here the delta of the tick cummulatives is being calculated in a different manner, i.e protocol implements (tickCumulatives[0] - tickCumulatives[1] instead of tickCumulatives[1] - (tickCumulatives[0] which is cause here, secondsAgos[0] = 0; & secondsAgos[1] = twapSeconds;, unlike in Uniswap OracleLibrary where secondsAgos[0] = secondsAgo; & secondsAgos[1] = 0;, so everything checks out and the tick deltas are calculated accurately, i.e in our case tickCumulativesDelta = tickCumulatives[0] - tickCumulatives[1]

The problem now is that in the case if our tickCumulativesDelta is negative, i.e int24(tickCumulatives[0] - tickCumulatives[1] < 0) , then the tick should be rounded down as it's done in the uniswap library

But this is not being done and as a result, in case if int24(tickCumulatives[0] - tickCumulatives[1]) is negative and (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0, then the returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.

Impact

  1. In the case if int24(tickCumulatives[0] - tickCumulatives[1]) is negative and ((tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0, then returned tick will be bigger than it should be which places protocol wanting prices to be right not be able to achieve this goal, note that where as protocol in some cases relies on multiple sources of price, they still come down and end on weighing the differences between the prices and reverting if a certain limit is passed (MIN_PRICE_DIFFERENCE) between both the Chainlink price and Uniswap twap price.

  2. Now in the case where the implemented pricing mode is only TWAP then protocol would work with a flawed price since the returned price would be different than it really is, potentially leading to say for example some positions that should be liquidatable not being liquidated cause before liquidation there is a check to see if the loan is healthy, now this check queries the value of this asset via getValue() and if returned price is wrong then unhealthy loans could be pronounced as healthy and vice versa.

  3. Also this indirectly curbs the access to functions like borrow(), transform() and decreaseLiquidityAndCollect(), since they all make a call to _requireLoanIsHealthy() which would be unavailable due to it's dependence on _checkLoanIsHealthy()

  4. This bug case causes the Automator's _getTWAPTick() function to also return a wrong tick which then leads to _hasMaxTWAPTickDifference() returning false data since the difference would now be bigger eventually leading to wrongly disabling/enabling of swaps in `AutoCompound.sol whereas it should be otherwise.

Note that for the second / third case the call route to get to _getReferencePoolPriceX96() is : "_checkLoanIsHealthy() -> getValue() ->_getReferenceTokenPriceX96 -> _getTWAPPriceX96-> _getReferencePoolPriceX96() " as can be seen in https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol

Tool used

Add this line: if (tickCumulatives[0] - tickCumulatives[1] < 0 && (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0) timeWeightedTick --;

Assessed type

Uniswap

#0 - c4-pre-sort

2024-03-22T08:20:52Z

0xEVom marked the issue as duplicate of #498

#1 - c4-pre-sort

2024-03-22T08:20:55Z

0xEVom marked the issue as sufficient quality report

#2 - c4-pre-sort

2024-03-25T12:11:18Z

0xEVom marked the issue as not a duplicate

#3 - c4-pre-sort

2024-03-25T12:11:21Z

0xEVom marked the issue as primary issue

#4 - c4-sponsor

2024-03-26T15:04:41Z

kalinbas (sponsor) confirmed

#5 - c4-judge

2024-03-31T00:04:11Z

jhsagd76 marked the issue as satisfactory

#6 - jhsagd76

2024-04-01T08:24:48Z

I think H is appropriate, but it's strange that the other wardens submitted as M. I have reviewed the other submissions and, although they are not as detailed as this report, they all point out the key issues and key impacts, so I will not reduce the reward ratio of the other reports.

#7 - c4-judge

2024-04-01T15:33:41Z

jhsagd76 marked the issue as selected for report

#8 - kalinbas

2024-04-09T17:35:35Z

Findings Information

🌟 Selected for report: t4sk

Also found by: Bauchibred, hunter_w3b, lanrebayode77

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_98_group
duplicate-10

Awards

221.1232 USDC - $221.12

External Links

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L137-L156

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L137-L156

    function _checkPoolPrice(address token0, address token1, uint24 fee, uint256 derivedPoolPriceX96) internal view {
        IUniswapV3Pool pool = _getPool(token0, token1, fee);
        uint256 priceX96 = _getReferencePoolPriceX96(pool, 0);
        _requireMaxDifference(priceX96, derivedPoolPriceX96, maxPoolPriceDifference);
    }

    function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000)
        internal
        pure
    {
        //@audit-issue M logic to verify the max difference is wrong
        uint256 differenceX10000 = priceX96 > verifyPriceX96
            ? (priceX96 - verifyPriceX96) * 10000 / priceX96
            : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;
        // if too big difference - revert
        if (differenceX10000 >= maxDifferenceX10000) {
            revert PriceDifferenceExceeded();
        }
    }

These are two helper functions used in order to ensure that the correct prices are ingested, i.e by checking that difference between both the primary and secondary source is not more that the acceptable "maxDifference".

Now the issue arises in the conditional logic for calculating differenceX10000, where the calculation dynamically chooses the base price for the percentage difference calculation based on which of the two prices is greater. This approach would result in varying methods of comparison, affecting the integrity of the price verification process.

uint256 differenceX10000 = priceX96 > verifyPriceX96
    ? (priceX96 - verifyPriceX96) * 10000 / priceX96
    : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;

The current implementation would fail to use the right primary price as the base for comparison in as much as the secondary price is higher, leading to inconsistencies in how price differences are evaluated. This would incorrectly flag acceptable price variations as excessive or overlook actual problematic price differences.

Consider this case (Using BTC for example):

  • maxDifference is as initially set of 2%
  • Current pricing mode has been set to CHAINLINK_TWAP_VERIFY
  • Chainlink returned price is $70,000
  • Twap returned price is $71,428
  • This should revert since the difference between the primary price (chainlink) and the secondary price is more than 2% of the Chainlink returned price _(1428/70000 * 70000 is > 2% of 70000)_.
  • But _requireMaxDifference() does not actually revert, this is cause it wrongly uses the TWAP price now as the "primary price" since it's higher, now calculating the max difference with the TWAP price as base, leads to the protocol ingesting an invalid price on the grounds of the documented max difference since we now get _(1428/71428 * 71428 which is < 2% of 71428)_ , where 71428 is the wrong base to be used.

Impact

The function _requireMaxDifference within the V3Oracle contract is designed to enforce a maximum allowable price difference between two prices (priceX96 and verifyPriceX96) with a specified tolerance (maxDifferenceX10000). However, the current implementation inaccurately calculates the price difference, as it conditionally uses either price as the base for the percentage difference calculation (which is wrong cause based on historical data for some tokens either of Chainlink/ Uniswap Twap is the most reliable source of pricing and protocol would place the most reliable as the primary source). This method leads to inconsistencies in how price differences are assessed, resulting in incorrect validations and heavy discrepancies if coupled with the leveraging logic, since a price that's originally not meant to be ingested due to having pass the maximum difference would now be ingested.

Note that _maxPoolPriceDifference can only be set higher than the 2%, and if the value is ever to be changed it only exarcebates this issue as this would even lead to allowing a higher discrepancy in prices that would be ingested in protocol.

Modify the difference calculation to always use the correct primary price as the base, that's depending on the current pricing mode for that token.

This would mean always using the primary priceX96 as the base for calculating the percentage difference to verifyPriceX96 (the secondary price), i.e apply these changes to

    function _requireMaxDifference(uint256 priceX96, uint256 verifyPriceX96, uint256 maxDifferenceX10000)
     internal
     pure
 {
     uint256 differenceX10000 = priceX96 > verifyPriceX96
          ? (priceX96 - verifyPriceX96) * 10000 / priceX96
-            : (verifyPriceX96 - priceX96) * 10000 / verifyPriceX96;
+            : (verifyPriceX96 - priceX96) * 10000 / priceX96;
     // if too big difference - revert
     if (differenceX10000 >= maxDifferenceX10000) {
         revert PriceDifferenceExceeded();
     }
 }

Assessed type

Oracle

#0 - c4-pre-sort

2024-03-22T08:40:02Z

0xEVom marked the issue as duplicate of #10

#1 - c4-pre-sort

2024-03-22T08:40:05Z

0xEVom marked the issue as sufficient quality report

#2 - c4-judge

2024-04-01T13:25:35Z

jhsagd76 marked the issue as satisfactory

Awards

55.6122 USDC - $55.61

Labels

bug
grade-a
high quality report
QA (Quality Assurance)
selected for report
sponsor acknowledged
Q-36

External Links

QA Report for Revert Lend

Table of Contents

Issue IDDescription
QA-01MIN_TWAP_SECONDS is too low and an avenue to ingest manipulated prices
QA-02Unhandled Chainlink revert would halt core functionalities
QA-03_checkApprovals should be reimplemented to count for the allowance depleting
QA-04Setters should always have equality checkers
QA-05Allow totalRewardX64 to be set higher via AutoCompound.setReward()
QA-06Pricing logic shouldn't be unavailable in any case or at least the provider of the secondary source should not halt the system
QA-07Introduce better naming conventions
QA-08Introduce a contract existence check before a low level call to send ETH
QA-09safeIncreaseAllowance has been deprecated

QA-01 MIN_TWAP_SECONDS is too low and an avenue to ingest manipulated prices

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/automators/Automator.sol#L86-L98

    function setTWAPConfig(uint16 _maxTWAPTickDifference, uint32 _TWAPSeconds) public onlyOwner {
        if (_TWAPSeconds < MIN_TWAP_SECONDS) {
            revert InvalidConfig();
        }
        if (_maxTWAPTickDifference > MAX_TWAP_TICK_DIFFERENCE) {
            revert InvalidConfig();
        }
        emit TWAPConfigChanged(_TWAPSeconds, _maxTWAPTickDifference);
        TWAPSeconds = _TWAPSeconds;
        maxTWAPTickDifference = _maxTWAPTickDifference;
    }

One can see that the accepted TWAP seconds could be as low as 60 seconds which is just a minute and one could say is even same with a spot price, this is cause the timeframe is too short and still allows for manipulatable prices.

Impact

Since MIN_TWAP_SECONDS is very low, then this might lead to protocol ingesting manipulated prices

Have the MIN_TWAP_SECONDS value be within the 10-30 minutes range.

QA-02 Unhandled Chainlink revert would halt core functionalities

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L331-L349


    function _getChainlinkPriceX96(address token) internal view returns (uint256) {
        if (token == chainlinkReferenceToken) {
            return Q96;
        }

        TokenConfig memory feedConfig = feedConfigs[token];
    //@audit
        // if stale data - revert
        (, int256 answer,, uint256 updatedAt,) = feedConfig.feed.latestRoundData();
        if (updatedAt + feedConfig.maxFeedAge < block.timestamp || answer < 0) {
            revert ChainlinkPriceError();
        }

        return uint256(answer) * Q96 / (10 ** feedConfig.feedDecimals);
    }

One can see that all pricing logic that requires us to query chainlink at the end of the calls on the _getChainlinkPriceX96(), i.e three out of the 5 accepted pricing modes require the call to this function not to revert.

But evidently, we can see that this call lacks error handling for the potential failure of feedConfig.feed.latestRoundData(), note that Chainlink pricefeeds could revert due to whatever reason, i.e say maintenance or maybe the Chainlink team decide to change the underlying address. Now this omission of not considering thiks call failing would lead to systemic issues, since calls to this would now revert halting any action that requires this call to succeed.

Impact

A revert of the entire operation that requires querying the Chainlink pricing logic, This limitation poses a significant risk, since all three pricing modes that use the Chainlink mode either as a primary or secondary source would now revert.

Wrap the feedConfig.feed.latestRoundData() call in a try-catch block, even if it's only for the pricing logic that use Chainlink as a secondary sourceand then handle the error (e.g., revert with a specific message or use an alternative pricing method) and also apply other chainlink safety integration checks from here: https://medium.com/cyfrin/chainlink-oracle-defi-attacks-93b6cb6541bf

QA-03 _checkApprovals should be reimplemented to count for the allowance depleting

Proof of Concept

The function _checkApprovals is designed to only set approvals if the current allowance is zero. This is a one-time setup meant to minimize gas costs associated with setting allowances for token transfers.

function _checkApprovals(address token0, address token1) internal {
    uint256 allowance0 = IERC20(token0).allowance(address(this), address(nonfungiblePositionManager));
    if (allowance0 == 0) {
        SafeERC20.safeApprove(IERC20(token0), address(nonfungiblePositionManager), type(uint256).max);
    }
    uint256 allowance1 = IERC20(token1).allowance(address(this), address(nonfungiblePositionManager));
    if (allowance1 == 0) {
        SafeERC20.safeApprove(IERC20(token1), address(nonfungiblePositionManager), type(uint256).max);
    }
}

While setting the allowance to type(uint256).max reduces the need for repeated approvals (and thus saves gas), it neglects the scenario where the allowance might be fully utilized. In typical use cases, reaching type(uint256).max would require an unrealistic volume of transactions. However, it does not account for potential bugs, exploits, or changes in contract logic that could deplete this allowance unexpectedly.

Impact

The current implementation of the _checkApprovals function sets the token allowance to type(uint256).max for the nonfungiblePositionManager contract, intending to save on gas costs for future transactions. However, this approach introduces a vulnerability where, once the type(uint256).max allowance is exhausted, there would be no mechanism in place to renew the approval. This could lead to a situation where the smart contract is unable to perform operations requiring token transfers on behalf of users, effectively freezing any functionality dependent on these approvals.

Instead of only checking for an allowance of zero, implement a mechanism to check if the allowance is below a certain threshold and, if so, replenish it.

QA-04 Setters should always have equality checkers

Proof of Concept

Use this search prompt code: https://github.com/search?q=repo%3Acode-423n4%2F2024-03-revert-lend%20function%20set&type=code

Evidently we can see that multiple setter functions exist in scope, with them not having any check that the new accepted value is not equal to the previously stored value.

For e.g setReward() cehcks that the new value is "<=" where as it should only check "<" leading to an unnecessary update of totalRewardX64 if _totalRewardX64 is already == totalRewardX64.

    function setReward(uint64 _totalRewardX64) external onlyOwner {
        require(_totalRewardX64 <= totalRewardX64, ">totalRewardX64");
        totalRewardX64 = _totalRewardX64;
        emit RewardUpdated(msg.sender, _totalRewardX64);
    }

Another example would be the below that does not implement any checks whatsoever:

function setWithdrawer(address _withdrawer) public onlyOwner { emit WithdrawerChanged(_withdrawer); withdrawer = _withdrawer; }

And so on.

Impact

Unnecessary code execution

Introduce equality checkers for setter functions

QA-05 Allow totalRewardX64 to be set higher via AutoCompound.setReward()

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/ac520c5fedf4e1654c597a46efaf5a7c27295de1/src/transformers/AutoCompound.sol#L243-L247

    function setReward(uint64 _totalRewardX64) external onlyOwner {
        require(_totalRewardX64 <= totalRewardX64, ">totalRewardX64");
        totalRewardX64 = _totalRewardX64;
        emit RewardUpdated(msg.sender, _totalRewardX64);
    }

This function is used to set the reward value, keep in mind that there are no checks for e.g that the provided _totalRewardX64 != 0, now if this value gets set to 0 it can never be changed cause as implemented in code it can only be lower in value

Impact

Low since this requires admin mistake, but the whole rewards logic could be broken

Allow this value to be reset to atleast an accepted max.

QA-06 Pricing logic shouldn't be unavailable in any case or atleast the provider of the secondary source should not halt the system

Proof of Concept

The relevant code segment attempts to calculate TWAP using the pool.observe function with two time points, 'now' and 'now - twapSeconds'. This method is prone to failure if the pool does not have enough historical data to cover the requested time span, a scenario not uncommon for newer or less active pools, i.e

    function _getReferencePoolPriceX96(IUniswapV3Pool pool, uint32 twapSeconds) internal view returns (uint256) {
        uint160 sqrtPriceX96;
        // if twap seconds set to 0 just use pool price
        if (twapSeconds == 0) {
            (sqrtPriceX96,,,,,,) = pool.slot0();
        } else {
            uint32[] memory secondsAgos = new uint32[](2);
            secondsAgos[0] = 0; // from (before)
            secondsAgos[1] = twapSeconds; // from (before)
            //@audit-issue pool.observe could fail due to lack of history.
            (int56[] memory tickCumulatives,) = pool.observe(secondsAgos); // pool observe may fail when there is not enough history available (only use pool with enough history!)
            int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds)));
            sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick);
        }

        return FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, Q96);
    }

Now the lack of error handling for the potential failure of pool.observe due to insufficient historical data. This omission would lead to systemic issues, particularly in financial systems that rely on continuous access to accurate pricing data for operational stability.

Impact

The method _getReferencePoolPriceX96 within the V3Oracle contract is designed to calculate the reference price from a Uniswap V3 pool, either directly from the latest slot price or through a Time-Weighted Average Price (TWAP) if a twapSeconds parameter is specified. An identified issue arises when the pool.observe function is called for TWAP calculation: if there is insufficient historical data within the specified twapSeconds, the call to pool.observe could fail, leading to a revert of the entire operation. This limitation poses a significant risk, since all three pricing modes that use the TWAP mode either as a primary or secondary source would now revert.

Wrap the pool.observe call in a try-catch block, even if it's only for the pricing logic that use TWAP as a secondary source.

```solidity try pool.observe(secondsAgos) returns (int56[] memory tickCumulatives,) { int24 tick = int24((tickCumulatives[0] - tickCumulatives[1]) / int56(uint56(twapSeconds))); sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick); } catch { // Handle the error (e.g., revert with a specific message or use an alternative pricing method) } ```

QA-07 Introduce better naming conventions

Proof of Concept

Take a look at (https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L25-L28) we can see that this is used to declare the MIN_PRICE_DIFFERENCE as 2%, issue is that this value is actually the min max price difference, i.e looking at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Oracle.sol#L183-L191

    function setMaxPoolPriceDifference(uint16 _maxPoolPriceDifference) external onlyOwner {
        if (_maxPoolPriceDifference < MIN_PRICE_DIFFERENCE) {
            revert InvalidConfig();
        }
        maxPoolPriceDifference = _maxPoolPriceDifference;
        emit SetMaxPoolPriceDifference(_maxPoolPriceDifference);
    }

We can see that whenever setting the max pool price difference, it's checked to not be lower than our MIN_PRICE_DIFFERENCE

Impact

Confusion in naming conventions leading to hard time of users/developers understanding code.

Consider renaming the vcariable and take into acount that it's the minimum max difference between the prices.

QA-08 Introduce a contract existence check before a low level call to send ETH

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/automators/Automator.sol#L125-L138

    function withdrawETH(address to) external {
        if (msg.sender != withdrawer) {
            revert Unauthorized();
        }

        uint256 balance = address(this).balance;
        if (balance > 0) {
            //@audit QA low level call
            (bool sent,) = to.call{value: balance}("");
            if (!sent) {
                revert EtherSendFailed();
            }
        }
    }

Now consider this minimal contract as a POC



// SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;
import "hardhat/console.sol";
contract test {
constructor() {
bytes memory txData*; (bool success,) = payable(address(0)).call{ value: 0 }(txData*); console.log("success",success);
}
}

One can see that the even as we used an invalid address, 0x0, the call still comes back successful, whereas it shouldn't.

Recommedned Mitigation Steps

Introduce a contract existence check before transferring eth.

QA-09 safeIncreaseAllowance has been deprecated

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/transformers/LeverageTransformer.sol#L75-L80

            amount1 += amountOut;
        }
//@todo QA safeIncreaseAllowance has been deprecated
        SafeERC20.safeIncreaseAllowance(IERC20(token0), address(nonfungiblePositionManager), amount0);
        SafeERC20.safeIncreaseAllowance(IERC20(token1), address(nonfungiblePositionManager), amount1);

Impact

Usage of deprecated functions.

Do not use deprecated functions

#0 - c4-pre-sort

2024-03-24T21:02:50Z

0xEVom marked the issue as sufficient quality report

#1 - c4-pre-sort

2024-03-25T20:42:05Z

0xEVom marked the issue as high quality report

#2 - 0xEVom

2024-03-25T20:44:46Z

GA-05: reckless admin mistake, invalid QA-09: invalid

#3 - kalinbas

2024-03-27T16:05:48Z

QA-01 Depending on the network 1 minute may be enough, also TWAP is only used to verify prices in the standard scenario where Chainlink prices are used mainly. 30 minutes is way to much when prices move fast. QA-02 If chainlink is down, only liquidiate, decreaseLiquidity and borrow functionality would be down. Thats why there is the emergency mode to disable chainlink completely if needed. QA-03 This will never reach 0 for a normal token (and this code is already deployed) - so we will leave it QA-04 Only called by admin so not that important QA-05: Invalid QA-06 Will be part of adding new tokens taking care that pools have enough history. If there is a problem with it, the history size may be increased by anyone. QA-07 Ok agree QA-08 No, its ok that ETH is sent to any address. QA-09: Invalid

#4 - c4-sponsor

2024-03-27T16:09:30Z

kalinbas (sponsor) acknowledged

#5 - c4-judge

2024-03-31T00:06:59Z

jhsagd76 marked the issue as grade-a

#6 - c4-judge

2024-04-01T15:38:44Z

jhsagd76 marked the issue as selected for report

#7 - jhsagd76

2024-04-12T00:28:39Z

Invalid

  • QA-01: Depending on the network 1 minute may be enough, also TWAP is only used to verify prices in the standard scenario where Chainlink prices are used mainly. 30 minutes is way to much when prices move fast.
  • QA-02: If chainlink is down, only liquidiate, decreaseLiquidity and borrow functionality would be down. Thats why there is the emergency mode to disable chainlink completely if needed.
  • QA-05: The current condition is correct.
  • QA-06: Will be part of adding new tokens taking care that pools have enough history. If there is a problem with it, the history size may be increased by anyone.
  • QA-08: No, its ok that ETH is sent to any address.
  • QA-09: Yep, its deprecated. But the use in the code is correct (0 approval is guaranteed each time)

Low

  • QA-03

Downgraded Low

  • #98
  • #119
  • #129

NC

  • QA-04
  • QA-07

Summary

1 Low 2 NC + 3 Downgraded QA Low

#8 - thebrittfactor

2024-04-24T21:35:57Z

Just a note that C4 is excluding the invalid entries from the official report.

Findings Information

🌟 Selected for report: 14si2o_Flint

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

Labels

analysis-advanced
grade-b
sufficient quality report
edited-by-warden
A-07

Awards

83.0533 USDC - $83.05

External Links

Analysis Report for Revert Lend

Alttext

Table of Contents

Approach

Started with a comprehensive analysis of the provided docs and whitepaper to understand protocol functionality and key points, ambiguities were discussed with the sponsors(devs), and possible risk areas were outlined.

Followed by a manual review of each contract in scope, testing function behavior, protocol logic against expectations, and working out potential attack vectors. Vulnerabilities related to dependencies and inheritances, were also assessed. Comparisons with similar protocols was also performed to identify recurring issues and evaluate fix effectiveness.

Finally, identified issues from the security review were compiled into a comprehensive audit report.

Brief Overview

Revert Lend stands as a peer-to-pool lending platform, tailor-made for AMM Liquidity Providers , specifically within the Uniswap v3 ecosystem. It introduces an opportunity for users to secure ERC-20 token loans by using their Uniswap v3 liquidity provider positions as collateral. What seems to set this protocol apart is its capacity to allow Liquidity Providers to keep managing their capital actively within the Uniswap v3 pools, even while it serves as collateral. This functionality ensures that Liquidity Providers can continuously optimize and manage their positions to adapt to the fluctuating demands of the market, providing an uninterrupted liquidity provision experience.

Scope and Architecture Overview

Automators


Automator.sol

This contract enables the automatic reinvestment of fees into liquidity positions with potential for swaps to optimize token ratios, all within a single transaction. Controlled by a Revert bot operator and capable of direct and vault-based position transformations, this contract keeps track of compounding actions, configuration adjustments, and token balance changes, providing a comprehensive tool for liquidity management. The system also facilitates the management of rewards and leftover balances, ensuring a seamless process for liquidity providers to maximize their yield through strategic reinvestment and position optimization.

Configuration and Management
  • Operators and Vaults: It allows the contract owner to designate specific addresses as operators or vaults, enabling them to execute certain privileged actions within the contract's ecosystem.

  • Withdrawer: A specific address that's authorized to withdraw accumulated fees or tokens from the contract. This role is typically reserved for the contract owner or a trusted entity.

  • TWAP Configuration: The contract owner can set parameters related to Time Weighted Average Price (TWAP) calculations, including the observation period (TWAPSeconds) and the maximum allowable tick difference (maxTWAPTickDifference) to mitigate price manipulation risks.

TWAP Validation
  • Swap Validation: Before performing swaps, the contract validates them against current market conditions and configured TWAP parameters to ensure they're executed within acceptable price ranges and slippage limits.

  • TWAP Calculation: Utilizes Uniswap V3's observe function to calculate TWAP over a specified period, assisting in price verification and ensuring swaps don't deviate significantly from the market price.

Liquidity Management
  • Decrease Liquidity and Collect: Allows for reducing a position's liquidity in a Uniswap V3 pool and collecting accrued fees. This operation can be crucial for automated strategies that adjust position sizes in response to market conditions.
Token and ETH Withdrawals
  • Balance Withdrawals: The contract supports withdrawing ERC20 token balances and ETH, facilitating the management of accrued fees or rebalancing of assets.

  • WETH Handling: Includes mechanisms for wrapping and unwrapping WETH (Wrapped ETH), allowing the contract to seamlessly handle ETH in operations requiring ERC20 tokens.

Access Control and Security
  • Operator and Vault Checks: Enforces checks to ensure that only authorized operators can perform certain actions and that interactions with vaults are restricted to approved contracts.

  • Ownership Validation: For operations involving Uniswap V3 positions or vault-held assets, the contract verifies that the caller owns the relevant NFT or vault asset.

Event Logging
  • Logs significant administrative actions, such as changes to the operator list, vault list, withdrawer address, and TWAP configuration, aiding in transparency and auditability.
Error Handling
  • Implements custom error messages for unauthorized access, invalid configurations, and operational failures, enhancing contract usability and debuggability.
Eth Handling
  • Receive Function: Includes a fallback receive function to handle ETH, specifically for unwrapping WETH, ensuring that the contract can accept ETH refunds or payments as needed.

AutoExit.sol

This contract introduces an innovative approach to managing Uniswap V3 positions, enabling automatic execution of limit or stop-loss orders based on specific market conditions. This functionality allows liquidity providers to either remove their liquidity position or swap it to the opposite token when the market reaches a predetermined tick, effectively serving as an automated risk management tool. Operated by a Revert controlled bot, the contract utilizes external swap routers to ensure optimized swap execution. Position owners must approve the AutoExit contract to manage their positions, which can be configured for automatic execution using the contract's configToken method. Once a position reaches its trigger tick, the operator can execute the pre-configured action, with the event details broadcasted for transparency.

Constructor
  • Initializes the contract by setting up necessary parameters such as the Nonfungible Position Manager, operator, withdrawer, TWAP seconds, TWAP tick difference, and routers for external swaps.
Event Logging
  • Executed: Logs when a position is executed, including details about the execution such as whether it was a swap or removal, the amounts returned, and the tokens involved.
  • PositionConfigured: Logs the configuration of a position, detailing the active status, swap behavior, trigger ticks, slippage settings, and reward parameters.
Position Configuration
  • PositionConfig struct: Defines the configuration for each position, including whether it's active, swap behaviors, trigger ticks for execution, slippage tolerances, whether only fees should be used for rewards, and the maximum reward percentage.
  • configToken: Allows the owner of a Uniswap V3 position to configure how their position should be managed by the contract, including setting up the conditions for automated exit strategies.
Execution
  • ExecuteParams struct: Contains parameters required for executing a position's configured strategy, including swap details, liquidity information, minimum amounts for removal, deadline, and reward percentage.
  • execute: Can be called by an authorized operator to execute a position's exit strategy based on its configuration. It validates the position's state, performs necessary swaps or liquidity removal, applies configured rewards, and transfers the resulting assets back to the position owner. It also deactivates the position's configuration to prevent re-entrancy or duplication of execution.
    • The function performs checks to ensure it's only called when the market conditions meet the predefined triggers (ticks) and validates the swap against the current pool price and configured slippage.
    • It manages reward calculation based on whether the strategy involves swapping and whether only fees are considered for rewards.
Access Control and Security
  • Enforces that only the position owner can configure a token and that only authorized operators can execute configured strategies, ensuring that positions are managed according to their owners' intentions and protected against unauthorized access.
TWAP Validation and Liquidity Management
  • Inherits functionalities from the Automator contract for validating swaps against TWAP settings and managing liquidity in Uniswap V3 pools, ensuring that exit strategies are executed in line with market conditions and within acceptable price ranges.

Transformers


LeverageTransformer.sol

This contract provides functionalities for directly leveraging or deleveraging positions within a single transaction in the vault system, specifically it allows users to increase their position's leverage by borrowing a specified amount, swapping borrowed tokens to desired assets, and then adding those assets to their Uniswap V3 position. Conversely, it supports the reduction of leverage by removing liquidity from the position, optionally swapping assets back to the loan token, and repaying the borrowed amount.

Constructor
  • Initializes the contract with references to the Nonfungible Position Manager and routers for swaps, enabling the leveraging functionalities.
Leverage Up
  • leverageUp Method: Allows increasing the leverage of a Uniswap V3 position by borrowing additional assets (either token0 or token1 of the Uniswap V3 position), optionally swapping a portion of the borrowed asset to another, and adding both assets back as liquidity to the position.
    • Parameters include details about the amount to borrow, swap specifics (amounts in, minimum amounts out, swap data for 0x API calls), liquidity addition tolerances, and the recipient for any leftover tokens after adding liquidity.
    • This method calls external swap functionalities (through inherited Swapper functionalities) to execute any necessary swaps and interacts with a vault contract to manage the borrowing of assets.
Leverage Down
  • leverageDown Method: Facilitates decreasing the leverage of a position by removing liquidity, optionally swapping some of the removed assets back to the borrowed asset type, and repaying the borrowed amount to the vault.
    • Parameters include the specifics of the liquidity to remove, swap details for converting assets back to the borrowed asset, minimum amounts expected from swaps, and the address to send any leftover tokens.
    • Similar to leverageUp, it uses the Swapper functionalities for executing swaps and interacts with a vault contract for the repayment of the borrowed assets.
Swap Integration
  • Both leverage up and down functionalities rely on the Swapper contract's swap mechanisms to execute asset conversions through predefined swap routes, allowing for efficient market operations based on live swap data.
Vault Interaction
  • Interacts with a vault contract (referenced as IVault) for borrowing and repaying assets. This integration allows the contract to leverage or deleverage positions based on external conditions and configurations.
Use Cases
  • Leverage Up: Users can increase their exposure to price movements of a Uniswap V3 pool's underlying assets without adding more of their own capital, potentially amplifying returns (and risks).
  • Leverage Down: Users can reduce their exposure to minimize risk or lock in profits without completely exiting their positions, providing flexibility in managing their investments.
Security Considerations
  • The contract expects certain operations like borrowing and repaying to be executed through a trusted vault contract, which should implement adequate security measures to manage these financial operations safely.
  • It includes checks to ensure swaps meet minimum output requirements and deadlines, protecting users from slippage and front-running.

AutoRange.sol

This contract facilitates the automatic adjustment of Uniswap V3 positions' ranges via a controlled bot, aiming to optimize liquidity positions based on market movements. This contract enables operators to reconfigure the range of approved positions to maintain optimal placement within the Uniswap V3 pool. When a position is adjusted, a new position is created with settings mirroring the original, and the configuration is transferred to this new position. For positions housed within a Vault, the contract invokes a transform call to execute the range adjustment. Key features here include leveraging Uniswap's flash loans for liquidity adjustments and managing position configurations through struct mappings, ensuring operational flexibility and efficiency in liquidity management strategies.

Event Logging
  • RangeChanged: Logs when a position's range has been successfully changed, capturing both the old and new token IDs.
  • PositionConfigured: Logs the configuration of a position with details on tick limits, tick deltas for range adjustment, slippage parameters, and reward settings.
Constructor
  • Initializes with references to Nonfungible Position Manager, operator, withdrawer, TWAP settings, routers, and additional configuration parameters for automated operations.
Position Configuration
  • PositionConfig Struct: Holds configuration details for range adjustment of a position, including tick limits for triggering adjustments, tick deltas for new range boundaries, slippage tolerances for swaps, and reward calculation parameters.
  • configToken Function: Allows users or designated vaults to configure how a position should automatically adjust its range based on the specified parameters.
Automated Range Adjustment
  • executeWithVault and execute Functions: Executed by the designated operator, these functions adjust the range of a configured position when market conditions meet predefined triggers. The adjustment process can include swapping tokens to optimize position composition before adjusting the range.
    • When a position's range is adjusted, a new position is minted with the adjusted range, and the original position's configuration is transferred to the new position. If the position is held within a vault, the transform method is called to execute the range adjustment.
    • The process ensures that positions remain aligned with market conditions, potentially improving the position's profitability or reducing risk.
Swap Integration
  • Utilizes the inherited Swapper functionalities to execute necessary swaps as part of the range adjustment process. This allows for efficient market operations and ensures that positions are adjusted with optimal asset composition.
Vault Interaction
  • Supports interaction with vault contracts for positions held within vaults. This feature allows for seamless range adjustments of positions managed by vaults, extending the contract's utility to more complex DeFi strategies.
Security and Access Control
  • Enforces strict access control, allowing only authorized operators or vaults to execute range adjustments. This prevents unauthorized modifications of positions and ensures that adjustments are made in the best interest of the position holders.
Reward Mechanism
  • Incorporates a reward mechanism for the operator, calculated based on the configured parameters, incentivizing the maintenance and adjustment of positions to optimize performance.

AutoCompound.sol

This contract enables automatic reinvestment of fees into liquidity positions with potential for swaps to optimize token ratios, all within a single transaction. Controlled by a Revert bot operator and capable of direct and vault-based position transformations, this contract integrates with Uniswap's INonfungiblePositionManager, supporting multicall for batch operations and safeguarded by a reentrancy guard. It keeps track of compounding actions, configuration adjustments, and token balance changes, providing a comprehensive tool for liquidity management. The system also facilitates the management of rewards and leftover balances, ensuring a seamless process for liquidity providers to maximize their yield through strategic reinvestment and position optimization.

Constructor
  • Sets up the contract with necessary configurations, including references to the Nonfungible Position Manager and specified TWAP settings for swap validation.
Event Logging
  • AutoCompounded: Logs details of the compounding action, including amounts added to the position, rewards generated, and tokens involved.
  • RewardUpdated: Logs changes to the reward configuration initiated by the contract owner.
  • BalanceAdded / BalanceRemoved / BalanceWithdrawn: Logs the management of token balances associated with positions, whether adding to the balance, removing from it, or withdrawing it to a specific address.
Position Balances Management
  • Maintains a mapping of position token balances (positionBalances) to track the accumulated fees and rewards for each position. This allows for precise management of resources when compounding positions.
Compounding Functionality
  • executeWithVault / execute Functions: Allow for the compounding of positions either directly or through a vault. These functions facilitate the collection of fees, optional swapping of tokens for optimal position alignment, and the addition of liquidity to the Uniswap V3 position.
    • Compounding can include fee collection, token swaps based on market conditions (validated through TWAP to prevent manipulation), and the reinvestment of tokens back into the position to increase its size.
    • The process is designed to optimize the position's value over time by reinvesting earnings and adjusting its composition according to predefined strategies.
Reward Mechanism
  • Implements a reward mechanism to incentivize the operator's actions, with rewards calculated as a percentage of the fees generated from compounding actions. The total allowable reward percentage (totalRewardX64) is configurable by the contract owner, subject to a maximum limit.
Security and Access Control
  • Includes non-reentrancy guards to prevent potential security vulnerabilities during execution of compounding actions.
  • Enforces access control, allowing only authorized operators or vaults to initiate compounding actions, ensuring that positions are managed securely and according to their owners' preferences.
Withdrawal Functions
  • withdrawLeftoverBalances: Enables position owners to withdraw any leftover tokens after compounding actions have been executed.
  • withdrawBalances: Allows the contract's designated withdrawer to remove accumulated protocol fees from the contract.
Reward Configuration
  • setReward: Allows the contract owner to adjust the reward percentage allocated to the protocol from compounding actions, providing flexibility in managing incentives for operators.
Utility Functions
  • _checkApprovals: Ensures that the contract has the necessary approvals to manage the position's tokens, optimizing gas costs by setting max allowances where needed.

V3Utils.sol

This contract provides a suite of utility functions to interact with Uniswap V3 positions, including complex operations like swap, mint, and liquidity management, without holding any ERC20 or NFT assets itself. It extends the Swapper contract for executing swaps and integrates with the Permit2 system for EIP-712 style token spending permissions. Key functionalities include executing pre-defined instructions on NFTs, such as changing liquidity ranges, compounding fees into liquidity, or withdrawing and swapping tokens, all within atomic transactions. It supports direct execution with or without EIP712 permits and leverages flash loans for liquidations. The contract operates in a completely ownerless and stateless manner, emphasizing flexibility and upgradability for Uniswap V3 position management.

Constructor
  • Sets up essential components like the Nonfungible Position Manager, swap routers, and the Permit2 contract for permit-based operations.
Operations
  • Execute with Permit: Allows operations on Uniswap V3 positions using EIP712 permits, facilitating actions without requiring upfront token approvals.
  • Execute: Executes configured instructions on Uniswap V3 positions, supporting a range of operations including changing ranges, compounding fees, and withdrawing or collecting fees followed by swaps.
  • Swap: Facilitates direct token swaps with slippage control, optionally handling wrapped ETH conversions.
  • Swap and Mint: Performs token swaps (if necessary) and uses the output tokens to mint a new Uniswap V3 position, applying to both specified tokens and other tokens if configured.
  • Swap and Increase Liquidity: Allows for token swaps and the addition of liquidity to an existing Uniswap V3 position, optimizing position value.
  • Each operation can handle WETH wrapping/unwrapping if specified, accommodating strategies involving ETH.
Integration and Extensions
  • Inherits Swapper for its swap capabilities, allowing efficient market operations and liquidity management.
  • Implements IERC721Receiver to handle NFTs safely, facilitating operations on Uniswap V3 positions directly through transfers.
  • Utilizes OpenZeppelin's ReentrancyGuard to prevent reentrancy attacks, ensuring the security of transactions.
Events
  • Various events (CompoundFees, ChangeRange, WithdrawAndCollectAndSwap, etc.) are emitted to log significant actions performed on Uniswap V3 positions, aiding in transparency and traceability.
Flexibility and Security
  • The contract offers high flexibility for Uniswap V3 position management, allowing users to configure detailed actions based on market conditions or strategic needs.
  • Implements checks to ensure operations are authorized and secure, including validations for permits and NFT ownership.
  • Designed to be ownerless and stateless, the contract focuses purely on executing operations without managing state, reducing the risk profile associated with holding assets.
Utility and Efficiency
  • Through the use of permit signatures and direct NFT transfers, V3Utils minimizes the need for multiple transactions and approvals, streamlining operations for Uniswap V3 liquidity providers.
  • Offers a platform for advanced strategies like automatic range adjustments or fee compounding, enhancing liquidity provision strategies on Uniswap V3.

Utils

Swapper.sol

Contract is designed to facilitate token swaps utilizing various decentralized exchange (DEX) protocols, including Uniswap V3 and 0x Exchange. It acts as an intermediary to execute trades with external routing based on pre-calculated swap instructions, ensuring slippage control through a minimum output amount parameter. Key features include integration with Uniswap V3's non-fungible position manager for liquidity management, support for WETH transactions, and the ability to interact with a universal router for executing complex trade strategies. The contract emits events for each swap, providing transparency over trade details such as input and output token addresses and amounts. r

Swap Execution
  • Router Swap (_routerSwap): Facilitates token swaps using external routing protocols, employing off-chain calculated instructions. It involves slippage checks to ensure the minimum amount of output tokens (amountOutMin) is received. It supports integration with multiple routers like 0x Exchange Proxy and Uniswap's Universal Router.

  • Direct Pool Swap (_poolSwap): Executes swaps directly within a specified Uniswap V3 pool. It requires the contract to have available amounts of both participating tokens. Similar to router swaps, it includes slippage control to ensure the output meets a predefined minimum.

Swap Callback
  • Uniswap V3 Callback (uniswapV3SwapCallback): Required for participating in Uniswap V3 swaps, this callback function facilitates the transfer of the necessary input tokens to the Uniswap pool after a swap is initiated but before it's completed.
Swap Data Structures
  • ZeroxRouterData: Contains information specific to executing swaps via the 0x Exchange Proxy, including the target address for token allowance and the data payload for the swap.
  • UniversalRouterData: Designed for the Uniswap Universal Router, detailing commands and inputs for the swap, alongside a deadline for execution.
Support for Diverse DeFi Protocols
  • Integrates with key components of the Uniswap V3 ecosystem, including the Non-Fungible Position Manager for managing liquidity positions and the WETH9 contract for handling wrapped ETH transactions.
  • The contract is designed to be flexible, allowing for the addition of other protocols and routing mechanisms through external routers.
Event Logging
  • Swap Event: Emits details of each swap transaction, including the input and output tokens, and the amounts involved. This aids in transparency and tracking of swap operations.
Error Handling and Security
  • Extends a custom IErrors interface for consistent error management across various operations, enhancing the contract's robustness and ease of debugging.

FlashloanLiquidator.sol

Contract builds on swapper's functionality and utilizing Uniswap V3's flash loans, enables atomic liquidation of loans alongside necessary token swaps. It integrates with V3 vault system to perform loan liquidations by acquiring assets through Uniswap V3 flash loans, swapping assets if necessary to meet liquidation requirements, and ensuring a minimum reward for the liquidator. This process is done within the liquidate function that orchestrates the flash loan, asset swaps, and loan liquidation in a single transaction. Upon completion, the contract settles the flash loan, returns any excess assets to the liquidator, and verifies that the liquidator's minimum reward condition is met, enhancing operational efficiency and security in liquidation scenarios.

The FlashloanLiquidator contract extends the Swapper contract to perform atomic liquidations of loans using Uniswap V3's flash loans. Here are its key functionalities:

Flash Loan Initiation for Liquidation
  • Liquidate Function: Initiates the liquidation process for a specified loan. It first checks if the loan is eligible for liquidation by ensuring the liquidation cost is non-zero. It then determines the correct asset to flash borrow (based on the vault's asset) and initiates a flash loan from a Uniswap V3 pool to cover the liquidation cost.
Handling the Flash Loan Callback
  • Uniswap V3 Flash Callback: Implements the uniswapV3FlashCallback function required for handling flash loans. This function is automatically called by the Uniswap V3 pool after issuing a flash loan. Within this callback, the contract:
    • Approves the vault to access the required asset amount for liquidation.
    • Calls the vault's liquidate function to perform the actual loan liquidation.
    • Executes necessary swaps via _routerSwap to convert any borrowed assets not in the form of the liquidation asset back into the liquidation asset.
    • Returns the borrowed amount plus fees back to the Uniswap pool.
    • Ensures that the operation yields at least the minimum required reward for the liquidator, failing which, the transaction is reverted.
Swap Execution

Leverages inherited functionalities from the Swapper contract to perform token swaps. This is utilized within the flash loan callback to manage assets post-liquidation:

  • Swaps for Asset Conversion: After liquidation, if the received tokens are not in the desired asset form, the contract swaps these tokens back to the required asset using previously defined _routerSwap functionality.
Reward Distribution
  • Returning Leftover Tokens and Reward: After fulfilling the flash loan and swap operations, any leftover tokens, including the liquidation reward, are transferred back to the liquidator. This includes:
    • Returning any remaining tokens that were swapped but not needed to repay the flash loan or exceed the liquidation cost.
    • Ensuring the liquidator receives a reward at least equal to the minReward specified, with the contract reverting if this condition is not met.
Error Handling and Security
  • Inherits custom error handling from the IErrors interface and Swapper contract to manage operation-specific errors, such as swap failures or insufficient rewards from liquidation.
Constructor and State Variables
  • Initializes with addresses for necessary components like the Nonfungible Position Manager, 0x Router, and Universal Router, required for swap operations and interacting with the Uniswap V3 ecosystem.

Src

InterestRateModel.sol

This contract offers a dynamic framework for calculating interest rates within athe Vault. It extends the Ownable contract, allowing for governance over crucial parameters affecting borrow and supply rates. This model incorporates a base rate, a multiplier effect based on utilization rates, and a kink point that transitions the interest rate calculation from a linear to a potentially exponential regime, accommodating rapid changes in utilization beyond certain thresholds.

The InterestRateModel contract, incorporating Ownable from OpenZeppelin alongside interfaces IInterestRateModel and IErrors, offers a dynamic framework for calculating both borrow and supply interest rates in a Vault, specifically designed for use within decentralized finance (DeFi) protocols. Here's an outline of its core functionalities and design:

Key Components
  • Interest Rate Calculation: Utilizes a model that factors in the base rate, multiplier, and jump multiplier per year, alongside a "kink" utilization rate, to calculate interest rates. This model allows for the adjustment of interest rates based on the current utilization rate of the protocol, enhancing the flexibility and economic stability of lending operations.

  • Q96 Precision: The contract employs fixed-point arithmetic with a factor of (2^{96}) (Q96) to maintain high precision in interest rate calculations without the need for floating-point numbers, which are not natively supported in Solidity.

  • Yearly Constants: Defines constants like YEAR_SECS to account for the number of seconds in a year, considering leap years, facilitating the annualization of interest rates.

  • Configurable Parameters: Allows the contract owner to set parameters such as the base rate, multiplier, jump multiplier, and kink utilization rate per year, each scaled by Q96 for precision. These parameters determine how interest rates adjust with the protocol's utilization rate.

Events
  • SetValues: Emitted when the interest rate model's parameters are updated, providing transparency and traceability for changes in the model's configuration.
Modifiers and Access Control:
  • onlyOwner: Ensures that only the contract owner can update the interest rate model's parameters, securing the protocol against unauthorized modifications.
Core Methods:
  • getUtilizationRateX96: Calculates the utilization rate given the current cash and debt within the protocol, returning a value scaled by Q96. This rate is crucial for determining how close the protocol is to being fully utilized, which in turn influences interest rates.

  • getRatesPerSecondX96: Given the current cash and debt, calculates the borrow and supply rates per second, each scaled by Q96. The calculation considers whether the current utilization rate is above or below the "kink" point, applying different multipliers accordingly to derive the final rates.

  • setValues: Allows the contract owner to update the model's parameters, including base rate, multipliers, and the kink utilization rate, each expressed on an annual basis and scaled by Q96. This method provides the flexibility to adjust the interest rate model in response to changing market conditions or economic policies.

Design Considerations:
  • The contract is designed to be highly flexible, allowing for detailed customization of the interest rate model based on empirical data or economic strategy.
  • Utilizes fixed-point arithmetic for precision and to accommodate Solidity's limitations regarding floating-point arithmetic.
  • Emphasizes security and controlled access through the onlyOwner modifier, ensuring that adjustments to the model are deliberate and authorized.

V3Oracle.sol

This contract integrates chainlink and Uniswap v3 TWAP oracles, providing a robust mechanism for determining the value of Uniswap v3 LP positions in a specified token. It features a fallback mode for enhanced security, addressing the need for accurate and reliable price information within DeFi applications. By harnessing both Chainlink's feeds for external market prices and Uniswap v3's time-weighted average prices (TWAP), it offers flexibility in price source selection through various operational modes. These modes allow for verification of price consistency across oracles, reducing the risk of price manipulation. The oracle supports configuration for multiple tokens, enabling customization of price verification thresholds and update intervals to ensure data relevancy and integrity. This hybrid approach aims to balance decentralized oracle resilience.

Key Features:
  • Multiple Oracle Modes: Supports different modes of price data retrieval, including direct Chainlink feeds, TWAP from Uniswap V3, and hybrid modes that verify one source against another for added security.
  • Chainlink and Uniswap V3 Integration: Seamlessly integrates with Chainlink for reliable off-chain price feeds and Uniswap V3 for on-chain TWAPs, providing flexibility and redundancy in price sourcing.
  • Emergency Fallback Mode: Includes provisions for an emergency mode that can be activated by a designated admin, allowing for rapid response to adverse conditions like oracle manipulation or failure.
Configuration and Management:
  • Dynamic Oracle Configuration: Offers the ability to dynamically configure oracle sources and parameters for each supported token, including feed addresses, maximum feed age, decimal precision, and mode of operation.
  • Token and Pool Configurations: Supports setting up configurations for each token, including their corresponding Chainlink feeds and reference Uniswap V3 pools, making it adaptable to various token ecosystems.
  • Emergency Admin Controls: Allows an emergency administrator to adjust oracle modes in response to critical situations, enhancing the protocol's resilience against external shocks or manipulations.
Functionality Highlights:
  • Position Value Calculation: Can calculate the total value, fee value, and individual token prices of Uniswap V3 positions in a specified token, factoring in current liquidity, fees accrued, and prevailing market prices.
  • Price Verification: Implements checks to ensure the derived pool price from oracle data does not deviate significantly from the actual pool price, serving as a safeguard against price manipulation.
Events and Updatability:
  • Config Update Events: Emits events for critical updates, including token configurations and oracle mode changes, ensuring transparency and traceability.
  • Admin Functions: Provides functions for the contract owner to update token configurations, oracle modes, and the emergency admin, facilitating ongoing maintenance and adaptation to changing market dynamics.
Design Considerations:
  • The contract's design prioritizes security and data integrity by incorporating mechanisms to verify oracle data and enabling emergency interventions. It also demonstrates a flexible approach to oracle selection, allowing users to choose between or combine different data sources based on reliability, timeliness, and relevance to their specific needs.

V3Vault.sol

The V3Vault contract utilizes Uniswap V3 LP positions as collateral within a framework that strictly adheres to the IERC4626 Vault Standard. It represents a seamless representation of ERC20 shares to denote participation in the lending pool, facilitating a singular ERC20 asset for both borrowing and lending operations. The architecture is deeply integrated with the Uniswap ecosystem, leveraging its Nonfungible Position Manager for efficient liquidity management and swaps. Incorporating a dynamic interest rate model and an oracle for accurate collateral assessment, it ensures competitive rates and secure collateralization. Moreover, it empowers administrators with extensive management capabilities, including adjustable limits and configurations, to adapt to evolving market conditions.

Key Features:
  • Lending and Borrowing: Users can lend ERC20 tokens to earn interest or borrow against their Uniswap V3 LP positions.
  • ERC4626 Compliance: Implements the ERC4626 standard for tokenized vaults, making the vault's shares tradable as ERC20 tokens.
  • Dynamic Interest Rates: Utilizes an interest rate model to dynamically adjust lending and borrowing rates based on market conditions.
  • Collateral Management: Supports multiple tokens as collateral, each configured with a collateral factor to determine its borrowing power.
  • Liquidation Mechanism: Allows positions to be liquidated if they become undercollateralized, ensuring the system's stability.
Administrative Controls:
  • Reserve and Risk Management: Includes mechanisms for managing reserves and setting risk parameters like collateral factors and liquidation penalties.
  • Token and Pool Configuration: Enables the configuration of supported tokens and their Uniswap V3 pools for collateral valuation.
  • Emergency Functions: Provides emergency administrative controls to respond to adverse market conditions or system vulnerabilities.
Core Mechanisms:
  • Position Management: Facilitates the creation, transformation, and management of collateralized positions using Uniswap V3 LP tokens.
  • Interest Rate Adjustment: Periodically recalculates interest rates for both lenders and borrowers to align with market dynamics.
  • Loan Health Checks: Evaluates the health of loans to prevent undercollateralization and initiates liquidation if necessary.
User Interactions:
  • Deposits and Withdrawals: Allows users to deposit ERC20 tokens to lend to the vault or withdraw their lent assets.
  • Borrowing and Repayment: Users can borrow against their collateralized Uniswap V3 positions and repay their loans with flexibility.
  • Collateral Adjustment: Offers functions to manage Uniswap V3 positions used as collateral, including adding liquidity or withdrawing it as part of loan management.
Safety Features:
  • Collateral Factor and Limits: Each token's borrowing power is determined by its collateral factor, and global borrowing limits prevent excessive risk exposure.
  • Reserve Management: Maintains financial reserves to cover potential losses from loan defaults or liquidations, with parameters to ensure sufficient coverage.

Systemic Risks

  • Possible issues with the ecosystem being on an L2 since the sequencer being down would cease all timing logics to now be faulty.
  • Protocol uses secondary sources of pricing data for some pricing logic, but still 100 percent depends on this data, i.e if the query to pool.observe() is to to fail for any reason it's going to halt the system even if Chainlink's latestRoundData() is the primary source for that tx and vice versa.
  • External dependencies from pragma oracle, inherited contracts, etc.
  • Large approvals may not work with some ERC20 tokens which would fault the logic around _checkApprovals() for these tokens
  • Incorrect implementations of supported EIPs, not following the CEI pattern and not using local variables for emitting packed storage variables.

Centralization Risks

  • An admin could just set any value for the twap via Automator::setTWAPConfig() efffectively affecting the priceof the token.
  • An admin could set the reserve protection value very low and financially flaw the system
  • An admin could maliciously set the management reward system and lower the reward to 0, essentially breaking the rewards logic since now it can't be reset to any value whatsoever due to the <= check.
  • An admin could still maliciously withdraw the reserves after reducing the reserved protection section of these reserves.
  • An admin could maliciously use setMaxPoolPriceDifference() to allow for the ingestion of faulty oracle data for innocent users by placing the acceptable max difference to be very high.

Recommendations

  • Enhance documentation quality, currently the code's natspec and protocol's provided docs, do not always align, for example in the docs.
  • Improve protocol's testability, one thing to note about tests is that there's always room for further refinement and improvement., a few out of the box ideas need to also be attached to test cases, for example V3Oracle._getReferencePoolPriceX96() will show incorrect price for negative tick deltas, whcih would've been caught if these were considered during testing.
  • To support the above, even fuzz and invariant tests could be further incorporated to help secure protocol.
  • There seems to be a need to improve the naming conventions in some areas of protocol to ease user/developer experience while trying to use/understand protocol, for example the MIN_PRICE_DIFFERENCEis not the min price difference but actually the min max price difference, i.e looking at trying to set the max price difference we can see that it's checked to not be lower than our MIN_PRICE_DIFFERENCE.
  • Two step variable and address updates should be implemented including zero address checks. A timelock can also be considered for the setter functions to give users time to react to protocol changes. Adding these fixes can help protect from mistakes and unexepected behaviour.
  • Onboard more developers, cause multiple eyes checking out a protocol can be invaluable. More contributors can significantly reduce potential risks and oversights. Evidence of where this could come in handy can be gleaned from codebase typos. Drawing from the broken windows theory, such lapses may signify underlying code imperfections.
  • Re-enhance event monitoring, current implementation subtly suggests that event tracking isn't maximized. Instances where events are missing or seem arbitrarily embedded have been observed.

Security Researcher Logistics

My attempt on reviewing the Codebase spanned over 60 hours distributed over 9 days:

  • 2 hours dedicated to writing this analysis.
  • 6 hours (first day) spent exploring the whole system (docs/whitepaper) to grasp the foundations before diving into it line by line
  • 2 hours were allocated for discussions with sponsors on the private discord group regarding potential vulnerabilities.
  • 2 hours over the course of the 8 days (~15 minutes per day) was spent lurking in the public discord group to get more ideas based on questions other security researchers ask and explanation provided by sponsors
  • The remaining time was spent on finding issues and writing the report for each of them.

Conclusion

The codebase was a very great learning experience, though it was a pretty hard nut to crack, nonetheless during my review, I uncovered a few issues within the protocol and they need to be fixed. Recommended steps should be taken to protect the protocol from potential attacks. Timely audits and codebase cleanup for mitigations should also be conducted to keep the codebase fresh and up to date with evolving security times.

Resources

Time spent:

60 hours

#0 - c4-pre-sort

2024-03-24T08:42:47Z

0xEVom marked the issue as sufficient quality report

#1 - jhsagd76

2024-04-01T11:25:21Z

emm, a little strange

#2 - c4-judge

2024-04-01T11:25:27Z

jhsagd76 marked the issue as grade-b

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