sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - Claim Rewards Can Fail Due To Transfer Failure #53

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

Claim Rewards Can Fail Due To Transfer Failure

Summary

When a transfer failure occurs, the claiming of rewards will fail. This causes the value of the vault shares to be stuck and not increase.

Vulnerability Detail

Note: This issue only affects MetaStable2 and Boosted3 balancer leverage vaults

The claimRewardTokens function will be triggered to claim the reward tokens from the Aura Reward Pool on behalf of the vault. The code will loop through all the reward tokens it receives and attempt to transfer a certain percentage of the accrued reward to FEE_RECEIVER as fees at Line 78 below. However, a single token transfer revert will cause the reward claim to fail.

Token transfers can fail due to various reasons:

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/AuraStakingMixin.sol#L61

File: AuraStakingMixin.sol
61:     function claimRewardTokens() external returns (uint256[] memory claimedBalances) {
62:         uint16 feePercentage = BalancerVaultStorage.getStrategyVaultSettings().feePercentage;
63:         IERC20[] memory rewardTokens = _rewardTokens();
64: 
65:         uint256 numRewardTokens = rewardTokens.length;
66: 
67:         claimedBalances = new uint256[](numRewardTokens);
68:         for (uint256 i; i < numRewardTokens; i++) {
69:             claimedBalances[i] = rewardTokens[i].balanceOf(address(this));
70:         }
71: 
72:         AURA_REWARD_POOL.getReward(address(this), true);
73:         for (uint256 i; i < numRewardTokens; i++) {
74:             claimedBalances[i] = rewardTokens[i].balanceOf(address(this)) - claimedBalances[i];
75: 
76:             if (claimedBalances[i] > 0 && feePercentage != 0 && FEE_RECEIVER != address(0)) {
77:                 uint256 feeAmount = claimedBalances[i] * feePercentage / BalancerConstants.VAULT_PERCENT_BASIS;
78:                 rewardTokens[i].checkTransfer(FEE_RECEIVER, feeAmount);
79:                 claimedBalances[i] -= feeAmount;
80:             }
81:         }
82: 
83:         emit BalancerEvents.ClaimedRewardTokens(rewardTokens, claimedBalances);
84:     }

Impact

Claiming rewards is one of the most critical components of Balancer vaults because the vault depends solely on claiming the rewards and selling them to obtain more BPT to increase the value of the vault shares. If reward claims stop working, the value of the vault shares will be stuck and will not increase.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/AuraStakingMixin.sol#L61

Tool used

Manual Review

Recommendation

It is recommended to adopt a withdrawal pattern where the fees are accumulated within the vault, and the FEE_RECEIVER can withdraw the accumulated fees from the vault at a later time.

Alternatively, use try-catch logic to filter out pausable error cases or implement partial reward claims if possible.