The day a `safeTransferFrom` ate our liquidity

The page came in at 03:11 . A staking vault we’d shipped two weeks earlier was bleeding. Not all of it — about 38% of TVL had moved out of the contract in a single transaction. The remainder followed in the next two blocks before the ops team paused the contract.

The transaction that drained us looked, on Etherscan, like a perfectly ordinary unstake() call. There was no obvious exploit in the calldata. There were no admin actions. The amount being unstaked, on its face, matched the user’s balance.

Below is the version of events I wrote up the next morning, mostly so I would never do this to myself again.

What it looked like first

The first thing I did was open Tenderly and replay the offending transaction with full traces. The shape was startling: our unstake() function appeared to execute twice against the same balance entry.

[CALL]   Vault.unstake(amount=10_000e18)
  [CALL]   ERC20.transfer(user, 10_000e18)
    [CALL]   user.tokensReceived(...)
      [CALL]   Vault.unstake(amount=10_000e18)   <-- here
        [CALL]   ERC20.transfer(user, 10_000e18)
        ...

That tokensReceived call was the giveaway. We were not handling an ERC20. We were handling an ERC777.

The token in question had been added to the supported-asset list during a routine governance vote five days earlier. Nobody on the engineering side had reviewed which interface it implemented, because the proposal said “ERC20-compatible” and the token honored every ERC20 method. Which it does. ERC777 is a strict superset of ERC20 — every ERC777 token is also a valid ERC20. That’s the whole point, and that’s the problem.

Reproducing it locally

I wrote a minimal reproduction in Foundry that night, mostly to convince myself I understood it. The test contract is below; the full version (with the malicious receiver hook) is in the gist linked at the bottom.

// pseudocode of our buggy unstake()
function unstake(uint256 amount) external {
    require(balances[msg.sender] >= amount, "insufficient");
    token.safeTransfer(msg.sender, amount);   // (1) external call BEFORE state update
    balances[msg.sender] -= amount;           // (2) only updated after
}

Two facts that don’t matter individually but turn lethal together:

1. We made the external safeTransfer call before updating internal accounting (textbook checks-effects-interactions violation, except we hadn’t thought it could matter for a “boring” ERC20)
2. The token had a tokensReceived callback that re-entered our contract while balances[msg.sender] still held the pre-deduction value

So when the attacker’s contract received the tokens, it called unstake() again, and again the balance check passed, and again safeTransfer was called.

Why we missed it

A few honest reasons.

Misplaced trust in SafeERC20

Our threat model said “use SafeERC20, you’re fine.” SafeERC20 protects you from non-standard return values (e.g. tokens that don’t return a bool). It does not protect you from the side effects of transfer calls when the token is reentrant. We had conflated “safe” in the wrapper’s name with “safe in every meaningful sense,” and never re-checked.

A nonReentrant we thought we didn’t need

We had nonReentrant on stake(). Not on unstake(). The original author left a comment that said “unstake is exit-only, no need.” Exit-only is exactly when an attacker wants to call you. The comment was reasonable when the only tokens we accepted were ETH and USDC. It stopped being reasonable the moment we added a token with hooks. Nobody re-read the comment.

A token allowlist nobody audited

A list with 30+ tokens is a list nobody reads carefully, and “ERC20-compatible” was treated as a synonym for “boring.” There was no checkbox in our governance template asking what interfaces does this token implement?

What we changed

The contract patch

Move the state update before the transfer, and add nonReentrant to every external function that touches balances:

// after
function unstake(uint256 amount) external nonReentrant {
    require(balances[msg.sender] >= amount, "insufficient");
    balances[msg.sender] -= amount;            // effects first
    token.safeTransfer(msg.sender, amount);    // then interaction
}

The allowlist check

We wrote a one-liner script that, for every token in the allowlist, calls IERC1820Registry.getInterfaceImplementer(token, keccak256("ERC777Token")) and flags any non-zero result for human review. ERC777 tokens aren’t banned — they’re escalated to a human who has to decide whether the integration handles re-entry safely.

The governance change

The deeper fix was governance. We moved the token-onboarding process out of “anyone proposes, vote happens” into a two-step where engineering signs off on interface compatibility before the vote. The form has six checkboxes; one of them is “this token does not implement ERC777, ERC1363, or any other hook-bearing extension.” Boring forms catch boring bugs.

What this changed in how I read code

For a few months after this I had a tic where, every time I saw an ERC20 token interaction, I would mentally ask: what if this token re-enters me? Most of the time the answer was “it can’t” or “even if it does, nothing bad happens.” But asking the question once per call site is cheap, and the few times the answer is interesting, you find a real bug.

A senior friend told me afterward that this is the same instinct C developers used to have around realloc: every time you call it, you ask yourself what if this returns a different pointer? Most of the time it doesn’t matter. The handful of times it does, you would have leaked a use-after-free. The instinct is what saves you.

I’d had the abstract knowledge of reentrancy since the first DAO post-mortem I read. I had not had the instinct. Three weeks of writing post-mortems, attending the user calls, and watching governance refund the affected wallets gave me the instinct. It’s expensive but I haven’t lost it since.

Closing

Computers are not magic, the spec was sitting on the ERC-777 page the whole time, and I could have read it. I will read everything that touches money the long way from now on. That’s the lesson, and it’s not a particularly satisfying one — but unsatisfying lessons are usually the ones I actually remember.