Skip to content
This repository has been archived by the owner on Jan 12, 2025. It is now read-only.

kmXAdam - Insufficient Reward Validation Allows Malicious Bribes to Block Legitimate Rewards for Multiple Periods #373

Closed
sherlock-admin2 opened this issue Jul 15, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented Jul 15, 2024

kmXAdam

High

Insufficient Reward Validation Allows Malicious Bribes to Block Legitimate Rewards for Multiple Periods

Summary

Users can create bribes to reward voters for voting for a pool. the contract only allows a max of 4 bribes to be registered for a pool at any given period. but because anybody can create bribeRewarders, it opens up a griefing attack where attackers can fill up a pool with dummy bribes not allowing legitimate bribes to register. leading to voters not being incentivised to vote.

Vulnerability Detail

creating bribes for voting is permissionless and anybody can do it through the RewarderFactory:

function createBribeRewarder(IERC20 token, address pool) external returns (IBribeRewarder rewarder) {
        rewarder = IBribeRewarder(_cloneBribe(RewarderType.BribeRewarder, token, pool));

        emit BribeRewarderCreated(RewarderType.BribeRewarder, token, pool, rewarder);
    }

this basically creates a bribe for a pool, that can then be registered to it for any amount of periods the user wants:

 function _bribe(uint256 startId, uint256 lastId, uint256 amountPerPeriod) internal {
        _checkAlreadyInitialized();
=>        if (lastId < startId) revert BribeRewarder__WrongEndId();
=>        if (amountPerPeriod == 0) revert BribeRewarder__ZeroReward();
        ...

the user is in complete control of how many periods to bribe and also with what amount.

the bribeRewarer registers with a pool through the onRegister function:

 function onRegister() external override {
        IBribeRewarder rewarder = IBribeRewarder(msg.sender);

        _checkRegisterCaller(rewarder);

        uint256 currentPeriodId = _currentVotingPeriodId;
        (address pool, uint256[] memory periods) = rewarder.getBribePeriods();
        for (uint256 i = 0; i < periods.length; ++i) {
            require(periods[i] >= currentPeriodId, "wrong period");
            
=>            require(
                _bribesPerPriod[periods[i]][pool].length + 1 <=
                    Constants.MAX_BRIBES_PER_POOL,
                "too much bribes"
            );

            _bribesPerPriod[periods[i]][pool].push(rewarder);
        }
    }

and as we can see there is a check on the number of bribes that can be registered per period, it only allows a maximum of 4.

this can be exploited by an attacker by registering 4 dummy bribes for each pool he is trying to grief. and not allowing legitimate bribes to be registered.

basically not allowing legitimate users to bribe voters to vote for a pool.

Proof Of Concept

Knowing this information, the way for a bribe to exploit this and leave a pool with no bribe rewards for N periods ahead is.

Right after the pool is up for voting, the malicious user decides to make the pool not receive any bribe rewards for 40 periods ahead.

  1. In a loop create 4 BribeRewarders for this pool by calling RewarderFactory::createBribeRewarder which is permissionless function.
  2. Send 40 wei of rewardToken to all 4 of the rewarders.
  3. In a loop, he calls BribeRewarder::bribe, with parameters startPeriod=currentPeriod + 1, endPeriod= currentPeriod + 41, rewardPerPeriod=1.
  4. Now the _bribesPerPriod array is filled with malicious bribes up to 40 periods ahead and since we have a check of max bribes per pool which is 5, no one will be able to register a normal BribeRewarder.

Impact

Malicious user can fill up to N periods ahead with BribeRewarders that do not accumulate any rewards for the users, also blocking good BribeRewarders from contributing to the pool.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/tree/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L226-L258

https://github.com/sherlock-audit/2024-06-magicsea/tree/main/magicsea-staking/src/Voter.sol#L130-L144

https://github.com/sherlock-audit/2024-06-magicsea/tree/main/magicsea-staking/src/rewarders/RewarderFactory.sol#L109-L113

Tool used

Manual Review

Recommendation

Consider having only trusted people create the bribes and let other people fund them, this will make sure that no malicious bribes can be created.

Duplicate of #190

@github-actions github-actions bot added duplicate Medium A Medium severity issue. labels Jul 21, 2024
@sherlock-admin2 sherlock-admin2 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Sleepy Navy Ant - Insufficient Reward Validation Allows Malicious Bribes to Block Legitimate Rewards for Multiple Periods kmXAdam - Insufficient Reward Validation Allows Malicious Bribes to Block Legitimate Rewards for Multiple Periods Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants