sherlock-audit / 2023-10-notional-judging

5 stars 5 forks source link

Bauer - `reinvestReward()` will not function as intended #49

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

Bauer

high

reinvestReward() will not function as intended

Summary

The protocol initializes by retrieving tokens only from BALANCER_VAULT.getPoolTokens(), without specifying a reward token during initialization. However, in the reinvestReward() function, the protocol attempts to exchange the reward token for pool tokens, and there is a check that the reward token must be one of the tokens specified during initialization. Since the reward token is not specified during initialization, this check fails, preventing the execution of reinvestReward.

Vulnerability Detail

The reinvestReward() function is responsible for reinvesting rewards into the protocol.

  function reinvestReward(
        SingleSidedRewardTradeParams[] calldata trades,
        uint256 minPoolClaim
    ) external whenNotLocked onlyRole(REWARD_REINVESTMENT_ROLE) returns (
        address rewardToken,
        uint256 amountSold,
        uint256 poolClaimAmount
    ) {
        // Will revert if spot prices are not in line with the oracle values
        _checkPriceAndCalculateValue();

        // Require one trade per token, if we do not want to buy any tokens at a
        // given index then the amount should be set to zero. This applies to pool
        // tokens like in the ComposableStablePool.
        require(trades.length == NUM_TOKENS());
        uint256[] memory amounts;
        (rewardToken, amountSold, amounts) = _executeRewardTrades(trades);

The issue lies in the _isInvalidRewardToken() check within the _executeRewardTrades() function. This check ensures that the rewardToken used in the reward trading process is a valid reward token, i.e., one of the tokens specified during the protocol initialization.


    function _isInvalidRewardToken(address token) internal override view returns (bool) {
        return (
            token == TOKEN_1 ||
            token == TOKEN_2 ||
            token == TOKEN_3 ||
            token == TOKEN_4 ||
            token == TOKEN_5 ||
            token == address(AURA_BOOSTER) ||
            token == address(AURA_REWARD_POOL) ||
            token == address(Deployments.WETH)
        );
    }

However, the rewardToken obtained during the reinvestReward function execution is not part of the initially specified reward tokens, the _isInvalidRewardToken() check will fail.

As a result, the protocol won't be able to execute the reinvestReward() function successfully because the reward token obtained from the trades is not considered valid

Impact

The protocol won't be able to execute the reinvestReward() function successfully

Code Snippet

https://github.com/sherlock-audit/2023-10-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/AuraStakingMixin.sol#L38-L49 https://github.com/sherlock-audit/2023-10-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/mixins/BalancerPoolMixin.sol#L84-L102

Tool used

Manual Review

Recommendation

The recommended fix is to specify the reward token during initialization.

nevillehuang commented 11 months ago

Invalid, check working as intended based on comments here.

jeffywu commented 11 months ago

Agree, invalid. The reward tokens are dynamic an therefore we simply want to exclude any tokens that are not required to the functioning of the system.