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

Increase on `totalStaked` via `donateTokens` causes problems

mbushbush

Title

Increase on totalStaked via donateTokens causes problems

Github link

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/base/StakingRewardsPool.sol#L435

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/base/StakingRewardsPool.sol#L579

Summary

donateTokens function of StakingPool contract increases totalStaked amount without considering impacts. Thus, minting shares can be DoSed.

Vulnerability Details

The problem arises at the beginning when total shares is 0. Ahead of any deposit operation, malicious attacker can donate any amount of token to increase totalStaked.

function donateTokens(uint256 _amount) external {
token.safeTransferFrom(msg.sender, address(this), _amount);
-> totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}

At the moment, totalStaked > 0 while totalShares remains 0. This causes problem while minting shares for receivers:

// distribute fees to receivers if there are any
if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
-> _mintShares(address(this), sharesToMint);
​
uint256 feesPaidCount;

sharesToMint will be 0 which will cause _mintShares revert:

function _mintShares(address _recipient, uint256 _amount) internal {
require(_recipient != address(0), "Mint to the zero address");
​
if (totalShares == 0) {
shares[address(0)] = DEAD_SHARES;
totalShares = DEAD_SHARES;
-> _amount -= DEAD_SHARES;
}
​
totalShares += _amount;
shares[_recipient] += _amount;
}

Impact

revert in _mintShares would cause failure of _updateStrategyRewards function which is called from removeStrategy and updateStrategyRewards functions. This means, StakingPool contract is totally blocked and not able to recover.

Tools Used

Manual Review

Recommendations

By donating, it doens't necessarily mean increasing stake. So remove the increase actions:

function donateTokens(uint256 _amount) external {
token.safeTransferFrom(msg.sender, address(this), _amount);
- totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

donateTokens() allows a malicious user to manipulate the system in such a way that users may receive 0 shares.

Support

FAQs

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