c4-bot-2 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L154 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L104-L111 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L57-L71 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L70

Vulnerability details

Impact

Liquidation fails because of cooldown expiration of an user. The cooldown can be kept engaged all the time if user calls depositCollateralAndIncreaseShare just to trigger the cooldown everytime it passes

Proof of Concept

The liquidation function in CollateralAndLiquidity can fail if the user Share activity was put in cooldown.

DOS

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

function liquidateUser( address wallet ) external nonReentrant
.
.

        _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );
.
.

We can see in _decreaseUserShare

function _decreaseUserShare( address wallet, bytes32 poolID, uint256 decreaseShareAmount, bool useCooldown ) internal
.
.
if ( useCooldown )
if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown
            {
            require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" );

            // Update the cooldown expiration for future transactions
            user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown();
            }
.
.

There's a require statement that user must not be in a cooldown now, this will cause the Liquidation to fail

Enable cooldown

In collateralAndLiquidity

depositCollateralAndIncreaseShare -> _depositLiquidityAndIncreaseShare -> _increaseUserShare

function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal
.
.
.
if ( useCooldown )
        if ( msg.sender != address(exchangeConfig.dao()) ) // DAO doesn't use the cooldown
            {
            require( block.timestamp >= user.cooldownExpiration, "Must wait for the cooldown to expire" );

            // Update the cooldown expiration for future transactions
            user.cooldownExpiration = block.timestamp + stakingConfig.modificationCooldown();
            }
.
.

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L57-L71

User can activate the cooldown using the deposit mechanism https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L70

Tools Used

Manual analysis

Recommended Mitigation Steps

Change the bool for useCooldown in liquidateUser to false from true

Assessed type

Timing

c4-judge commented 5 months ago

Picodes marked the issue as duplicate of #891

c4-judge commented 4 months ago

Picodes marked the issue as satisfactory

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

Cooldown in _decreaseUserShare can cause Liquidity to fail #981