Liquid Staking

Github

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

Summary

The LSTRewardsSplitter contract uses the OpenZeppelin Ownable library but introduces a critical issue by calling the internal function _transferOwnership directly in its constructor. This approach bypasses essential validations, increasing the risk of incorrect ownership assignment.

Vulnerability Details

The issue lies in the constructor of LSTRewardsSplitter, where _transferOwnership is called directly to set the contract's owner. While _transferOwnership changes ownership without validating the new owner’s address, the public transferOwnership function in OpenZeppelin’s Ownable contract offers additional safety by:

• Sanity Check: Ensuring the new owner is not the zero address.

• Access Control: Restricting usage to the current owner.

• Event Emission: Emitting OwnershipTransferred only after a validated transfer.

Impact

• If _transferOwnership is called with an incorrect or malicious address, the contract’s control can be lost or assigned to an invalid party without any warning.

• If set to the zero address (address(0)), the contract will become unusable since all onlyOwner functions will be inaccessible.

Tools Used

Manual Review

Recommendations

Replace the direct call to _transferOwnership with transferOwnership. This ensures the new owner’s address is validated before the transfer is finalized.