code423n4 commented 2 years ago

Handle

Tomio

Vulnerability details

Impact

When adding a reward a user can call addRewards() which is callable by anyone, to prevent executing more gas than it should, it's better to stop the execution when extraRewardsLength on convexpool is zero, this can save some gas because it didn't have to execute https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexStakingWrapper.sol#L114-L120

Proof of Concept

https://github.com/code-423n4/2022-01-yield/blob/main/contracts/ConvexStakingWrapper.sol#L113

Tools Used

Recommended Mitigation Steps

function addRewards() public {
        address mainPool = convexPool;

        uint256 rewardsLength = rewards.length;

        if (rewardsLength == 0) {
            RewardType storage reward = rewards.push();
            reward.reward_token = crv;
            reward.reward_pool = mainPool;
            rewardsLength += 1;
        }

        uint256 extraCount = IRewardStaking(mainPool).extraRewardsLength();
    if(extraCount > 0){
            uint256 startIndex = rewardsLength - 1;
            for (uint256 i = startIndex; i < extraCount; i++) {
                    address extraPool = IRewardStaking(mainPool).extraRewards(i);
                    RewardType storage reward = rewards.push();
                    reward.reward_token = IRewardStaking(extraPool).rewardToken();
                    reward.reward_pool = extraPool;
            }
    }
    }
save ~200 gas

devtooligan commented 2 years ago

USer would save a small amount of gas when extraCount == 0 but spend extra gas on the check for all the times its > 0

GalloDaSballo commented 2 years ago

I believe the finding would add about 6 gas for the check (>), and it would save on the specific case, 12 gas.

Because the sponsor disputed, and the savings are below the 100 gas threshold, am going to mark as invalid

code-423n4 / 2022-01-yield-findings