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

Insufficient Validation of Input Parameters

mustapha

Summary

The contract does not adequately validate input parameters in various functions, which can lead to unintended consequences, including fund loss or incorrect contract behavior.

Vulnerability Details

In the context of the LSTRewardsSplitter contract, certain functions accept input parameters that could benefit from additional validation. Here are specific areas of concern

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

Withdraw Function:

function withdraw(uint256 _amount, address _receiver) external onlyController {
principalDeposits -= _amount;
lst.safeTransfer(_receiver, _amount);
emit Withdraw(_amount);
}

No Balance Check: The function does not check if the _amount to withdraw exceeds the available balance of the contract. This can lead to a situation where principalDeposits becomes negative, which is not desirable.

Zero Address Check: The _receiver address is not checked to ensure it is not the zero address. Transferring tokens to the zero address would result in those tokens being permanently lost.

Add Fee Function

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

current implementation

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
fees.push(Fee(_receiver, _feeBasisPoints));
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}

Zero Address Check: The _receiver parameter is not validated to ensure it is a valid address. If a zero address is added as a fee receiver, it could lead to unintended consequences when attempting to transfer fees.

Update Fee Function

Current Implementation:

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

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];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
​
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
​

No Zero Address Check: Similar to the addFee function, the _receiver parameter is not checked, potentially allowing for a zero address to be assigned as a fee receiver.

Index Validation: While there is a check to ensure the index is within the bounds of the fees array, more comprehensive error handling could improve clarity and robustness.

Impact

Loss of Funds: If a user attempts to withdraw more tokens than available, the state could become inconsistent, allowing for erroneous behavior that could lead to loss of funds.

Permanent Loss of Tokens: Sending tokens to a zero address results in those tokens being irretrievable, effectively reducing the total balance available to users.

Decreased User Trust: Insufficient validation could lead to user frustration and loss of trust in the contract's reliability.

Tools Used

Recommendations

To mitigate this vulnerability, consider implementing the following checks:

  1. Withdraw Function:

    Validate that the _amount to withdraw does not exceed the current balance of the contract:

function withdraw(uint256 _amount, address _receiver) external onlyController {
require(_amount <= lst.balanceOf(address(this)), "Insufficient balance");
require(_receiver != address(0), "Invalid receiver address");
principalDeposits -= _amount;
lst.safeTransfer(_receiver, _amount);
emit Withdraw(_amount);
}
​

Add Fee Function

Check that the _receiver address is not the zero address:

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
require(_receiver != address(0), "Invalid receiver address");
fees.push(Fee(_receiver, _feeBasisPoints));
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
​

Update Fee Function:

Similar checks can be added for the _receiver parameter:

function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < fees.length, "Fee does not exist");
require(_receiver != address(0), "Invalid receiver address");
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.