133.md

Proper Purple Toad

High

Broken functionality in SolidlyV2AMO::getReward

Summary

When passTokens is set to false, the contract collects reward for default reward token but does not calculate the default reward token's amount hence, does not send to rewards vault.

Root Cause

In getReward:328 there is no implementation for when passTokens is false

Internal pre-conditions

none

External pre-conditions

none

Attack Path

1. check balance of rewards vault
2. Call getRewards(), make sure all tokens passed are whitelisted
3. set passTokens to false
4. Check the difference of the balance before and after you'll notice that nothing has been added

Impact

Rewards of default token collected cannot be sent to rewards vault, in addition the loop will run over tokens with empty rewards and that will cause the contract to waste gas.

PoC

According to the comment @notice This function collects reward tokens from the gauge and transfers them to the reward vault, we know what the function does. However there is a bool passTokens parameter in the function declaration which is A boolean indicating whether to pass the token array to the external getReward() function

So the work of passTokens is to control whether the tokens[] is passed into the internally called getReward() function or not as we can see below.

When passTokens==true we can see that the rewards for all the tokens are collected and before sending to the rewards vault they are checked against protocol's whitelist. BUT on the other hand when passTokens==False the rewards for the tokens array are not collected but rather it is only the reward of the default reward token that is collected and when about to send to the reward's vault is where the issue is,

The contract does not check and send the rewards collected by the default reward's token but rather it loops over the empty tokens[] array that is passed into the function, has no rewards collected and tries to send 0 amounts to the rewards vault.

function getReward(
        address[] memory tokens,
        bool passTokens
    ) external override onlyRole(REWARD_COLLECTOR_ROLE) whenNotPaused nonReentrant {
        uint256[] memory rewardsAmounts = new uint256[](tokens.length);
        // Collect the rewards
        if (passTokens) {
            IGauge(gauge).getReward(address(this), tokens);
        } else {//when passTokens==false it collects rewards of default reward token
>>            IGauge(gauge).getReward();
        }
        //in the for loop we can see it only implements logic for when passTokens==True and does nothing when passTokens==False
>>        for (uint i = 0; i < tokens.length; i++) {
            if (!whitelistedRewardTokens[tokens[i]]) revert TokenNotWhitelisted(tokens[i]);
            rewardsAmounts[i] = IERC20Upgradeable(tokens[i]).balanceOf(address(this));
            IERC20Upgradeable(tokens[i]).safeTransfer(rewardVault, rewardsAmounts[i]);
        }
        // Emit an event for collecting rewards
        emit GetReward(tokens, rewardsAmounts);
    }

Mitigation

1. Implement logic to transfer default reward tokens collected to reward's vault
2. Avoid looping over empty arrays to reduce cost on gas