Relevant GitHub Links
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L388
Summary
Lack of inputs checks during initialization of the VaultControllerStrategy contract.
Vulnerability Details
No check was made on the inputs when the VaultControllerStrategy contract was initialized, which is very dangerous because any value can be stored.
Addresses such as _token, _stakingPool, _stakeController, _vaultImplementation and _vaultDepositController can be initialised to address(0) which is an invalid address. uint256 types such as _maxDepositSizeBP and _vaultMaxDeposits which represent some maximum values can be initialised to zero which makes no sense and will lead to unexpected behaviour. And an array such as _fees can be initialised with an empty array, which means that no fees will be applied during user transactions even though the protocol allows for fees. So if these variables take on invalid values, this will affect the way the protocol works:
function __VaultControllerStrategy_init(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits,
address _vaultDepositController
) public onlyInitializing {
__Strategy_init(_token, _stakingPool);
stakeController = IStaking(_stakeController);
vaultImplementation = _vaultImplementation;
for (uint256 i = 0; i < _fees.length; ++i) {
fees.push(_fees[i]);
}
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
if (_maxDepositSizeBP > 10000) revert InvalidBasisPoints();
maxDepositSizeBP = _maxDepositSizeBP;
vaultMaxDeposits = _vaultMaxDeposits;
vaultDepositController = _vaultDepositController;
}
Impact
Unexpected Behavior leading to potential errors, attacks or state inconsistencies
Tools Used
Manual analysis.
Recommendations
function __VaultControllerStrategy_init(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits,
address _vaultDepositController
) public onlyInitializing {
+ require( _token != address(0), "Invalid token address!");
+ require( _stakingPool != address(0), "Invalid stakingPool address!");
+ require( _stakeController != address(0), "Invalid stakeController address!");
+ require( _vaultImplementation != address(0), "Invalid vaultImplementation address!");
+ require( _vaultDepositController != address(0), "Invalid vaultDepositController address!");
+ require( _maxDepositSizeBP != 0, "Invalid maxDepositSizeBP value!");
+ require( _vaultMaxDeposits != 0, "Invalid vaultMaxDeposits value!");
+ require( _fees.length != 0, "Empty array not accepted!");
__Strategy_init(_token, _stakingPool);
stakeController = IStaking(_stakeController);
vaultImplementation = _vaultImplementation;
for (uint256 i = 0; i < _fees.length; ++i) {
+ require(_fees[i] !=0, "Invalid value!");
fees.push(_fees[i]);
}
if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge();
if (_maxDepositSizeBP > 10000) revert InvalidBasisPoints();
maxDepositSizeBP = _maxDepositSizeBP;
vaultMaxDeposits = _vaultMaxDeposits;
vaultDepositController = _vaultDepositController;
}
