Liquid Staking

Summary

addFee function in the LSTRewardsSplitter contract contains a vulnerability that allows for the addition of fees that cause the total fees to exceed the maximum limit of 10000 basis points.

The issue is that the check is performed after the fee is already added to the fees array. Even if the total fees exceed the limit, the new fee will still be added to the array before the revert occurs.

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

The line fees.push(Fee(_receiver, _feeBasisPoints)); adds the new fee to the fees array before checking if the total fees exceed the limit.

Reproduction Steps:

1. Call addFee with a _feeBasisPoints value that causes the total fees to exceed 10000 basis points.

2. The addFee function will add the fee and then revert due to the FeesExceedLimit error, but the fee will still be added to the fees array.

Vulnerability Details

The function first adds the new fee to the fees array using fees.push(Fee(_receiver, _feeBasisPoints));. Only after adding the fee, it checks if the total fees exceed the maximum limit using if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();.

This sequence of operations allows for a scenario where a new fee is added, causing the total fees to exceed the limit, but the fee is still successfully added to the fees array before the revert occurs. Because of that, the fees array can contain fees that collectively surpass the intended maximum limit.

Scenario:

1. The LSTRewardsSplitter contract is deployed with an initial set of fees that do not exceed the maximum limit.

2. The addFee function is called with a _feeBasisPoints value that causes the total fees to exceed 10000 basis points. For example:

   • Existing fees: [{receiver: 0x1, basisPoints: 5000}, {receiver: 0x2, basisPoints: 4000}]

   • New fee: {receiver: 0x3, basisPoints: 2000}

3. The addFee function adds the new fee to the fees array, resulting in a total of 11000 basis points.

4. The function then reverts with the FeesExceedLimit error, but the new fee remains in the fees array.

This sequence of steps proves that the addFee function allows for the addition of fees that violate the maximum limit, leading to an inconsistent state of the fees array.

Impact

Users interacting with the contract may expect the total fees to always be within the maximum limit. However, due to the bug, the contract can contain fees that exceed the limit, leading to unexpected behavior and potential confusion.

Tools Used

Vs Code

Recommendations

By checking _totalFeesBasisPoints() + _feeBasisPoints > 10000 before adding the new fee, the function ensures that the total fees will not exceed the maximum limit. If the addition of the new fee would cause the total fees to surpass the limit, the function reverts preemptively, preventing the inconsistent state.