Velodrome Finance contest - RoiEvenHaim's results

Lines of code

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L626-L633 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L511-L543 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L424-L431 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L432-L470 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L347-L373

Vulnerability details

swapping reward token problem in Gause.sol

There is the option to have the same token twice or more in the isReward array because in swapOutRewardToken function there is no check if the oldToken is diffrent from the newToken. Therefor lets assume we have 3 tokens in the array and he looks like that:

token a

token b

token c

Now we will call the swapOutRewardToken( i= 1, b, a) and the new array will look like that:

token a

token a

token c

Same way to token c and the array will look like:

token a

token a

token a

Now we will call the deposit function and deposit will call _updateRewardForAllTokens, there we are updating the reward for all tokens. we will go over our array(the arrray has the same token couple times).

(rewardPerTokenStored[token], lastUpdateTime[token]) = _updateRewardPerToken(token);

this line will call _updateRewardPerToken .

if (_endIndex > 0) {
            for (uint i = _startIndex; i < _endIndex; i++) {
                SupplyCheckpoint memory sp0 = supplyCheckpoints[i];
                if (sp0.supply > 0) {
                    SupplyCheckpoint memory sp1 = supplyCheckpoints[i+1];
                    (uint _reward, uint _endTime) = _calcRewardPerToken(token, sp1.timestamp, sp0.timestamp, sp0.supply, _startTimestamp);
                    reward += _reward;
                    _writeRewardPerTokenCheckpoint(token, reward, _endTime);
                    _startTimestamp = _endTime;
                }
            }
        }

And this loop will raise the reward by _reward that calculates from the _calcRewardPerToken and instead of raise by one time it will raise the rewardPerTokenStored[token] many times.

and now we can call getReward function and take our reward.

Recommended Mitigation Steps

function swapOutRewardToken(uint i, address oldToken, address newToken) external {
        require(msg.sender == IGaugeFactory(factory).team(), 'only team');
        require(rewards[i] == oldToken);
        isReward[oldToken] = false;
        isReward[newToken] = true;
        rewards[i] = newToken;
    }

add a require to check that the new token is not already in the array.

fix- 

function swapOutRewardToken(uint i, address oldToken, address newToken) external {
        require(msg.sender == IGaugeFactory(factory).team(), 'only team');
        require(rewards[i] == oldToken);
        require( isReward[newToken] == false); // added require
        isReward[oldToken] = false;
        isReward[newToken] = true;
        rewards[i] = newToken;
    }