Wise Lending - AM's results

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L493-L499 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L229-L232

Vulnerability details

It is stated in the documentation of the contest from the c4 site that liquidations are rounded against the liquidator when it will evaluate the amount of tokens they need to payback related to the borrowshares they payoff. This becomes a problem when the amount that will be liquidated will be bigger then the amount of tokens that are available in the pool, when this happens the liquidation process logic will get in the function _withdrawOrAllocateSharesLiquidation which it's role is to cashout all the possible tokens from the pool and if that amount is not enough it will also give some shares to the liquidator. Now, the problem comes from the fact that totalPoolInShares will be rounded down because variable _maxSharePrice is selected as false, while totalPoolToken is not rounded down and represents the actual(real) amount of tokens that are in the pool. After that when the function _coreWithdrawBare is called and those 2 values will be used to update pseudoTotalPool and totalDepositShares ( using functions _decreasePseudoTotalPool and _decreaseTotalDepositShares ) the variables pseudoTotalPool and totalDepositShares will not be synced anymored as one is updated with a rounded down value and the other one with a real value. The variable totalDepositShares will be bigger then pseudoTotalPool and this becomes a problem in _syncPoolAfterCodeExecution because it will determine that lendingSharePriceAfter is smaller then lendingSharePriceBefore breaking the invariant and reverting the liquidation transaction.

Impact

Any liquidation over a pool that does not have enough tokens in it's balance to to pay the liquidator will be blocked

Proof of Concept

Run the test from below using command

forge test -vvvv --match-test "test_liq__create_shares_for_liquidator_correct_price_feeds_price_drop_20_perc" --fork-url mainnet

 function test_liq__create_shares_for_liquidator_correct_price_feeds_price_drop_20_perc() public {
        _useBlock(NEW_BLOCK);
        _deployNewWiseLending({_mainnetFork: true});
        //This is the bug, for this tests to work properly the _maxSharePrice of calculateLendingShares in _withdrawOrAllocateSharesLiquidation needs to be on false
        //Carol initialization
        address carol = vm.addr(0x17896);
  
        //Bob initialization
        uint256 bobPositionId;
        uint256 bobDepositSharesAfterFirstDeposit;
        address bob = vm.addr(0x45678);


        //Alice initialization
        uint256 alicePositionId;
        uint256 alicesDepositSharesAfterFirstDeposit;
        uint256 aliceBorrowSharesAfterFirstBorrow;
        address alice = vm.addr(0x12345);

        vm.deal(alice, 200 ether);
        vm.deal(bob, 200 ether);
        vm.deal(carol, 200 ether);
        deal(USDC_ADDRESS, alice, 20000e6);

        vm.warp(block.timestamp+1);

        (   ,
            int256 goodAnswer,
            ,
            ,
        ) = IPriceFeed(USDC_ETH_PRICE_FEED).latestRoundData();

        (   uint80 roundId,
            ,
            ,
            ,
        ) = IPriceFeed(USDC_USD_PRICE_FEED).latestRoundData();

        vm.mockCall(
            USDC_USD_PRICE_FEED, 
            abi.encodeWithSelector(IPriceFeed(USDC_USD_PRICE_FEED).latestRoundData.selector),
            abi.encode(roundId,goodAnswer,0,0,0)
        );

        vm.mockCall(
            USDC_USD_PRICE_FEED, 
            abi.encodeWithSelector(IPriceFeed(USDC_USD_PRICE_FEED).decimals.selector),
            abi.encode(18)
        );

        //Bob operations
        vm.startPrank(bob);
        bobPositionId = POSITION_NFTS_INSTANCE.mintPosition();
        bobDepositSharesAfterFirstDeposit = LENDING_INSTANCE.depositExactAmountETH{value: 10 ether}(bobPositionId);
        vm.stopPrank();
        console.log("Bob deposit shares: ", bobDepositSharesAfterFirstDeposit );

        //Alice operations
        vm.startPrank(alice);
        IERC20(USDC_ADDRESS).approve(address(LENDING_INSTANCE), 20000e6);
        alicePositionId = POSITION_NFTS_INSTANCE.mintPosition();
        alicesDepositSharesAfterFirstDeposit = LENDING_INSTANCE.depositExactAmount(alicePositionId, USDC_ADDRESS, 20000e6);
        aliceBorrowSharesAfterFirstBorrow = LENDING_INSTANCE.borrowExactAmountETH(alicePositionId, 4.1 ether);
        vm.stopPrank();

        //Carol operations
        vm.startPrank(carol);
        uint256 carolPositionId = POSITION_NFTS_INSTANCE.mintPosition();
        LENDING_INSTANCE.depositExactAmountETH{value: 10 ether}(carolPositionId);
        LENDING_INSTANCE.borrowExactAmount(carolPositionId, USDC_ADDRESS, 4000e6);
        vm.stopPrank();

        
        vm.mockCall(
            USDC_USD_PRICE_FEED, 
            abi.encodeWithSelector(IPriceFeed(USDC_USD_PRICE_FEED).latestRoundData.selector),
            abi.encode(roundId,goodAnswer/10000*8100,0,0,0)
        );
        deal(WETH_ADDRESS, bob, 200e18);
        vm.startPrank(bob);
        IERC20(WETH_ADDRESS).approve(address(LENDING_INSTANCE), type(uint256).max);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            alicePositionId,
            bobPositionId,
            WETH_ADDRESS,
            USDC_ADDRESS,
            aliceBorrowSharesAfterFirstBorrow
        );
        vm.stopPrank();
        
        uint256 numbersOfBobPositions = LENDING_INSTANCE.getPositionLendingTokenLength(bobPositionId);

        console.log("Alice deposit shares: ", alicesDepositSharesAfterFirstDeposit );
        console.log("Alice borrow shares: ", aliceBorrowSharesAfterFirstBorrow );

        console.log("Alice ether balance: ", address(alice).balance);
        console.log("Bob USDC balance: ", IERC20(USDC_ADDRESS).balanceOf(bob));

        console.log("Bob lending positions: ", numbersOfBobPositions);

    }

The output of the test

├─ [0] console::log("lendSharePriceBefore: ", 1000000000049999997 [1e18]) [staticcall]
    │   │   └─ ← ()
    │   ├─ [0] console::log("lendSharePriceAfter: ", 999999999750000062 [9.999e17]) [staticcall]
    │   │   └─ ← ()
    │   └─ ← InvalidAction()
    └─ ← InvalidAction()

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.85s (1.77s CPU time)

Ran 1 test suite in 4.85s (2.85s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in contracts/FindingsTest.t.sol:FindingsTest
[FAIL. Reason: InvalidAction()] test_liq__create_shares_for_liquidator_correct_price_feeds_price_drop_20_perc() (gas: 34530857)

We can clearly observe that lendSharePriceAfter is smaller then lendSharePriceBefore and this is why the transaction is reverted with the error InvalidAction.

Tools Used

Manual Review

Recommended Mitigation Steps

When you are calculating the value of totalPoolInShares round up the value turning _maxSharePrice to true.

        uint256 totalPoolInShares = calculateLendingShares(
            {
                _poolToken: _poolToken,
                _amount: totalPoolToken,
                _maxSharePrice: true
            }
        );

Assessed type

DoS