Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Unprotected `splitRewards` Function in `LSTRewardsSplitter` Allows Unauthorized Reward Distribution

Summary

The splitRewards function in the LSTRewardsSplitter contract is declared as external and lacks proper access control mechanisms.
This permits any external account to invoke the function, enabling them to trigger the reward splitting process at will.
As a result, the intended reward threshold mechanism can be bypassed, leading to premature and potentially unfair distribution of rewards, and undermining the integrity of the reward logic.

Vulnerability Details

The splitRewards function is defined without any access control modifiers such as onlyController or onlyOwner, making it accessible to anyone:

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));
}
}

This lack of restriction allows any user to call splitRewards, regardless of whether the reward threshold set by the controller has been met.
The reward threshold is intended to prevent the splitting of rewards until a certain amount of new rewards have accumulated.
By calling splitRewards directly, an attacker can:

  • Bypass the reward threshold: Triggering reward distribution before the minimum reward amount is reached.

  • Manipulate timing of rewards: Continuously call splitReward to distribute rewards in smaller, inefficient amounts.

  • Potentially disrupt reward calculations: Frequent splitting might affect the accuracy of reward allocations due to rounding errors or increased gas costs.

Furthermore, while the performUpkeep function includes a threshold check, it also lacks access control, allowing any user to call it. However, it will revert if the threshold is not met:

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

In contrast, splitRewards does not enforce the threshold, making it the primary target for exploitation.

Exploit Scenario

  • An attacker observes that the splitRewards function is unprotected.

  • The attacker calls splitRewards repeatedly, forcing the contract to distribute rewards prematurely.

  • This action disrupts the reward distribution schedule and could lead to unfair advantages or system instability.

Impact

  • Unauthorized Reward Distribution: Users can trigger reward splitting without authorization or regard for the threshold.

  • Disruption of Reward Logic: The intended mechanism to accumulate rewards before distribution is undermined.

  • Inefficient Operations: Frequent, unauthorized reward splits can lead to increased gas costs and potential performance issues.

  • Potential for Denial of Service: Excessive calls to splitRewards could congest the network or deplete contract funds due to gas consumption.

Tools Used

  • Manual code review

  • Slither

Recommendations

  • Implement Access Control: Add an access control modifier such as onlyController or onlyOwner to the splitRewards function to restrict its usage to authorized entities.

function splitRewards() external onlyController { // function body remains the same }
- function splitRewards() external {
+ function splitRewards() external onlyController {
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));
}
}
  • Consolidate Reward Functions: Consider merging performUpkeep and splitRewards into a single function that respects the reward threshold and includes proper access control

  • Restrict performUpkeep Access: Add appropriate access control to the performUpkeep function to prevent unauthorized calls.

  • Review Access Control Across Contract: Ensure all external functions that can alter contract state or impact core logic have proper access restrictions.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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