Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

Zero address check required in StakingPool contract's `addStrategy` function

Summary

The addStrategy function in the StakingPool contract lacks a vaildation check for the zero address (address(0)) when adding new strategies. This oversight could potentially lead to irretrievable loss of user funds if exploited, despite the function being restricted to the contract owner.

NOTE: The absence of this check is particularly concerning given that similar address checks are implemented in other parts of the contract, such as the reorderStrategies function (which means this might have been an oversight).

Relevant links

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

Vulnerability Details

The addStrategy function allows the contract owner to add new strategies to the pool. However, it does not include a check to prevent the zero address (0x0) from being added as a strategy. The current implementation is as follows:

function addStrategy(address _strategy) external onlyOwner {
require(!_strategyExists(_strategy), "Strategy already exists");
token.safeApprove(_strategy, type(uint256).max);
strategies.push(_strategy);
}

The function only checks if the strategy already exists but fails to verify that the provided address is not the zero address. This could allow the zero address to be added as a valid strategy, potentially leading to an irretrievable loss of funds.

Like mentioned earlier, the absence of this check is a concern because address checks are in functions like reorderStrategies but not here which means this might have been an oversight.

Impact

The impact of this vulnerability could be severe:

  1. Loss of Funds: If the zero address is added as a strategy and funds are subsequently transferred to it, these funds would be lost forever, as they cannot be retrieved from the zero address.

  2. Contract Malfunction: Other parts of the contract that interact with strategies may assume all strategy addresses are valid, potentially leading to unexpected behavior or failures.

  3. Token Approval Risks: The function approves the strategy address to spend the maximum possible amount of tokens. If this approval is granted to the zero address, it could potentially be exploited in unforeseen ways.

While the function is restricted to the owner, a single mistake in address input could have severe consequences for user funds.

Tools Used

Manual review

Recommendations

Add a zero address check in the addStrategy function:

function addStrategy(address _strategy) external onlyOwner {
require(_strategy != address(0), "Strategy cannot be zero address");
require(!_strategyExists(_strategy), "Strategy already exists");
token.safeApprove(_strategy, type(uint256).max);
strategies.push(_strategy);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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