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

dany.armstrong90 - Anyone can create bribe rewarder. #56

Closed
sherlock-admin4 opened this issue Jul 15, 2024 · 0 comments
Closed

dany.armstrong90 - Anyone can create bribe rewarder. #56

sherlock-admin4 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-admin4
Copy link

sherlock-admin4 commented Jul 15, 2024

dany.armstrong90

High

Anyone can create bribe rewarder.

Summary

RewarderFactory.sol#createBribeRewarder doesn't check if caller is owner.
Therefore, attacker can create bribe rewarders many times as he/she wants and then register them to the Voter contract.

Vulnerability Detail

RewarderFactory.sol#createBribeRewarder function is the following.

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

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

As can be seen, since it doesn't check if msg.sender is owner, attacker can create bribe rewarders many times as he/she wants and then register them to the Voter contract.

Voter.sol#onRegister function is the following.

    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) {
            // TODO check if rewarder token + pool  is already registered @audit-info no implementation 

            require(periods[i] >= currentPeriodId, "wrong period");
141:        require(_bribesPerPriod[periods[i]][pool].length + 1 <= Constants.MAX_BRIBES_PER_POOL, "too much bribes");
            _bribesPerPriod[periods[i]][pool].push(rewarder);
        }
    }

As can be seen, the maximum count of bribes per pool is limited to MAX_BRIBES_PER_POOL = 5.
So, if the attacker create 5 fake bribe rewarders and register them to the voter, administrator can't register normal bribe rewarder to the Voter contract.

Impact

An attacker can prevent administrator from registering bribe rewarders to the Voter contract.
As a result, voters cannot receive bribes.

Code Snippet

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

Tool used

Manual Review

Recommendation

Modify the RewardFactory.sol#createBribeRewarder function as follows.

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

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

Duplicate of #190

@github-actions github-actions bot added duplicate Medium A Medium severity issue. labels Jul 21, 2024
@sherlock-admin4 sherlock-admin4 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 Petite Rouge Huskie - Anyone can create bribe rewarder. dany.armstrong90 - Anyone can create bribe rewarder. 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

1 participant