Bribe Manual Review Findings

BEB-01M: Potential Denial-of-Service Attack of Rewards

Type	Severity	Location
Logical Fault		Bribe.sol:L442-L445

Description:

The notifyRewardAmount mechanism will automatically add a token if it does not already exist in the rewards array without an upper limit on the total entries thus potentially creating a Denial-of-Service (DoS) attack on the _updateRewardForAllTokens function.

Impact:

It is currently possible to add an unlimited number of reward tokens which could cause the contract to become inoperable wherever the _updateRewardForAllTokens function is invoked, a trait highly undesirable for such a high-value system.

Example:

Copy
421// used to notify a gauge/bribe of a given reward, this can create griefing attacks by extending rewards
422function notifyRewardAmount(address token, uint amount) external lock {
423    require(amount > 0);
424
425    if (rewardRate[token] == 0) _writeRewardPerTokenCheckpoint(token, 0, block.timestamp);
426    (rewardPerTokenStored[token], lastUpdateTime[token]) = _updateRewardPerToken(token, type(uint).max, true);
427
428    if (block.timestamp >= periodFinish[token]) {
429        _safeTransferFrom(token, msg.sender, address(this), amount);
430        rewardRate[token] = amount / DURATION;
431    } else {
432        uint _remaining = periodFinish[token] - block.timestamp;
433        uint _left = _remaining * rewardRate[token];
434        require(amount > _left);
435        _safeTransferFrom(token, msg.sender, address(this), amount);
436        rewardRate[token] = (amount + _left) / DURATION;
437    }
438    require(rewardRate[token] > 0);
439    uint balance = IERC20(token).balanceOf(address(this));
440    require(rewardRate[token] <= balance / DURATION, "Provided reward too high");
441    periodFinish[token] = block.timestamp + DURATION;
442    if (!isReward[token]) {
443        isReward[token] = true;
444        rewards.push(token);
445    }
446
447    emit NotifyReward(msg.sender, token, amount);
448}

Recommendation:

We advise the reward tokens to be specified during the initialization of the contract with a maximum upper limit imposed via a contract-level constant to ensure no malicious tokens can be added to the rewards array and that all the expected reward tokens are defined during the creation of the contract rather than dynamically. Alternatively, we advise a dedicated and privileged function to be introduced that permits manually white-listing reward tokens that would then be accepted by the notifyRewardAmount function. Either of the two solutions are sufficient to deal with the vulnerability described.

Alleviation:

The Xcaliswap team has implemented a constant MAX_REWARD_TOKENS upper bound on the rewards array. Please note, since the require statement uses < operator for upper bound comparison the rewards array can only contain a maximum of 15 values.