The logic for liquidating a user involves calling _decreaseUserShare for the user being liquidated, as their entire WBTC-WETH collateral is being taken. However, in this call, the parameter useCooldown=true, which means that you must wait until cooldownExpiration is reached for a user. A malicious user who is getting liquidated can then abuse this by front-running all calls to liquidate themselves by adding a very small amount of LP such that cooldownExpiration is incremented, making the liquidation call revert. This action can be done indefinitely to prevent liquidations forever at very low cost.
function liquidateUser( address wallet ) external nonReentrant
{
require( wallet != msg.sender, "Cannot liquidate self" );
// First, make sure that the user's collateral ratio is below the required level
require( canUserBeLiquidated(wallet), "User cannot be liquidated" );
uint256 userCollateralAmount = userShareForPool( wallet, collateralPoolID );
// Withdraw the liquidated collateral from the liquidity pool.
// The liquidity is owned by this contract so when it is withdrawn it will be reclaimed by this contract.
(uint256 reclaimedWBTC, uint256 reclaimedWETH) = pools.removeLiquidity(wbtc, weth, userCollateralAmount, 0, 0, totalShares[collateralPoolID] );
// Decrease the user's share of collateral as it has been liquidated and they no longer have it.
@> _decreaseUserShare( wallet, collateralPoolID, userCollateralAmount, true );
...
}
As can be seen, there is a call to _decreaseShare with the parameter useCooldown set to true. Let's see how this is used in the StakingRewards:_decreaseUserShare function call:
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();
}
As can be seen, (block.timestamp >= user.cooldownExpiration) is required, else this call will fail. Although this is already an issue in itself, a malicious user can abuse this to continuously increase user.cooldownExpiration every time there is an attempt to liquidate them by simply calling CollateralAndLiquidity:depositCollateralAndIncreaseShare with maxAmountWBTC=101, maxAmountWETH=101. This function does in internal call to _depositLiquidityAndIncreaseShare, which includes the following logic:
Lines of code
https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L140-L188 https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/StakingRewards.sol#L104-L111
Vulnerability details
Impact
The logic for liquidating a user involves calling
_decreaseUserShare
for the user being liquidated, as their entire WBTC-WETH collateral is being taken. However, in this call, the parameteruseCooldown
=true, which means that you must wait untilcooldownExpiration
is reached for a user. A malicious user who is getting liquidated can then abuse this by front-running all calls to liquidate themselves by adding a very small amount of LP such thatcooldownExpiration
is incremented, making the liquidation call revert. This action can be done indefinitely to prevent liquidations forever at very low cost.Proof of Concept
When a user is getting liquidated, the liquidator will call
CollateralAndLiquidity:liquidateUser
, which is defined as follows:As can be seen, there is a call to
_decreaseShare
with the parameteruseCooldown
set totrue
. Let's see how this is used in theStakingRewards:_decreaseUserShare
function call:As can be seen,
(block.timestamp >= user.cooldownExpiration)
is required, else this call will fail. Although this is already an issue in itself, a malicious user can abuse this to continuously increaseuser.cooldownExpiration
every time there is an attempt to liquidate them by simply callingCollateralAndLiquidity:depositCollateralAndIncreaseShare
withmaxAmountWBTC
=101,maxAmountWETH
=101. This function does in internal call to_depositLiquidityAndIncreaseShare
, which includes the following logic:Here
useCooldown
is alsotrue
, meaning thatuser.cooldownExpiration
is updated, allowing the attacker to prevent themselves from getting liquidated.Tools Used
Manual review
Recommended Mitigation Steps
In
CollateralAndLiquidity:liquidateUser
on line 154, the call to_decreaseUserShare
should not haveuseCooldown
=true.Assessed type
Other