sherlock-audit / 2024-06-magicsea-judging

2 stars 0 forks source link

Icy Basil Seal - In case MasterChef stake token is ERC777, it allows to take the contracts balance as free flashloan #716

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Icy Basil Seal

Low/Info

In case MasterChef stake token is ERC777, it allows to take the contracts balance as free flashloan

Summary

When using an ERC777 as the staking token in MasterChefV2, an attack can drain the contract by utalizing the toSender() hook. The protocol mentions in the README all wierd tokens are supported without ristrictions.

Vulnerability Detail

The ERC777 standart implements a toSender() hook, which creates a callback to the account where tokens are transfered from.

MasterChefV2 does not implement reentrancy protection, this allows to call emergency withdraw in the toSender() hook to withdraw tokens before depositing:

function deposit(uint256 pid, uint256 amount) external override {
        _modify(pid, msg.sender, amount.toInt256(), false); < Internal state update

        if (amount > 0) _farms[pid].token.safeTransferFrom(msg.sender, address(this), amount); < Transfer
    }

We can see that the state in deposit is updated before tokens are transfered. This allows us to "FlashLoan" the contract balance, even if the transfer transfers tokens in the end.

Impact

Allows user to use contract balance as flashloan.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L295-L299

Tool used

Manual Review

Recommendation

In case protocol wants to support ERC777 tokens, use reentrancy protection by openzeppelin. Because there is no real impact to this issue this report should only inform magicsea about this.