code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Lack of Liquidation Information for can cause loss of funds for the the Liquidizer #837

Closed c4-bot-3 closed 9 months ago

c4-bot-3 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L311-L342

Vulnerability details

Impact

The protocol implements a findLiquidatableUsers to help users liquidate users which users can actually be liquidated, but the main concept of liquidations is about the fact that the value of the collateral has dropped to a value that it cannot effectively back the borrowed usds, the issue is that the findLiquidatableUsers function only returns the address of a liquidatable user and never the value of the collateral that is about to be liquidated, so with this information a user may liquidate a user in which the value of the collateral has dropped so much that the value of the 5% the liquidator will be awarded may be less than the Gas the user used to find and Liquidate the returned address, this Means a loss of funds for the user, as they did not make a informed decision because of the amount of information they had access to.

Proof of Concept

An Clear example will be made to see a loss of funds for the Liquidator

Below is a Coded POC that Shows The findLiquidatableUsers only returns address

 function testFindLiquidatableReturnsAddressAloneAndNotPosition() public {
        // Bob deposits collateral so alice can be liquidated
        vm.startPrank(bob);
        collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(bob), weth.balanceOf(bob), 0, block.timestamp, false );
        vm.stopPrank();

        // Deposit collateral
        vm.startPrank(alice);
        collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(alice), weth.balanceOf(alice), 0, block.timestamp, true );
        assertTrue(_userHasCollateral(alice));
        assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 0);

        // Borrow USDS
        uint256 borrowedAmount = collateralAndLiquidity.maxBorrowableUSDS(alice);
        collateralAndLiquidity.borrowUSDS(borrowedAmount);
        assertEq( collateralAndLiquidity.numberOfUsersWithBorrowedUSDS(), 1);

        // Check that Alice's borrowed amount increased
        assertEq(collateralAndLiquidity.usdsBorrowedByUsers(alice), borrowedAmount);

        // Crash collateral price to enable liquidation
        _crashCollateralPrice();
        vm.warp( block.timestamp + 1 days );

        address[] memory liquidatableUsers = collateralAndLiquidity.findLiquidatableUsers();
        for (uint i = 0; i < liquidatableUsers.length; i++) {
        console.log("Liquidatable User:", liquidatableUsers[i]);
        }
        assertEq(liquidatableUsers.length, 1, "No liquidatable users should be found");
    }

Run with COVERAGE="yes" NETWORK="sep" forge test -vv --rpc-url <<rpc-url>> --match-path src/stable/tests/CollateralAndLiquidity.t.sol --match-test testFindLiquidatableReturnsAddressAloneAndNotPosition

Tools Used

Manual Review

Recommended Mitigation Steps

Inside the findLiquidatableUsers, a collateral value return should also be implemented to help Liquidators make Informed decisions about accounts they are about to liquidate.

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as unsatisfactory: Overinflated severity

Picodes commented 9 months ago

This information is available elsewhere and it's up to liquidators to fetch it?