search

sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

xiaoming90 - All Reward Tokens Can Be Charged As Fee Due To Uncapped Fee #56

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

medium

All Reward Tokens Can Be Charged As Fee Due To Uncapped Fee

Summary

All reward tokens can be charged as fees due to uncapped fees.

Vulnerability Detail

Whenever some rewards are claimed, a certain percentage of the rewards will be sent to the FEE_RECEIVER as fees as shown in Line 78 below. It was understood from the sponsors that the FEE_RECEIVER is going to be set to Notional Treasury.

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:     }

Per the balancer environment file below, it was understood that the feePercentage is set to 1% in Line 49.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py

File: BalancerEnvironment.py
21: StrategyConfig = {
22:     "balancer2TokenStrats": {
23:         "StratStableETHstETH": {
24:             "vaultConfig": get_vault_config(
25:                 flags=set_flags(0, ENABLED=True, ALLOW_ROLL_POSITION=True),
26:                 currencyId=1,
27:                 minAccountBorrowSize=1,
28:                 maxBorrowMarketIndex=3,
29:                 secondaryBorrowCurrencies=[0,0]
30:             ),
31:             "secondaryBorrowCurrency": None,
32:             "maxPrimaryBorrowCapacity": 100_000_000e8,
33:             "name": "Balancer Stable ETH-stETH Strategy",
34:             "primaryCurrency": 1, # ETH
35:             "poolId": "0x32296969ef14eb0c6d29669c550d4a0449130230000200000000000000000080",
36:             "liquidityGauge": "0xcd4722b7c24c29e0413bdcd9e51404b4539d14ae",
37:             "auraRewardPool": "0xdcee1c640cc270121faf145f231fd8ff1d8d5cd4",
38:             "feeReceiver": "0x0190702d5e52e0269c9319144d3ad62a60ebe526",
39:             "maxUnderlyingSurplus": 100e18, # 10 ETH
40:             "oracleWindowInSeconds": 3600,
41:             "maxBalancerPoolShare": 2e3, # 20%
42:             "settlementSlippageLimitPercent": 5e6, # 5%
43:             "postMaturitySettlementSlippageLimitPercent": 10e6, # 10%
44:             "emergencySettlementSlippageLimitPercent": 10e6, # 10%
45:             "maxRewardTradeSlippageLimitPercent": 5e6,
46:             "balancerOracleWeight": 0.6e4, # 60%
47:             "settlementCoolDownInMinutes": 60 * 6, # 6 hour settlement cooldown
48:             "postMaturitySettlementCoolDownInMinutes": 60 * 6, # 6 hour settlement cooldown
49:             "feePercentage": 1e2, # 1%
50:             "settlementWindow": 3600 * 24 * 7,  # 1-week settlement
51:             "oraclePriceDeviationLimitPercent": 500, # +/- 5%
52:             "balancerPoolSlippageLimitPercent": 9900, # 1%
53:         },

However, per the require statement in Line 39 below, it is possible to set the feePercentage to 100%, thus allowing Notional to take away all the rewards accrued and leaving the vault shareholders with nothing.

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/BalancerVaultStorage.sol#L25

File: BalancerVaultStorage.sol
25:     function setStrategyVaultSettings(
26:         StrategyVaultSettings memory settings, 
27:         uint32 maxOracleQueryWindow,
28:         uint16 balancerOracleWeight
29:     ) internal {
30:         require(settings.oracleWindowInSeconds <= maxOracleQueryWindow);
31:         require(settings.settlementCoolDownInMinutes <= BalancerConstants.MAX_SETTLEMENT_COOLDOWN_IN_MINUTES);
32:         require(settings.postMaturitySettlementCoolDownInMinutes <= BalancerConstants.MAX_SETTLEMENT_COOLDOWN_IN_MINUTES);
33:         require(settings.maxRewardTradeSlippageLimitPercent <= BalancerConstants.SLIPPAGE_LIMIT_PRECISION);
34:         require(settings.balancerOracleWeight <= balancerOracleWeight);
35:         require(settings.maxBalancerPoolShare <= BalancerConstants.VAULT_PERCENT_BASIS);
36:         require(settings.settlementSlippageLimitPercent <= BalancerConstants.SLIPPAGE_LIMIT_PRECISION);
37:         require(settings.postMaturitySettlementSlippageLimitPercent <= BalancerConstants.SLIPPAGE_LIMIT_PRECISION);
38:         require(settings.emergencySettlementSlippageLimitPercent <= BalancerConstants.SLIPPAGE_LIMIT_PRECISION);
39:         require(settings.feePercentage <= BalancerConstants.VAULT_PERCENT_BASIS);
40:         require(settings.oraclePriceDeviationLimitPercent <= BalancerConstants.VAULT_PERCENT_BASIS);
41: 
42:         mapping(uint256 => StrategyVaultSettings) storage store = _settings();
43:         // Hardcode to the zero slot
44:         store[0] = settings;
45: 
46:         emit BalancerEvents.StrategyVaultSettingsUpdated(settings);
47:     }

Impact

Notional can take away all the rewards accrued and leave the vault shareholders with nothing. When this happens, the vault will not grow as it has nothing left to reinvest and the value of the vault share 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 https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/scripts/BalancerEnvironment.py https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/BalancerVaultStorage.sol#L25

Tool used

Manual Review

Recommendation

It is recommended to set an absolute cap on the maximum fee (e.g. 5%) that can be charged against the accrued rewards. This will give users more assurance and confidence about the security of their investment in the leverage vault.

Duplicate of #62