Liquid Staking

Summary

he issue lies in the order of operations within the updateFee function. The function first updates the fee at the given index with the new receiver and fee basis points, and then it checks if the total fees exceed the maximum allowed limit (MAX_BASIS_POINTS) using the _totalFeesBasisPoints function. If the total fees exceed the limit, the function reverts with the FeesExceedLimit error.

Vulnerability Details

updateFee function does not properly validate the total sum of fees after the update. It only checks if the total fees exceed the limit after the update, but it doesn't prevent the update from happening if the limit is exceeded.

This vulnerability can lead to a situation where the total fees exceed the maximum allowed limit (MAX_BASIS_POINTS), potentially causing unexpected behavior or breaking the contract's invariants. https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L151-L167

Validation Steps

1. The updateFee function is called with a specific index, receiver, and fee basis points.

2. The function updates the fee at the given index with the new receiver and fee basis points.

3. After the update, the function checks if the total fees exceed the MAX_BASIS_POINTS limit by calling the _totalFeesBasisPoints function.

4. If the total fees exceed the limit, the function reverts with the FeesExceedLimit error.

5. However, the update has already been performed before the check, allowing the total fees to exceed the limit.

The issue occurs because the validation of the total fees is performed after the update, rather than before. This allows the update to be executed even if it would result in the total fees exceeding the maximum allowed limit.

Can be exploited by following these steps:

1. Initialize the LSTRewardsSplitter contract with a set of fees that sum up to a value close to the MAX_BASIS_POINTS limit.

2. Call the updateFee function with an index of an existing fee and a new fee basis points value that, when added to the existing fees, exceeds the MAX_BASIS_POINTS limit.

3. Observe that the updateFee function allows the update to be executed, and the total fees exceed the limit.

Example scenario:

1. The contract is initialized with fees: [{ receiver: 0x123, basisPoints: 5000 }, { receiver: 0x456, basisPoints: 4000 }].

2. The updateFee function is called with index = 0, receiver = 0x789, and feeBasisPoints = 6000.

3. The function updates the fee at index 0 with the new receiver and fee basis points.

4. After the update, the total fees become 6000 + 4000 = 10000, which exceeds the MAX_BASIS_POINTS limit.

5. The function reverts with the FeesExceedLimit error, but the update has already been applied.

Impact

The fact that the fee is updated before validating the total fees. This lines directly contribute to the vulnerability are: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L162-L164

Tools Used

Manual Review

Recommendations

Validate the total sum of fees before performing the update. If the total fees after the update would exceed the MAX_BASIS_POINTS limit, the function should revert or prevent the update from happening.