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

Staking Pool can be bricked by an attacker immediately it is deployed.

bigsam

Summary

An attacker can brick the staking pool by calling the `donateTokens` function immediately after deployment. This action would result in a scenario where `totalShares = 0` and `totalStaked > 0`, preventing any subsequent normal deposits from being processed. All future attempts to deposit from the priority pool will fail due to a reversion in the minting process, effectively disabling the staking pool for all users.

Vulnerability Details

The vulnerability occurs due to the following flow:

1. The attacker calls the `donateTokens` function immediately after the staking pool is deployed. This increases `totalStaked` but does not mint any shares (because donations do not mint new shares), leaving `totalShares = 0`.

/**
* @notice Deposits asset tokens into the pool without minting liquid staking tokens,
* effectively donating them to the pool
* @param _amount amount to deposit
**/
audit>> 1. >> function donateTokens(uint256 _amount) external {
token.safeTransferFrom(msg.sender, address(this), _amount);
totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}

2. Subsequent deposit attempts from the priority pool revert when minting new shares. Since `totalShares = 0`, the function `getSharesByStake` will return 0 shares for any deposit amount, causing the `_mintShares` function to revert due to an underflow.

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);
audit>> 2. >> _mint(_account, _amount);
totalStaked += _amount;
} else {
_depositLiquidity(_data);
}
​
uint256 endingBalance = token.balanceOf(address(this));
if (endingBalance > startingBalance && endingBalance > unusedDepositLimit)
revert InvalidDeposit();
}
​

Next

function _mint(address _recipient, uint256 _amount) internal override {
audit>> 3. >> uint256 sharesToMint = getSharesByStake(_amount);
_mintShares(_recipient, sharesToMint);
​
emit Transfer(address(0), _recipient, _amount);
}
​

- The calculation for shares in `getSharesByStake` will return 0 because the formula relies on `totalShares`:

function getSharesByStake(uint256 _amount) public view returns (uint256) {
uint256 totalStaked = _totalStaked();
if (totalStaked == 0) {
return _amount;
} else {
audit>> 4. >> return (_amount * totalShares) / totalStaked;
}
}
​

- When attempting to mint shares in `_mintShares`, the system attempts to subtract `DEAD_SHARES` from the mint amount, which results in an underflow:

function _mint(address _recipient, uint256 _amount) internal override {
uint256 sharesToMint = getSharesByStake(_amount);
audit>> 5. 0 sharestomint >> _mintShares(_recipient, sharesToMint);
​
emit Transfer(address(0), _recipient, _amount);
}
​

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

3. The underflow occurs when `_amount - DEAD_SHARES` results in a negative value (e.g., 0 - 1000). This reverts the transaction, preventing any future deposits from being accepted by the staking pool.

4. As a result, the staking pool becomes permanently bricked, and no normal deposits can be made, rendering the system unusable for all users.

Impact

- **Complete Denial-of-Service (DoS) Attack**: The staking pool becomes permanently bricked, preventing any future deposits from the priority pool. This renders the staking pool unusable for all users and disrupts the entire staking process.

- **Unrecoverable State**: Since the system cannot handle deposits after `totalShares = 0` and `totalStaked > 0`, this leads to a state where normal operations cannot resume without manual intervention or redeployment.

Tools Used

Manual Review

Recommendations

1. **Revert `donateTokens` if `totalStaked == 0`**: To prevent the bricking of the staking pool by an initial donation, the contract should revert any `donateTokens` call if `totalStaked == 0`. This ensures that the staking process begins with a valid deposit, ensuring shares are minted correctly from the start.

​
function donateTokens(uint256 _amount) external {
++ require(totalStaked != 0, "Cannot donate tokens before staking begins");
token.safeTransferFrom(msg.sender, address(this), _amount);
totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months 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.