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

Reentrancy Vulnerability in `splitRewards` Function

chainsentryaudits

Summary

A critical reentrancy vulnerability exists in the splitRewards function of the LSTRewardsSplitter.sol contract. This flaw allows malicious fee receivers to exploit the contract by re-entering the splitRewards function during external calls, leading to unauthorized manipulation of rewards distribution and potential draining of funds.

Vulnerability Details

External Calls Before State Updates

function _splitRewards(uint256 _rewardsAmount) private {
for (uint256 i = 0; i < fees.length; ++i) {
Fee memory fee = fees[i];
uint256 amount = (_rewardsAmount * fee.basisPoints) / 10000;
​
if (fee.receiver == address(lst)) {
IStakingPool(address(lst)).burn(amount);
} else {
lst.safeTransfer(fee.receiver, amount);
}
}
​
principalDeposits = lst.balanceOf(address(this));
emit RewardsSplit(_rewardsAmount);
}

Explanation:
The _splitRewards function iterates through all fee receivers and performs external calls (lst.safeTransfer or IStakingPool(address(lst)).burn) before updating the critical state variable principalDeposits. This sequence violates the Checks-Effects-Interactions pattern, leaving the contract vulnerable to reentrancy attacks.

Reentrancy Through Malicious Fee Receiver

contract MaliciousFeeReceiver {
LSTRewardsSplitter public splitter;
​
constructor(address _splitter) {
splitter = LSTRewardsSplitter(_splitter);
}
​
function onTokenTransfer(address, uint256, bytes calldata) external {
// Re-enter the splitRewards function
splitter.splitRewards();
}
}

Explanation:
A malicious contract like MaliciousFeeReceiver can be set as a fee receiver. Upon receiving tokens, its onTokenTransfer function is triggered, which calls back into the vulnerable splitRewards function before principalDeposits is updated. This reentrancy allows the attacker to manipulate the rewards distribution.

Inconsistent State During Reentrancy

function splitRewards() external {
int256 newRewards = int256(lst.balanceOf(address(this))) - int256(principalDeposits);
if (newRewards < 0) {
principalDeposits -= uint256(-1 * newRewards);
} else if (newRewards == 0) {
revert InsufficientRewards();
} else {
_splitRewards(uint256(newRewards));
}
}

Explanation:
When splitRewards is called, it calculates newRewards based on the current lst.balanceOf and principalDeposits. However, if a reentrant call occurs within _splitRewards, principalDeposits has not yet been updated, leading to inaccurate calculations and potential exploitation of the rewards mechanism.

Impact

An attacker controlling a fee receiver can:

  • Re-enter the splitRewards Function: By triggering reentrancy during external calls, the attacker can call splitRewards multiple times before principalDeposits is updated.

  • Manipulate Rewards Calculation: Repeated reentrant calls can allow the attacker to receive more rewards than intended, effectively draining funds from the contract.

  • Disrupt Reward Distribution: The inconsistent state can lead to incorrect reward distributions, undermining the integrity and reliability of the staking platform.

Potential Consequences:

  • Financial Losses: Significant funds could be siphoned off, impacting all stakers and the overall platform's financial health.

  • Erosion of Trust: Such vulnerabilities can erode user trust, leading to decreased participation and potential abandonment of the platform.

Tools Used

  • Manual Review

Recommendations

Implement Reentrancy Guards

Solution:
Integrate OpenZeppelin's ReentrancyGuard to prevent reentrant calls.

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract LSTRewardsSplitter is Ownable, ReentrancyGuard {
// ...
​
function splitRewards() external nonReentrant {
_splitRewards(/* parameters */);
}
​
// ...
}

Explanation:
The nonReentrant modifier ensures that the splitRewards function cannot be called while it is already in execution, effectively blocking reentrancy attempts.

Apply Checks-Effects-Interactions Pattern

Solution:
Rearrange the _splitRewards function to update state variables before making external calls.

function _splitRewards(uint256 _rewardsAmount) private {
// Update state first
principalDeposits = lst.balanceOf(address(this));
​
// Emit event before external calls
emit RewardsSplit(_rewardsAmount);
​
// Perform external calls
for (uint256 i = 0; i < fees.length; ++i) {
Fee memory fee = fees[i];
uint256 amount = (_rewardsAmount * fee.basisPoints) / 10000;
​
if (fee.receiver == address(lst)) {
IStakingPool(address(lst)).burn(amount);
} else {
lst.safeTransfer(fee.receiver, amount);
}
}
}

Explanation:
By updating principalDeposits before any external interactions, the contract ensures that its state remains consistent, mitigating the risk of reentrancy attacks.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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