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

Fee Index Manipulation in updateFee Function Enables DoS and Integration Failures

cheatcode

Details

The updateFee function, responsible for modifying or removing existing fee structures, uses an unsafe method for removing fees. Instead of directly deleting the fee from the fees array, it swaps the fee to be removed with the last fee in the array and then uses pop() to remove the last element.

Code Snippet

function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < fees.length, "Fee does not exist");
​
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1]; // Swapping with last fee
fees.pop(); // Removing the last element
} else {
// ... (logic for updating the fee)
}
​
// ... (check for total fees exceeding the limit)
}

When a fee is removed, the code swaps its position with the last fee in the array. This alters the original order of fees and changes the indices associated with them.

This creates a situation where the indices of fees can change unexpectedly, especially when multiple fees are added and removed over time.

If any external systems or contracts rely on the assumption that fee indices remain stable, this behavior can break their integration with the LSTRewardsSplitter contract.

An attacker could exploit this to launch a Denial of Service (DoS) attack. By repeatedly adding and removing fees, they can cause the fees array to grow and shrink dynamically. This manipulation can significantly increase the gas cost of the _splitRewards function, potentially making it too expensive to execute and effectively halting the reward distribution process.

Impact

  • Broken Integrations: External systems or contracts relying on specific fee indices might fail to interact correctly with the contract, leading to unexpected behavior or errors.

  • Denial of Service: An attacker could prevent reward distribution by making the _splitRewards function computationally expensive through repeated fee manipulations.

  • Reward Manipulation: Even without a full DoS attack, the ability to manipulate fee indices could be used to subtly influence the reward distribution in favor of specific recipients.

Scenario

  • The contract is deployed with several fees, some of which are critical and accessed by external systems using their specific indices.

  • An attacker gains control of the owner account or exploits another vulnerability to gain access to the updateFee function.

  • The attacker repeatedly adds and removes fees, causing the fees array to grow and shrink, and the indices of the critical fees to change unpredictably.

  • External systems that rely on the original fee indices now fail to interact correctly with the contract, or the increased gas cost of _splitRewards makes it infeasible to execute.

Fix

To address this vulnerability, it's crucial to implement a more robust fee management system that doesn't rely on swapping and popping elements from the array. One approach is to use an isActive flag within the Fee struct:

struct Fee {
address receiver;
uint256 basisPoints;
bool isActive; // Add an isActive flag
}
​
mapping(uint256 => Fee) public fees; // Use a mapping instead of an array
uint256 public feeCount;
​
function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < feeCount, "Fee does not exist");
​
if (_feeBasisPoints == 0) {
fees[_index].isActive = false; // Deactivate the fee
} else {
// ... (logic for updating the fee)
fees[_index].isActive = true;
}
​
// ... (check for total fees exceeding the limit)
}
​
// Update _totalFeesBasisPoints to only consider active fees
function _totalFeesBasisPoints() private view returns (uint256) {
uint256 totalFees;
for (uint256 i = 0; i < feeCount; i++) {
if (fees[i].isActive) {
totalFees += fees[i].basisPoints;
}
}
return totalFees;
}

This approach ensures that fee indices remain stable even when fees are removed. It also allows for efficient calculation of total fees by considering only the active ones.

Updates

Lead Judging Commences

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.