Liquid Staking

Stakelink

DeFiHardhatOracle

50,000 USDC

View results

Previous Next

Submission Details

Severity: medium

Invalid

A forced revert of StakingPool::deposit can cause de-peg of the LST token

0xtimefliez

Summary

The interaction between PriorityPool::_deposit and StakingPool::deposit plus the possibility to revert StakingPool::deposit can cause problems to the peg of the LST.

Vulnerability Details

Within the PriorityPool, the functions PriorityPool::deposit and PriorityPool::_deposit handle user participation in LINK staking. For clarity, we'll focus on the more relevant function, PriorityPool::_deposit.

function _deposit(

address _account,

uint256 _amount,

bool _shouldQueue,

bytes[] memory _data

) internal {

if (poolStatus != PoolStatus.OPEN) revert DepositsDisabled();

uint256 toDeposit = _amount;

if (totalQueued == 0) {

uint256 queuedWithdrawals = withdrawalPool.getTotalQueuedWithdrawals();

if (queuedWithdrawals != 0) {

uint256 toDepositIntoQueue = toDeposit <= queuedWithdrawals

? toDeposit

: queuedWithdrawals;

withdrawalPool.deposit(toDepositIntoQueue);

toDeposit -= toDepositIntoQueue;

IERC20Upgradeable(address(stakingPool)).safeTransfer(_account, toDepositIntoQueue);

}

@> if (toDeposit != 0) {

@> uint256 canDeposit = stakingPool.canDeposit();

@> if (canDeposit != 0) {

@> uint256 toDepositIntoPool = toDeposit <= canDeposit ? @>toDeposit : canDeposit;

@> stakingPool.deposit(_account, toDepositIntoPool, _data);

@> toDeposit -= toDepositIntoPool;

}

} if (toDeposit != 0) {

if (_shouldQueue) {

_requireNotPaused();

if (accountIndexes[_account] == 0) {

accounts.push(_account);

accountIndexes[_account] = accounts.length - 1;

}

accountQueuedTokens[_account] += toDeposit;

totalQueued += toDeposit;

} else {

token.safeTransfer(_account, toDeposit);

}

emit Deposit(_account, _amount - toDeposit, _shouldQueue ? toDeposit : 0);

}

In this function, if there are no queued withdrawals or tokens in the PriorityPool, a user’s deposit is directly placed into the StakingPool via StakingPool::deposit, provided the amount doesn’t exceed StakingPool::canDeposit.

Now, looking at the StakingPool::deposit function:

function deposit(

address _account,

uint256 _amount,

bytes[] calldata _data

) external onlyPriorityPool {

require(strategies.length > 0, "Must be > 0 strategies to stake");

@> uint256 startingBalance = token.balanceOf(address(this));

if (_amount > 0) {

token.safeTransferFrom(msg.sender, address(this), _amount);

_depositLiquidity(_data);

@> _mint(_account, _amount);

totalStaked += _amount;

} else {

_depositLiquidity(_data);

}

@> uint256 endingBalance = token.balanceOf(address(this));

@> if (endingBalance > startingBalance && endingBalance > unusedDepositLimit)

revert InvalidDeposit();

}

In this function, the key observation is that before and after minting LSTs, the contract checks the startingBalance and endingBalance of the pool via balanceOf(). If the endingBalance exceeds the startingBalance and surpasses the unusedDepositLimit, the function reverts.

Exploiting the Revert Condition

A potential attacker can force a revert after minting LSTs by depositing tokens into the contract via StakingPool::donateTokens, which increases the endingBalance beyond the startingBalance. Additionally, the unusedDepositLimit can be exceeded, triggering a revert.

However, this attack is limited by the unusedDepositLimit and it's actual value during initialization, meaning the severity depends on the state of the contract. If the StakingPool is full the attack is basically not executable, while if it is less than 30% full the attack could be viewed as critical. Also an attacker would need a substantial financial incentive to deposit their own funds to force this scenario, while those funds sent via the StakingPool::donateTokens are not withdrawable, but as the name suggests a donation.

Scenario:

StakingPool is roughly 40% full.
The legitimate users of stake.link have their LST tokens mostly spread over various DeFi Protocols to capitalize on additional yield farming possibilities.
Now a malicious user discovers he can open a short position onto the LST in question via for example a lending protocol offering to borrow the LST against any other collateral.
Malicious user decides to take on flash loans from the market to open a relative big short position.
The Malicious user also takes a flash loan to deposit the underlying asset token into the contract via PriorityPool::_deposit and StakingPool::deposit minting up to 60% of the current supply of the LST to dump it at once onto the market, causing the price to drop extremely low.
Malicious User can now pay his short position and potentially liquidate other LST holders in the lending market for additional profits during this rapid price movement.
The transaction from StakingPool::deposit reverts, because the Malicious User used some money to StakingPool::donateTokens to avoid becoming a bag holder.

Impact

LST Price Manipulation: The ability to manipulate LST prices within a single transaction undermines the integrity of the token. Malicious actors could use this to exploit lending protocols by causing the token to deviate from its peg, putting users at risk of liquidation, while utilizing potential short positions or other markets before execution to profit from the downwards movement of prices.
Even though it is out of scope and uninvestigated by myself I want to leave it here as a possibility and on the digression of the judges and team.
Depending on the exact point when the merkle tree is updated, and most certainly the timing of this transaction the merkle tree could be left in an inconsistent state potentially harming other users. Especially since the merkle tree is stored on IPFS, and therefor disconnected from on chain activities like a potential revert, it would be crucial to validate or invalidate the possibility of leaving it in such an inconsistent state.

Tools Used

Manual Review

Recommended Mitigation

One potential mitigation would be to pause StakingPool::donateTokens and other possibilities to manipulate the balanceOf accounting, while execution of StakingPool::deposit, if the revert of said function should persist.
Remove the revert condition. The way the deposit chain works is, that the PriorityPool::_deposit already knows how many tokens can essentially be forwarded and executes on that knowledge. The revert condition should in theory not be necessary.
Remove possibilities for StakingPool to receive funds outside of user interactions in PriorityPool alltogether.