You are probably safe, but be aware…

Daniel Von Fange pinged me last week:

Hey, I just realized that the xSushi reward distribution contract that’s commonly cloned around would be vulnerable to complete theft if the deposit token used was an ERC777 style that allowed rentrancy.

The message set in motion the close examination of just ~15 lines of code, handling funds in the billions.

We found not one, but two latent bugs. Both have pretty specific conditions for becoming vulnerabilities. We did our best to ascertain that current deployments are not at risk. (There was a time when an attacker could steal $60M, though.) However, this doesn’t mean there’s no risk: there may be several tokens that if you intend to stake in, one can attack you, right now, let alone what can happen with future deployments.

Take-home message:

• be extra careful with the initial stakes of xSushi-like reward contracts
• never deploy such contracts with ERC777 underlying tokens.

and generally:

• be aware of reentrancy threats when interacting with ERC777 tokens.

The Code

Here is an instance of the code in question, a common snippet of a staking token contract. Such code was originally used in the xSushi staking contract and has since been extensively cloned.

contract VulnerableXToken {
    // ..

    // Pay some tokens and earn some shares.
    function enter(uint256 _amount) public {
        uint256 totalToken = token.balanceOf(address(this));
        uint256 totalShares = totalSupply();
        if (totalShares == 0 || totalToken == 0) {
            _mint(msg.sender, _amount);
        } else {
            uint256 what = _amount.mul(totalShares).div(totalToken);
            _mint(msg.sender, what);
        }
        token.transferFrom(msg.sender, address(this), _amount);
    }

    // Claim back your tokens.
    function leave(uint256 _share) public {
        uint256 totalShares = totalSupply();
        uint256 what = _share.mul(token.balanceOf(address(this))).div(totalShares);
        _burn(msg.sender, _share);
        token.transfer(msg.sender, what);
    }
    
    // ..
}

The enter function just accepts an investment in an underlying token (token, in the above) and issues shares by minting staking tokens (VulnerableXToken). The staking tokens accrue rewards, and upon leave the investor can claim back their rightful proportion of the underlying token.

Bug #1

The code looks reentrancy-safe at first glance. The external call (transferFrom) happens after all state updates (_mint). So, it seems that nothing can go wrong.

Back on Feb. 24, well-known Ethereum security engineer t11s had tweeted a warning about ERC777 tokens.

A killer feature of ERC777 is its receive hooks, allowing contracts to react when receiving tokens. What is not mentioned often is the respective hooks called on the senders of the funds, which MUST be called before updating the state.

Implementation Requirement:
The token contract MUST call the tokensToSend hook before updating the state.
The token contract MUST call the tokensReceived hook after updating the state.

This means that any ERC777 token is a reentrancy death trap! The token itself violates the “effects before external calls” rule: it calls out to the sender before it commits the effects of a transfer. Any caller into the token may be maintaining the effects-before-calls rule, but it may not matter, if the token itself does not. The caller should either be agnostic to the token’s effects (i.e., never read balances) or should use reentrancy locks.

What Daniel had realized regarding xSushi-like code is that the PRE-transfer hook in an ERC777 token’s transferFrom would allow an attacker to reenter (literally: re-enter, in the above code) before any funds had been transferred to the staking contract, taking advantage of any state changes made before the call. In our case, upon reentering, the VulnerableXToken balance has changed (by the internal _mint call) but the underlying token’s balance has not: there are more shares but the same funds, so, when re-entering, shares appear to be cheaper!

Of course, the underlying token does not necessarily need to be an ERC777, as this exploit could be possible for any token that implements similar callback mechanisms to the ones described.

To summarize:

• IF the underlying token (token, in the above code) of an xSushi-like rewards contract calls back a hook (e.g., is an ERC777, which calls tokensToSend on the sender)
• AND the underlying token does this callback before adjusting the balance,
• THEN one can reenter and get shares cheaper, all the way to full depletion of everyone else’s funds.

Bug #2

When I shared the code in the Dedaub internal channels, Konstantinos (one of our senior engineers) immediately commented:
“I see the bug — haven’t we encountered this in an audit before?”

Indeed we had…

… but he wasn’t seeing Daniel’s bug!!!

It was an entirely different bug, based on making the division _amount.mul(totalShares).div(totalToken) round down to zero when another user is depositing. In this way, the depositor would get zero shares, but the old holders of shares would keep the newly deposited funds.

A simple attack scenario with only two depositors (attacker and victim) would go as follows:

• The attacker is the first person to call enter and deposits amount1 of token, getting back an equal amount of shares.
• The next depositor comes in and tries to deposit amount2 of token. The attacker front-runs them and directly transfer to the contract any amount of token greater than (amount2–1)*amount1.
• The attacker gets no shares in return, but they have all the shares to begin with! In this way, amount2*totalShares/totalToken rounds down to zero, leaving the next depositor with nothing, while the attacker can withdraw all the deposited token by calling leave, as they own all the shares.

To see how big an impact this bug can have, consider the first transfer to the xDVF staking token:

This was a transfer for 12.5 million DeversiFi tokens, currently valued at $5 each. An attacker could have front-run that transfer and stolen all $60M worth of tokens!

Checks for Live Vulnerabilities

To determine if there are funds under threat right now, we used the Dedaub Watchdog database to query all currently-deployed contracts on Ethereum, together with their current balances.

• There are 239 deployed Ethereum contracts with the xSushi-like enter/leave code in publicly posted source.
• 13 of those have staked funds right now. The highest-value are xShib ($960M), xSushi ($233M), and xDVF ($72M).
• None of those has an ERC777 as an underlying.
• The largest value that would have been at risk of a front-running attack during the initial deposit is the $60M in xDVF, as discussed earlier.
• We also checked Polygon, found only 4 xSushi-like contracts, none with staked funds.

Although the above numbers should be fairly complete, it’s worth noting there may still be threats we are missing. The same code could be deployed in networks other than Ethereum; vulnerable contracts may have no published source, so our search may have missed them; our balances query may be incomplete for tokens that don’t emit the expected events upon transfers; and our Polygon scan was not exhaustive — just considered the last 200K contracts or so.

And, of course, any initial staking in any xSushi-like contract, among the current 200+ deployed or future ones, is vulnerable to front-running attacks.

Conclusion

The code we looked at is very simple and problematic only in very subtle ways, not all under its control (e.g., the ERC777 reentrancy is arguably not the xSushi code’s problem). It is likely to keep getting cloned, or to have independently arisen in other settings. (In the latter case, please let us know!)

Either way, we repeat our message for awareness:

• be extra careful with the initial stakes of xSushi-like reward contracts
• never deploy such contracts with ERC777 underlying tokens.

and generally:

• be aware of reentrancy threats when interacting with ERC777 tokens.

These are attack vectors that the community should know about, lest we see one of them being exploited for heavy damages.