Summary

The check for the total fees exceeding the limit is performed after the new fee is already added to the fees array. This means that if the new fee causes the total to exceed the limit, the revert will happen, but the invalid fee will still be present in the fees array.

Vulnerability Details

The addFee function allows adding a new fee that can cause the total fees to exceed the maximum limit of 10000 basis points (100%). The check for the total fees exceeding the limit is performed after the new fee is already added to the fees array. If the new fee causes the total to exceed the limit, the function will revert with the FeesExceedLimit error, but the invalid fee will still be present in the fees array.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L140-L143

Imagine this Steps

1. The addFee function takes two parameters: _receiver (the address of the fee receiver) and _feeBasisPoints (the fee amount in basis points).

2. Inside the function, the new fee is immediately added to the fees array using fees.push(Fee(_receiver, _feeBasisPoints)).

3. After adding the new fee, the function checks if the total fees exceed the limit of 10000 basis points by calling the _totalFeesBasisPoints() function.

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

5. However, the invalid fee that caused the total to exceed the limit is still present in the fees array.

The issue occurs because the check for the total fees exceeding the limit is performed after the new fee is already added to the fees array. This allows the fees array to contain invalid fees that cause the total to exceed the limit, even though the function reverts.

PoC

Can be exploited by calling the addFee function with a _feeBasisPoints value that, when added to the existing total fees, exceeds 10000 basis points. Here's a scenario demonstrating the issue:

1. Assume the current total fees in the fees array sum up to 9000 basis points.

2. An attacker calls the addFee function with _receiver set to their own address and _feeBasisPoints set to 2000.

3. The new fee is added to the fees array, making the total fees 11000 basis points.

4. The function checks if the total fees exceed the limit and reverts with the FeesExceedLimit error.

5. However, the invalid fee (2000 basis points) is still present in the fees array.

To perform the total fees limit check after adding the new fee to the fees array, It allows the fees array to contain invalid fees that violate the limit, even though the function reverts.

Impact

If the fees array contains invalid fees that cause the total to exceed the limit, it can affect the fee distribution process. The _splitRewards function, which is responsible for splitting the rewards among the fee receivers, may distribute the rewards based on the invalid fees. This can result in incorrect allocation of rewards to the fee receivers.

Tools Used

Manual Review

Recommendations

The check for the total fees exceeding the limit should be performed before adding the new fee to the fees array.