Liquid Staking

Summary

The smart contract in question suffers from a reentrancy vulnerability in the _updateStrategyRewards function.(in stakingPool.sol) The vulnerability arises from the use of the transferAndCallFrom function, which implements ERC677 token transfers, allowing receivers to execute a callback function upon receiving tokens. Malicious actors can exploit this callback mechanism to perform a reentrancy attack, leading to an uneven distribution of rewards and fees across multiple fee receivers. This issue can result in one or more attackers receiving a disproportionately large amount of shared tokens.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L592

Vulnerability Details

• The _updateStrategyRewards function uses transferAndCallFrom to distribute fees to fee receivers. This function allows the receiving contract (if implemented) to execute a callback function during the transfer, as per the ERC677 standard.

• Reentrancy Vector: If a fee receiver is a malicious contract, the callback function can reenter the contract and manipulate the state by donating tokens (donateTokens) or burning shared tokens (burn), increases totalStaked or reduces totalShares.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L423-L426

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L433-L437

• This reentrancy attack affects the calculation of shares and rewards for other fee receivers. The calculation in getSharesByStake depends on the totalStaked value, which is modified by the reentrant donation or burning. This distorts the reward calculation for subsequent receivers. (Reduced totalShares or increased totalStaked reduces amount of fee shared tokens)

• Second Attacker: Another malicious receiver can exploit the altered state (i.e., reduced totalStaked) to receive more shared tokens than expected. Since getSharesByStake calculates rewards based on the manipulated totalStaked, the last fee receiver can receive the remaining fee amount, leading to an unfair distribution of tokens.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L585-L590

The rest shared[_account] is greater than expected because victim receivers received less fees than expected.

As result, two attackers stole more shared Tokens than expected and victim receivers received less fee shared Tokens.

Impact

• Manipulated Token Distribution: Attackers can manipulate the distribution of fees and rewards, causing other legitimate fee receivers to receive fewer tokens than expected.

• Disproportionate Fee Allocation: Attackers executing reentrant attacks can donate them during the fee distribution, leading to an inflated share of fees for the last receiver, who could be another attacker.

• Financial Loss: The imbalance in reward distribution could result in a significant financial loss for fee receivers who are not part of the attack, as attackers walk away with a larger share of the pool.

** Attacker donate tokens, so attacker loose his tokens at that time but intead of that, he can get more shared Tokens. This is staking, so totalStaked will be increased and this will give him more and more amount of tokens because of shared tokens stolen.**

Tools Used

Recommendations

1. Implement Reentrancy Guard: Add a reentrancy guard (such as OpenZeppelin's nonReentrant modifier) to functions that update critical state variables like _updateStrategyRewards and others that handle token transfers (e.g., donateTokens, burn).

2. Avoid Using ERC677 Callback: Avoid using the transferAndCallFrom mechanism if not strictly necessary, especially in functions that distribute rewards or fees. Consider replacing it with a simpler ERC20 transfer call, which does not involve any callbacks.