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

CommunityVault::claimRewards claims more token rewards than getRewards() allows it to

JanuaryPersimmon2024

Summary

The CommunityVault::claimRewards function is intended to claim the yielded rewards from the Chainlink staking program, but instead it transfers the full balance of tokenheld by CommunityVault.

Vulnerability Details

/**
* @notice Claims rewards from the Chainlink staking contract
* @param _minRewards min amount of rewards required to claim
* @param _rewardsReceiver address to receive rewards
**/
function claimRewards(
uint256 _minRewards,
address _rewardsReceiver
) external onlyVaultController {
uint256 rewards = getRewards();
if (rewards != 0 && rewards >= _minRewards) {
rewardsController.claimReward();
token.safeTransfer(_rewardsReceiver, token.balanceOf(address(this)));
}
}

https://github.com/Cyfrin/2024-09-stakelink/blob/ea5574ebce3a86d10adc2e1a5f6d5512750f7a72/contracts/linkStaking/CommunityVault.sol#L34C1-L48C6

Impact

As you can see, it is not the getRewards rewardsamount that is transferred to the _rewardsReceiver, but instead the whole tokenbalance held in the contract.

The getRewardsfunction is inherited from the parent Vaultcontract, and it determines how many rewards have accumulated thanks to the Chainlink restaking program:

/**
* @notice Returns the claimable rewards balance of this contract in the Chainlink staking rewards contract
* @return rewards balance
*/
function getRewards() public view returns (uint256) {
return rewardsController.getReward(address(this));
}

https://github.com/Cyfrin/2024-09-stakelink/blob/ea5574ebce3a86d10adc2e1a5f6d5512750f7a72/contracts/linkStaking/base/Vault.sol#L105C1-L111C6

That means that when the vault controller calls claimRewards, he will withdraw not only the rewardsamount that the vault is eligible for, but the full tokenamount held by the CommunityVaultcontract, therefore withdrawing in excess.

Tools Used

Manual review.

Recommendations

Consider only transferring the rewardsamount that is intended to represent the real claimable amount of token, instead of transferring the full balance of the address(this)contract, by changing the code of the claimRewardsfunction accordingly:

/**
* @notice Claims rewards from the Chainlink staking contract
* @param _minRewards min amount of rewards required to claim
* @param _rewardsReceiver address to receive rewards
**/
function claimRewards(
uint256 _minRewards,
address _rewardsReceiver
) external onlyVaultController {
uint256 rewards = getRewards();
if (rewards != 0 && rewards >= _minRewards) {
rewardsController.claimReward();
- token.safeTransfer(_rewardsReceiver, token.balanceOf(address(this)));
+ token.safeTransfer(_rewardsReceiver, rewards);
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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