Open sherlock-admin3 opened 1 month ago
In Voter.sol
and BribeRewarders.sol
contracts, a maximum of 5 BribeRewarder.sol
contracts are allowed per pool and these can be added without permission.
A malicious actor could use worthless tokens to fill this limit, preventing real rewards from being added. This results in user incentives being ineffective and the voting system becoming dysfunctional.
The protocol team fixed this issue in the following PRs/commits: https://github.com/metropolis-exchange/magicsea-staking/pull/35
The Lead Senior Watson signed off on the fix.
dimulski
High
Adding genuine BribeRewarder contract instances to a pool in order to incentivize users can be DOSed
Summary
In the
Voter.sol
contract, protocols can createBribeRewarder.sol
contract instances in order to bribe users to vote for the pool specified by the protocol. The more users vote for a specific pool, the bigger the weight of that pool will be compared to other pools and thus users staking the pool LP token in theMasterchefV2.sol
contract will receive more rewards. The LP token of the pool the protocol is bribing for, receives bigger allocation of the LUM token in theMasterchefV2.sol
contract. Thus incentivizing people to deposit tokens in the AMM associated with the LP token in order to acquire the LP token for a pool with higher weight, thus providing more liquidity in a trading pair. In theVoter.sol
contract a voting period is defined by an id, start time, and a end time which is the start time + the globaly specified _periodDuration. When a protocol tries to add aBribeRewarder.sol
contract instance for a pool and a voting period, they have to call the fundAndBribe() function or the bribe() funciton and supply the required amount of reward tokens. Both of the above mentioned functions internally call the _bribe() function which in turn calls the onRegister() function in theVoter.sol
contract. There is the following check in the onRegister() function:Constants.MAX_BRIBES_PER_POOL is equal to 5. This means that each pool can be associated with a maximum of 5 instances of the
BribeRewarder.sol
contract for each voting period. The problem is that addingBribeRewarder.sol
contract instances for a pool for a certain voting period is permissionless. There is no whitelist for the tokens that can be used as reward tokens in theBribeRewarder.sol
contract or a minimum amount of rewards that have to be distributed per each voting period. A malicious actor can just deploy an ERC20 token on the network, that have absolutely no dollar value, mint as many tokens as he wants and then deploy 5 instance of theBribeRewarder.sol
contract by calling the createBribeRewarder() function in theRewarderFactory.sol
contract. After that he can either call fundAndBribe() function or the bribe() function in order to associate the above mentionedBribeRewarder.sol
contract instances for a specific pool and voting period. A malicious actor can specify as many voting periods as he want. _periodDuration is set to 1209600 seconds = 2 weeks onVoter.sol
initialization. If a malicious actor setsBribeRewarder.sol
contract instances for 100 voting periods, that means 200 weeks or almost 4 years, no otherBribeRewarder.sol
contract instance can be added during this period. When real rewards can't be used as incentives for users, nobody will vote for a specific pool. A malicious actor can dos all pools that can potentially be added for example by tracking what new pools are created at the MagicSea exchange and then immediately performing the above specified steps, thus making the entireVoter.sol
andBribeRewarders.sol
contracts obsolete, as their main purpose is to incentivize users to vote for specific pools and reward them with tokens for their vote. Or a malicious actor can dos specific projects that are direct competitors of his project, thus more people will provide liquidity for an AMM he is bribing users for, this way he can also provide much less rewards and still his pool will be the most lucrative one.Vulnerability Detail
Gist
After following the steps in the above mentioned gist add the following test to the
AuditorTests.t.sol
file:To run the test use:
forge test -vvv --mt test_BrickRewardBribers
Impact
A malicious actor can dos the adding of
BribeRewarder.sol
contract instances for specific pools, and thus either makeVoter.sol
andBribeRewarders.sol
contracts obsolete, or prevent the addition of genuineBribeRewarder.sol
contract instances for a specific project which he sees as a competitor. I believe this vulnerability is of high severity as it severely restricts the availability of the main functionality of theVoter.sol
andBribeRewarders.sol
contracts.Code Snippet
https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L130-L144
Tool used
Manual Review & Foundry
Recommendation
Consider creating a functionality that whitelists only previously verified owner of pools, or people that really intend to distribute rewards to voters, and allow only them to add
BribeRewarders.sol
contracts instances for the pool they are whitelisted for.