Gridlock (a smart contract bug)

Edgeware’s Lockdrop smart contract has processed over $900 million of ETH and locked up over $290 million, all while hiding a fatal bug.

If you participated in the lockdrop you need not worry — your funds were never at risk, and after I reported the defect it was fixed — but what was the bug and what lessons can we learn?

The Edgeware Lockdrop

The Edgeware blockchain platform will have a native token called EDG. Rather than sell EDG to the highest bidder, or airdrop them to all and sundry, the generous folks over at Edgeware HQ have opted to give 90% of them away to holders of Ether (ETH) who ask nicely¹, via a process they’re calling a “lockdrop”.

If you have ETH, there are two ways to claim some EDG in the lockdrop:

1. signal your interest, from any Ethereum account or contract that holds ETH, without committing or locking any funds
2. lock ETH so that it is inaccessible for 3–12 months, in an Ethereum smart contract designed by Edgeware

Option 2 is more interesting and yields far more EDG than Option 1, so that’s the one we’re going to look at.

You’ve got to ask yourself one question: “Do I feel locky?” Well, do ya, punk?

Storing large amounts of ETH in Solidity smart contracts has not always ended well. This is presumably why Edgeware decided that instead of this sort of arrangement…

…they will segregate the locked funds like this:

In smart contract terms, each participant wishing to lock ETH will send it to a single Lockdrop contract (steps 1,3, 5). Every time this happens, the Lockdrop will create a time-locked and participant-specific Lock contract (steps 2, 4, 6), and will forward all the received ETH to that Lock.

Fig 1

The code for both Lockdrop contract and the Lock contracts is open source and has been professionally audited. Is that enough to alleviate concerns about losing funds to smart contract bugs? $290 million worth of locked ETH (so far) says, emphatically, yes!

But remember that all of these Lock contracts are running the same code and being created in the same way, so a bug in their code or in the initialisation logic could still affect all participants.

It doesn’t help that the code for the Lock contract itself is written in solidity assembly for dubious reasons, further reducing the pool of people who will be comfortable reviewing it. It may be interesting to talk about how the Lock contract works and why it was written that way some other time, but today we’re going to focus on the code that creates the Locks and sends money to them.

The Bug

Here is the Lockdrop code that carries out steps 2, 4 & 6 in Fig 1.

There are a few checks here to make sure that the work being carried out is going as expected:

• Lines 4 and 5 check that the current block time is within the 90 day lockdrop eligibility window.
• Line 9 checks that the funds are being locked for either 3, 6 or 12 months (affects EDG allocation).
• Line 13 checks that all the money arrived safely in the Lock contract.

Have a think about those checks and see if you can spot the bug I’m calling Gridlock.²

If you found the bug, well done. Skip to the next section.

If you did not, my earlier visual analogy probably didn’t help. A more apt image might have looked like this:

The Lock contract, like all Ethereum contracts, has a metaphorical slot on the top.

Even if a contract at address x does not have a payable function that explicitly receives ETH, anyone can forcibly increase its ETH balance by either:

• selfdestructing any other contract and sending its remaining ETH to address x
• sending ETH to address x before any contract is instantiated at that address

And so we see that line 13 makes a dangerous assumption:

assert(address(lockAddr).balance == msg.value);

Anyone can force this check to fail by sending some spare change to the address of the next Lock before it even exists.

But how can someone send ETH to a nonexistent contract that does not have an address? Those who’ve read Ethereum’s yellow paper (who hasn’t?!³) will remember from chapter 7 that the address of the next Lock is entirely deterministic. Coincidentally, theLockdrop contract can even help us calculate it. In a truffle console this looks something like:

const addr = “0x1b75b90e60070d37cfa9d87affd124bb345bf70a”
const ldrop = await Lockdrop.at(addr)
const ldropNonce = await web3.eth.getTransactionCount(addr)
const nextLockAddr = await ldrop.addressFrom(addr, ldropNonce)

If we send as little as 1 wei to nextLockAddr before the corresponding Lock is created, the Lockdrop.lock() function becomes jammed and unusable. And it is not jammed just once, but forever — because the address of the next Lock only changes when one is successfully created⁴. So anyone can break the whole lockdrop process indefinitely with about four lines of code!

Is it fixed?

Edgeware have just redeployed a new Lockdrop contract to fix this bug, by altering line 13 to:

assert(address(lockAddr).balance >= msg.value);

If you hurry over to the previous address you might still be able to be the one to break it. You didn’t hear it from me.

What lessons should we learn?

Invariants, obviously

If there were no non-obvious ways for a contract’s balance to increase then there would be no way to jam the lockdrop.

If sending ETH didn’t facilitate non-obvious re-entry attacks then The DAO would have ̶b̶e̶e̶n̶ ̶a̶b̶s̶o̶l̶u̶t̶e̶l̶y̶ ̶f̶i̶n̶e̶ found some other Byzantine way to screw itself up.

Taken individually, non-obvious behaviour that goes against reasonable developer expectations can often be well justified. But in aggregate, my feeling is that we — the Ethereum community — should try to do better when designing the details of WASM and Serenity. (Is there a variant of UX testing that focusses on the expectations of programmers programming for a given platform?)

Invariants, consistently

Better language design and developer education can both help to mitigate the problems of non-obvious behaviour. But these things alone will not suffice if we keep changing the rules! Upgrades that change behaviour, even in positive ways, can result in a non-obvious and therefore problematic behaviour.

For example, EIP-1283’s invariant-violating re-entrancy side effect caused it to be pulled from Constantinople at the last minute. But CREATE2’s invariant-violating “mutable contracts” side effect was considered acceptable; arguably only because it was not widely understood until shortly before the rescheduled fork.

We must aim to do better at preserving existing behaviour and respecting existing developer /auditor /contract expectations during platform upgrades. We should start with “Ethereum 1.X” changes like state rent / stateless clients.

Don’t be over-assertive

Assertions (including assert and require functions in solidity) are powerful beasts. They can be massively beneficial to ensure code fails fast and safely, rather than slowly and (perhaps) dangerously.

But over-zealous assertions like Gridlock, and another that was removed from lockdrop shortly before deployment, can completely cripple an otherwise working contract.

We should use assertions with great responsibility befitting their great power, by double-checking whether they can be violated for non-obvious reasons — particularly when they concern the state of another address or contract.

We must test our assertions thoroughly along the happy path where the assertion passes, but also along the sad path where it fails. (My two hour “audit” of the lockdrop codebase also found a significant testing bug, which meant that sad paths of requires and reverts were not really being tested at all, even though it looked like they were.)

Bugs gonna bug

Maybe this Gridlock bug would have been caught before deployment if it had put funds at risk. I guess we’ll never know.

On the other hand, maybe there are still undiscovered bugs in these contracts. And maybe there are bugs in the solidity compiler, or in the EVM, that would affect this contract.

Ultimately, smart contracts are software. Even carefully audited, well tested software will (almost always) contain bugs.

Therefore, and despite our best efforts…

Smart contracts will (almost always) contain bugs!

This is, admittedly, not very insightful.

We’ve been taught this lesson several times since 2015. But I’m not sure we’ve all learned it yet.

Around 50 people each sent more than $1million through this bugged lockdrop contract.

There’s nearly half a billion dollars of ETH locked in Maker CDPs!

All of this is not to say that smart contracts are hopeless — they are not — but if you ever put significant money into the trust of a contract, even an audited one, please make sure you’re factoring bugs into your risk vs. reward calculation.

I for one will not be locking $290 million of my money⁵ in a smart contract any time soon. 😜

Thanks to Edgeware for addressing this bug and other issues promptly after I raised them, and to Josep Sanjuas for characteristically helpful feedback on a draft of this post. All mistakes are my own.

[1] or not nicely, for that matter.

[2] if you have thought of a better pun, please let me know in the comments. I’ll be annoyed at myself, but I’d love to hear it.

[3] I haven’t.

[4] More accurately, the address of the next contract created by the Lockdrop changes exactly when the Lockdrop's nonce changes, which happens whenever the Lockdrop successfully creates any other contract. But the only contracts the Lockdrop ever creates are theLocks created inside this lock() function.

[5] Josep reminds me that “crypto internet be crazy”, and advises I state the bleeding obvious: I do not have $290 million. But even in the absurdly unjust alternative universe where I do have millions of dollars, they will not be going into a smart contract any time soon.