VaultRewardLib decreases a user's claimable reward amount even if the reward transfer fails
Summary
When a user claims rewards on VaultRewardLib, the contract accounting marks the amount as claimed even if the transfer failed, causing losses to claiming users
Vulnerability Detail
The VaultRewarderLib _claimRewardToken function wraps the reward token transfer transaction in a try/catch, so as to not fail the entire transaction which might disrupt vault operations (quoting from the function documentation: // Ignore transfer errors here so that any strange failures here do not prevent normal vault operations from working. Failures may include a lack of balances or some sort of blacklist that prevents an account from receiving tokens.). However, if the transfer transaction fails for some reason (such as temporary lack of balance or user blacklisting), the internal accounting of the reward claim still counts the reward as claimed. This is done in the following code (which is executed weather or not the transfer succeeds):
getAccountRewardDebt tracks the reward collected and reduces it from future claims. This means a user whose claim fails for reasons which may be temporary and expected to be resolved, loses the option to try and claim again later on when the problem is solved, effectively depriving them of their share of rewards. In addition, the non-claimed rewards remain stuck in the contract (which does not have a sweep option).
Root cause
Executing the reward accounting code regardless of weather or not the reward transfer succeeds.
Impact
Loss of rewards for users whose claim transactions fail temporarily.
In _claimRewardToken, only run the code that accounts for the claim if the transfer transaction succeeds, Allowing the user to try and claim again later.
nirohgo
Medium
VaultRewardLib decreases a user's claimable reward amount even if the reward transfer fails
Summary
When a user claims rewards on VaultRewardLib, the contract accounting marks the amount as claimed even if the transfer failed, causing losses to claiming users
Vulnerability Detail
The VaultRewarderLib _claimRewardToken function wraps the reward token transfer transaction in a try/catch, so as to not fail the entire transaction which might disrupt vault operations (quoting from the function documentation: // Ignore transfer errors here so that any strange failures here do not prevent normal vault operations from working. Failures may include a lack of balances or some sort of blacklist that prevents an account from receiving tokens.). However, if the transfer transaction fails for some reason (such as temporary lack of balance or user blacklisting), the internal accounting of the reward claim still counts the reward as claimed. This is done in the following code (which is executed weather or not the transfer succeeds):
getAccountRewardDebt tracks the reward collected and reduces it from future claims. This means a user whose claim fails for reasons which may be temporary and expected to be resolved, loses the option to try and claim again later on when the problem is solved, effectively depriving them of their share of rewards. In addition, the non-claimed rewards remain stuck in the contract (which does not have a sweep option).
Root cause
Executing the reward accounting code regardless of weather or not the reward transfer succeeds.
Impact
Loss of rewards for users whose claim transactions fail temporarily.
Code Snippet
https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/common/VaultRewarderLib.sol#L306
Tool used
Manual Review
Recommendation
In _claimRewardToken, only run the code that accounts for the claim if the transfer transaction succeeds, Allowing the user to try and claim again later.
Duplicate of #1