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

pashap9990 - If a user sets BribeRewarder for a specific pool all votes for that pool will be reverted #322

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 High A High severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented Jul 15, 2024

pashap9990

High

If a user sets BribeRewarder for a specific pool all votes for that pool will be reverted

Summary

In Voter::vote function _notifyBribe is called which in turn calls BriberRewerder::deposit, which means "msg.sender" is Voter contract but in the BriberRewerder::deposit MlumStaking::ownerOf is checked with msg.sender and this causes all votes for pools that have bribeRewarder is reverted

Vulnerability Detail

Add this test to voter.t.sol
Coded POC:
Add this lines to setUp function

+ RewarderFactory _rewardFactory;
function setUp()public {
...
+ address factoryImpl = address(new RewarderFactory());

+        _rewardFactory = RewarderFactory(
+           address(
+                new TransparentUpgradeableProxy2Step(
+                    factoryImpl, ProxyAdmin2Step(address(1)), abi.encodeWithSelector(RewarderFactory.initialize.selector, DEV, 
+ _rewardType, _rewarder)
+                )
+           )
+        );
    function testRevertVotes() public {

        ERC20Mock pool = new ERC20Mock("lp","LP", 18);
        ERC20Mock usdt = new ERC20Mock("USDT","USDT", 6);
        usdt.mint(address(this), 300e6);

        IBribeRewarder _bribeRewarder1 = _rewardFactory.createBribeRewarder(usdt, address(pool));
    
        _createPosition(ALICE);

        
        usdt.approve(address(_bribeRewarder1), 300e6);
        _bribeRewarder1.fundAndBribe(1, 3, 100e6);
        //start first epoch
        vm.prank(DEV);
        _voter.startNewVotingPeriod();
        address[] memory pools = new address[](1);
        pools[0] = address(pool);

        
        vm.prank(ALICE);
        vm.expectRevert();
        _voter.vote(1, pools, _getDeltaAmounts());
    
    }

Impact

LPs cannot vote for pools with bribeRewarder

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L211

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L225

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L264

Tool used

Manual Review

Recommendation

Its better pass account as parameter to deposit function

Duplicate of #39

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Jul 21, 2024
@0xSmartContract 0xSmartContract added High A High severity issue. Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Jul 28, 2024
@sherlock-admin4 sherlock-admin4 changed the title Chilly Iris Parakeet - If a user sets BribeRewarder for a specific pool all votes for that pool will be reverted pashap9990 - If a user sets BribeRewarder for a specific pool all votes for that pool will be reverted 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 High A High severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants