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

Potential Griefing Attack in `OperatorVault::updateDeposits` can lead to loss of funds

Joseph

Summary

In the OperatorVault::updateDeposits the use of an arbitrary, unvalidated _rewardsReceiver to whom rewards will be sent to is what poses the risk of a possible griefing attack, despite the presence of an initialized rewardsReceiver as a state variable.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorVault.sol#L180C1-L207C6

// /contracts/linkStaking/OperatorVault.sol
// #180-207
​
​
function updateDeposits(
uint256 _minRewards,
address _rewardsReceiver <@audit
) external onlyVaultController returns (uint256, uint256, uint256) {
uint256 principal = getPrincipalDeposits();
uint256 rewards = getRewards();
uint256 totalDeposits = principal + rewards;
int256 depositChange = int256(totalDeposits) - int256(uint256(trackedTotalDeposits));
​
uint256 opRewards;
if (depositChange > 0) {
opRewards =
(uint256(depositChange) *
IOperatorVCS(vaultController).operatorRewardPercentage()) /
10000;
unclaimedRewards += SafeCast.toUint128(opRewards);
trackedTotalDeposits = SafeCast.toUint128(totalDeposits);
}
​
if (_minRewards != 0 && rewards >= _minRewards) {
rewardsController.claimReward();
trackedTotalDeposits -= SafeCast.toUint128(rewards);
totalDeposits -= rewards;
token.safeTransfer(_rewardsReceiver, rewards);
}
​
return (totalDeposits, principal, opRewards);
}
contract OperatorVault is Vault {
using SafeERC20Upgradeable for IERC20Upgradeable;
​
address public operator;
address public rewardsReceiver; <@audit
IPFAlertsController public pfAlertsController;
​
uint128 public trackedTotalDeposits;
uint128 private unclaimedRewards;
//SNIPPED

Vulnerability Details

An arbitrary, user-defined validated _rewardsReceiver is used to receive rewards in the OperatorVault::updateDeposits. The problem is five fold

  • There's no check to see if it matches the already set rewardsReceiver from OperatorVault::rewardsReceiver.

  • Arbitrary user input can be manipulated if OperatorVault::VaultController is compromised or turns malicious.

  • Can be possibly set to address(0) thereby forever losing funds.

  • Gives room to malicious activity by protocol, think Rug Pull. Besides the operation lacked events too.

  • Why use arbitrary address when there's an initialized address set by the owner during initialization or OperatorVault::setRewardsReceiver

Impact

Risk of a possible griefing attack can lead to loss of funds.

Tools Used

Manual Review.

Recommendations

Use the stored OperatorVault::rewardsReceiver value only.

Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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