Liquid Staking

Summary

addFee function in the StakingPool contract has a vulnerability that allows the total fees to exceed the intended maximum limit of 40%. Due to the improper placement of the total fees check after adding the new fee to the fees array. The vulnerability can lead to excessive fees, reduced attractiveness for users, and potential for abuse by the contract owner.

Here's how and why it happens:

1. The addFee function takes a _receiver address and _feeBasisPoints amount as parameters.

2. It immediately pushes a new Fee struct with these values to the fees array using fees.push(Fee(_receiver, _feeBasisPoints)).

3. Only after adding the fee, it checks if the total fees basis points (including the newly added fee) exceed the maximum limit of 4000 (40%) using require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%").

4. If the total fees exceed 40% after adding the new fee, the transaction will revert at this point. However, the new fee has already been added to the fees array.

Vulnerability Details

The check to ensure that the total fees do not exceed the maximum limit of 40% (4000 basis points) is performed after the new fee has already been added to the array.

This improper placement of the check can lead to a scenario where the total fees exceed the intended limit, violating the expected behavior of the contract.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L347-L350

The issue arises because the require statement is placed after the fees.push operation. This means that the new fee is added to the fees array before checking if the total fees exceed the limit.

Burden of Proof

Consider the following scenario:

1. The StakingPool contract is deployed with initial fees that sum up to a value close to the maximum limit, e.g., 3900 basis points (39%).

2. The contract owner calls the addFee function with a _feeBasisPoints value that pushes the total fees above the 40% limit, e.g., 200 basis points (2%).

3. The addFee function adds the new fee to the fees array before checking the total fees against the limit.

4. The transaction succeeds, but the contract now has total fees exceeding the intended maximum of 40%.

Impact

Excessively high fees can make the staking pool less profitable and unappealing to users, potentially leading to decreased participation.

Tools Used

Vs

Recommendations

Modify the addFee function to perform the total fees check before adding the new fee to the fees array.