# Principal Freezing and Ransom Attacks with MasterChefV2

By [onewayfunction](https://paragraph.com/@onewayfunction) · 2022-01-19

---

One of the services I've been offering for the past several months is quick (less than 2hr) security checks of yield farming pools. I look for rug-pull potential and security risks in yield farming contracts to help protect would-be farmers and LPs. I basically try to spot trouble with farms before a user puts their funds at risk.

As you you might imagine, I've seen countless forks of `MasterChef` -- both its [original incarnation](https://etherscan.io/address/0xc2edad668740f1aa35e4d8f227fb8e17dca888cd) and its official second version, [MasterChefV2](https://etherscan.io/address/0xef0881ec094552b2e128cf945ef17a6752b4ec5d#code). I've seen all manner of shenanigans and optimizations, but this article is about a property of the official `MasterChefV2` and any fork that remains faithful to its original functionality.

One benefit of the original `MasterChef` is that the owner of the contract cannot steal a user's principal, even if the owner were malicious. A malicious owner might be able to mess with a user's rewards, but they could never prevent a user from getting back the LP tokens they had initially invested. For example, with the original `MasterChef`, a user can always call the [emergencyWithdraw](https://etherscan.io/address/0xc2edad668740f1aa35e4d8f227fb8e17dca888cd#code#L1596) function to retrieve their LP tokens, and there is nothing in the world that the owner of the `MasterChef` contract can do to prevent that.

By contrast, the owner of a `MasterChefV2` contract can freeze any user's principal at will. This freezing can be parlayed into a ransom attack, whereby users are able to remove their principal from `MasterChefV2` if and only if they've paid the owner a ransom.

This property of `MasterChefV2` means that I always have to perform additional safety checks (discussed below) on the owner and the values/addresses they set in `MasterChefV2`. It also means the client/user needs to make sure they have good alerting set up to catch any shenanigans the owner may try to pull.

How it works: The Freeze
------------------------

The key to freezing user’s funds is the [“set” function](https://github.com/sushiswap/sushiswap/blob/271458b558afa6fdfd3e46b8eef5ee6618b60f9d/contracts/MasterChefV2.sol#L131), which the owner can call at any time, including after a pool has been initialized and users have deposited LP tokens. The `set` function allows the owner [to assign](https://github.com/sushiswap/sushiswap/blob/271458b558afa6fdfd3e46b8eef5ee6618b60f9d/contracts/MasterChefV2.sol#L134) a `rewarder` to a pool (or update a pool's `rewarder` if it already has one). The `rewarder` is a contract that implements a mutable [“onSushiReward” function](https://github.com/sushiswap/sushiswap/blob/271458b558afa6fdfd3e46b8eef5ee6618b60f9d/contracts/interfaces/IRewarder.sol#L7), which can be anything the owner wants.

This function on the `rewarder` is called during both [the “withdraw” function](https://github.com/sushiswap/sushiswap/blob/271458b558afa6fdfd3e46b8eef5ee6618b60f9d/contracts/MasterChefV2.sol#L244) and [the “emergencyWithdraw” function](https://github.com/sushiswap/sushiswap/blob/271458b558afa6fdfd3e46b8eef5ee6618b60f9d/contracts/MasterChefV2.sol#L321) -- which are the only two functions that a user can call to get their LP tokens out of the contract.

To freeze all users’ LP tokens for a given pool, the owner can call the `set` function and set the `rewarder` of the pool to be the following contract:

    contract BadRewarder {
      function onSushiReward(...) external {
        revert('lulz');
      }
    }
    

The result would be that all user's attempts to withdraw would result in a reverting transaction.

If so desired, the owner can make the freeze selective, singling out individual users, by setting the `rewarder` to a contract like this:

    contract SelectiveFreezer {
      mapping (address => bool) public isFrozen;
    
      function setIsFrozen(address _user, bool _isFrozen) external onlyOwner {
        isFrozen[_user] = _isFrozen;
      }
    
      function onSushiReward(...) external {
        if (isFrozen[user]) revert('lulz');
      }
    }
    

How it works: The Ransom
------------------------

It is commonly the case that a DoS attack, like the above "freeze" attack, can be parlayed into a ransom attack. It’s just the old “pay me and I’ll stop DoS-ing you“ bit, but in smart contract form.

A malicious `MasterChefV2` owner could set the `rewarder` to be a contract similar to this one:

    contract Ransom {
      uint256 constant public requiredRansom = 1 ether;
      mapping (address => bool) public hasPaidRansom;
    
      function payRansom() external payable {
        require(msg.value >= requiredRansom, 'send more money');
        hasPaidRansom[msg.sender] = true;
      }
    
      function onSushiReward(...) external {
        if (!hasPaidRansom[user]) revert('must pay ransom');
      }
    
      function collectRansom() external onlyOwner {
        payable(owner).transfer(address(this).balance);
      }
    }
    

The malicious owner would then need to verify this contract on Etherscan, and then transfer ownership of the `MasterChefV2` instance to the zero address.

The result is that users would be able to withdraw their LP from `MasterChefV2` if and only if they pay a 1 ETH ransom.

Note that this is just a simple example, and the malicious `rewarder` could be made to scale its ransom based on the amount of LP tokens the user had deposited in `MasterChefV2`.

Mitigation
----------

This property of `MasterChefV2` means that users must trust the owner. As a result, I always have to perform checks that the owner is, for example, an instance of the [Timelock contract](https://etherscan.io/address/0x9a8541ddf3a932a9a922b607e9cf7301f1d47bd1#code#L185) with a reasonable value for `MINIMUM_DELAY`, and that the client/user always has good alerting set up to be notified if/when the owner tries to make a change to the `MasterChefV2` fork. It also means that I need to check the current `rewarder` (if any) before the user apes into the pool.

For honest developers who are forking `MasterChefV2`, I recommend removing the `_rewarder.onSushiReward` call from the `emergencyWithdraw` function. The `emergencyWithdraw` function should be a clear path for users to exit without any possibility of a malicious owner stopping them. (Note that a try/catch _could_ work here, but you would need to be sure not to forward too much gas to the potentially-malicious `rewarder`, which could otherwise burn enough gas so that the subsequent `safeTransfer` would always fail). If you don’t make this contract-level fix, then be sure to make the owner an instance of the `Timelock` contract with a reasonable minimum delay (e.g., 2 days or more).

For users aping into these `MasterChefV2` forks, be sure to check for all the things that I do:

*   Owner should be `Timelock` with a good minimum delay.
    
*   If any `rewarder` is currently set for your pool, it should be verified on Etherscan and its `onSushiReward` function should have no possibility of reverting or burning a ton of gas.
    
*   You should have monitoring and alerting set up (e.g., via Etherscan, Tenderly, Forta, etc) so you are notified anytime a change gets queued up in the `Timelock`, that way you can exit `MasterChefV2` before a malicious `rewarder` gets dropped into your pool.
    

If you found this helpful, please consider contributing to my [security spot checks project](https://github.com/Austin-Williams/pro-bono-spot-checks).

Stay safe out there. Cheers!

---

*Originally published on [onewayfunction](https://paragraph.com/@onewayfunction/principal-freezing-and-ransom-attacks-with-masterchefv2)*