Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Incorrect Increase of `totalStaked` in `deposit` Function When `_amount` is Zero

foxb868

Summary

The deposit function has an if-else block that checks if _amount is greater than 0. If it is, the function transfers tokens from the user to the contract, calls _depositLiquidity, mints liquid tokens to the user, and increases totalStaked by the deposited amount. However, if _amount is 0, the function skips the token transfer and directly calls _depositLiquidity without any further checks.

The _depositLiquidity function (StakingPool.sol) deposits available tokens from the contract's balance into strategies. When called with _amount = 0, it can still deposit tokens that are already in the contract's balance, leading to an increase in totalStaked without any actual deposit from the user. https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L111-L127

function deposit(
address _account,
uint256 _amount,
bytes[] calldata _data
) external onlyPriorityPool {
// ...
if (_amount > 0) {
// ... (token transfer, mint, increase totalStaked)
} else {
_depositLiquidity(_data); // <@audit call when _amount is 0
}
// ...
}

This bug could affect users by incorrectly inflating the totalStaked variable, which represents the total amount of tokens staked in the pool. The contract's staked token balance will not match the actual tokens deposited by users.

Vulnerability Details

This vulnerability allows the totalStaked variable to be incorrectly increased when the _amount parameter is zero. This happens because the function does not properly handle the case when _amount is zero and still calls the _depositLiquidity function, which can deposit tokens from the contract's balance into strategies, leading to an unintended increase in totalStaked.

StakingPool.sol:111-127# https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L111-L127

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); // <--- @audit Vulnerability: Calling _depositLiquidity when _amount is 0
}
​
uint256 endingBalance = token.balanceOf(address(this));
if (endingBalance > startingBalance && endingBalance > unusedDepositLimit)
revert InvalidDeposit();
}

The vulnerability is in the else block of the deposit function. When _amount is 0, it calls _depositLiquidity(_data) without any checks, allowing deposits from the contract's balance even though no tokens were transferred from the user.

StakingPool.sol:477-389# https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L477-L489

function _depositLiquidity(bytes[] calldata _data) private {
uint256 toDeposit = token.balanceOf(address(this));
if (toDeposit > 0) {
for (uint256 i = 0; i < strategies.length; i++) {
IStrategy strategy = IStrategy(strategies[i]);
uint256 strategyCanDeposit = strategy.canDeposit();
if (strategyCanDeposit >= toDeposit) {
strategy.deposit(toDeposit, _data[i]); // <--- @audit Vulnerability: Deposits tokens from contract balance into strategies
break;
} else if (strategyCanDeposit > 0) {
strategy.deposit(strategyCanDeposit, _data[i]); // <--- @audit Vulnerability: Deposits tokens from contract balance into strategies
toDeposit -= strategyCanDeposit;
}
}
}
}

The _depositLiquidity function is called by the vulnerable else block in the deposit function. It deposits tokens from the contract's balance (token.balanceOf(address(this))) into strategies without checking if the tokens were actually transferred from the user. This leads to an incorrect increase in totalStaked.

These two parts of the code are connected and contribute to the vulnerability that allows totalStaked to be increased without actual user deposits when _amount is 0 in the deposit function.

Impact

  • The totalStaked variable, which represents the total amount of tokens staked in the pool, can be artificially inflated without corresponding user deposits. This leads to discrepancies between the actual staked tokens and the recorded totalStaked value.

  • If the StakingPool contract distributes rewards based on the totalStaked value, users who didn't actually deposit tokens may receive undeserved rewards, while users who genuinely staked tokens may receive less than their fair share.

Tools Used

Vs

Recommendations

Handle the case when _amount is zero.

} else {
require(_data.length == 0, "Cannot deposit from contract balance when amount is 0");
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.