ZeroTrust - In `updateRewardToken()` function missing revoking tokens permissions in the TradingModule

[sherlock-admin2]

ZeroTrust

Medium

In updateRewardToken() function missing revoking tokens permissions in the TradingModule

Summary

In updateRewardToken() function missing revoking tokens permissions in the TradingModule

Vulnerability Detail

The developer mentions in the README that if an incentive token is whitelisted in VaultRewarderLib::updateRewardToken, this bot should be explicitly prevented from selling those tokens by revoking its permissions in the TradingModule. (This check is performed in the updateRewardToken method).

However, in updateRewardToken(), it only checks if the token is not in Deployments.TRADING_MODULE.tokenWhitelist, rather than implementing what was stated. If an incentive token is not explicitly whitelisted by updateRewardToken, then it will be in Automatic Reinvest mode. This has not been implemented in the code. Additionally, there is no code to prevent the bot from selling those tokens by revoking its permissions in the TradingModule.

 function updateRewardToken(
        uint256 index,
        address rewardToken,
        uint128 emissionRatePerYear,
        uint32 endTime
    ) external override {
        require(msg.sender == Deployments.NOTIONAL.owner());
        // Check that token permissions are not set for selling so that automatic reinvest
        // does not sell the tokens
        (bool allowSell, /* */, /* */) = Deployments.TRADING_MODULE.tokenWhitelist(
            address(this), rewardToken
        );
@>        require(allowSell == false);
        uint256 totalVaultSharesBefore = VaultStorage.getStrategyVaultState().totalVaultSharesGlobal;

        StrategyVaultSettings memory settings = VaultStorage.getStrategyVaultSettings();
        VaultRewardState memory state = VaultStorage.getVaultRewardState()[index];

        if (index < settings.numRewardTokens) {
            // Safety check to ensure that the correct token is specified, we can never change the
            // token address once set.
            require(state.rewardToken == rewardToken);
            // Modifies the emission rate on an existing token, direct claims of the token will
            // not be affected by the emission rate.
            // First accumulate under the old regime up to the current time. Even if the previous
            // emissionRatePerYear is zero this will still set the lastAccumulatedTime to the current
            // blockTime.
            _accumulateSecondaryRewardViaEmissionRate(index, state, totalVaultSharesBefore);

            // Save the new emission rates 修改原来的奖励配置
            state.emissionRatePerYear = emissionRatePerYear;
            if (state.emissionRatePerYear == 0) {
                state.endTime = 0;
            } else {
                require(block.timestamp < endTime);
                state.endTime = endTime;
            }
            VaultStorage.getVaultRewardState()[index] = state;
        } else if (index == settings.numRewardTokens) {//新增奖励配置
            // This sets a new reward token, ensure that the current slot is empty
            require(state.rewardToken == address(0));
            settings.numRewardTokens += 1;
            VaultStorage.setStrategyVaultSettings(settings);//update settings
            state.rewardToken = rewardToken;

            // If no emission rate is set then governance is just adding a token that can be claimed
            // via the LP tokens without an emission rate. These settings will be left empty and the
            // subsequent _claimVaultRewards method will set the initial accumulatedRewardPerVaultShare.
            if (0 < emissionRatePerYear) {
                state.emissionRatePerYear = emissionRatePerYear;
                require(block.timestamp < endTime);
                state.endTime = endTime;
                state.lastAccumulatedTime = uint32(block.timestamp);
            }
            VaultStorage.getVaultRewardState()[index] = state;
        } else {
            // Can only append or modify existing tokens
            revert();
        }

        // Claim all vault rewards up to the current time
        (VaultRewardState[] memory allStates, /* */, RewardPoolStorage memory rewardPool) = getRewardSettings();
        _claimVaultRewards(totalVaultSharesBefore, allStates, rewardPool);
        emit VaultRewardUpdate(rewardToken, emissionRatePerYear, endTime);
    }

Impact

This can easily lead to configuration confusion, resulting in rewards being incorrectly reinvested.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/common/VaultRewarderLib.sol#L68

Tool used

Manual Review

Recommendation

Add functionality to remove permissions from Deployments.TRADING_MODULE.tokenWhitelist.

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Low/QA at most on admin controlled call