csanuragjain - Duplicate reward can lead to deduction of extra fees than required

[sherlock-admin]

csanuragjain

medium

Duplicate reward can lead to deduction of extra fees than required

Summary

In AuraStakingMixin contract, if _rewardTokens contains duplicate token then fees will be deducted twice instead of deducting it once.

Vulnerability Detail

1. Observe the _rewardTokens method

function _rewardTokens() private view returns (IERC20[] memory tokens) {
        uint256 rewardTokenCount = LIQUIDITY_GAUGE.reward_count() + 2;
        tokens = new IERC20[](rewardTokenCount);
        tokens[0] = BAL_TOKEN;
        tokens[1] = AURA_TOKEN;
        for (uint256 i = 2; i < rewardTokenCount; i++) {
            tokens[i] = IERC20(LIQUIDITY_GAUGE.reward_tokens(i - 2));
        }
    }

2. As we can see 2 tokens BAL and AURA are hardcoded and rest tokens are coming from LIQUIDITY_GAUGE.reward_tokens
3. The problem arises if LIQUIDITY_GAUGE.reward_tokens has one of the token as BAL or AURA.
4. In the case of duplicate token claimRewardTokens will deduct fees twice for each instance of copy leading to giving excess fees than required

for (uint256 i; i < numRewardTokens; i++) {
            claimedBalances[i] = rewardTokens[i].balanceOf(address(this)) - claimedBalances[i];

            if (claimedBalances[i] > 0 && feePercentage != 0 && FEE_RECEIVER != address(0)) {
                uint256 feeAmount = claimedBalances[i] * feePercentage / BalancerConstants.VAULT_PERCENT_BASIS;
                rewardTokens[i].checkTransfer(FEE_RECEIVER, feeAmount);
                claimedBalances[i] -= feeAmount;
            }
        }

Impact

More fees than required will be deducted reducing the claimable balance

Code Snippet

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

Tool used

Manual Review

Recommendation

Revise the _rewardTokens method to remove all duplicate tokens

[jeffywu]

@weitianjie2000

This seems like a good safe guard, however, it's not clear to me that reward tokens actually can be duplicated on Liquidity Gauges. Also, adding a de-duplication check would add a decent amount of additional code to reduce a risk that has limited impact and is not clear actually exists.

I would decrease this to low severity since the fee transfer is proportional and also goes to a Notional protocol controlled address.