12/13/2021 sudoswap Outage Postmortem

NOTICE: The 0x v2 Exchange contracts are FINE, and the below details an issue (now fixed) with the EtherOrcs contract.

Issue

At 7:42 pm Pacific Time, I was alerted on Discord to the following transaction: https://etherscan.io/tx/0xc33d0c598053cdac9ca2b623b3a1f9b5adc7ef4ac5e60a471535a01a49dfecee

A user had intended to use sudoswap to swap their EtherOrc NFT for WETH. Instead, a generalized frontrunner bot had somehow filled the order instead. The NFT was transferred to the maker, but the bot received the WETH instead of the original NFT owner.

This wasn't a transaction I'd seen before, so I alerted the rest of the sudoswap team. We made the hasty decision to pause swap creation while we investigated what had happened. I also reached out to the 0x team to alert them of a possible issue.

The 0x team responded within 2 minutes, escalated internally, and stayed with us until the issue was resolved (ultimately it had nothing to do with the 0x contracts).

After around half an hour, boredGenius discovered that the EtherOrc NFT contract does not adhere to one part of the ERC721 standard. Specifically, when calling transferFrom(address from, address to, uint256 id), the from address is not checked to see if it owns the NFT with the specified id.

The 0x team quickly confirmed this was what was at fault.

When filling an order, the 0x Exchange contract will attempt to call transferFrom from one party to the other, in order to send both sides their assets. The situation here is that a frontrunning bot can spot a pending fillOrder transaction in the mempool and rebroadcast it themselves. Then, the 0x Exchange will attempt to transfer the NFT from the bot (who doesn't have it) to the other party. This should be expected to fail as the bot does not own the NFT. However, the non-standard behavior of the EtherOrcs transferFrom would still succeed (as from is not checked), transferring the NFT from its original owner to the other party, but crediting the bot as the from address.

The end result is that the bot receives the assets on the other side, but the NFT is still transferred out of the original owner's wallet.

Remediation

The user frontran has been compensated for their swap amount.

The EtherOrcs team has upgraded their transferFrom function to check if the address specified in from is the owner, which remedies this non-standard behavior.

NOTE: Other contracts using the ERC721 contract found here (https://github.com/lexDAO/Kali/blob/main/contracts/tokens/erc721/ERC721.sol) are also susceptible to this issue. LexDAO has been notified, and it has been patched in this PR: https://github.com/lexDAO/Kali/commit/546ed74c53511dc7074760de6ebca93667bd598f.

As a user, if you are trying to swap one of these NFT assets, please consider using something like the flashbots rpc (https://docs.flashbots.net/flashbots-protect/rpc/quick-start) to avoid being frontrun when swapping on sudoswap. (If the NFT's logic is not upgradeable).

On our end at sudoswap, we now enforce filling a takerAddress by default if an NFT is on the Have side of swap creation, which is set to be the NFT's current owner. (Users are free to change this if they want the default "anyone can fill" behavior). Specifying that the swap is only for the NFT owner also removes this frontrunning vector for non-standard ERC721s.

Improvements

As the first (what seemed to be) serious incident at sudoswap, I believe I (0xmons) handled several things improperly.

First, I acted too hastily. Upon receiving the initial report, I could have taken more time with our team to understand what had happened. The 0x Exchange contracts have been live on mainnet for years without issue. Had I taken a step back, I could have better thought about what other areas (e.g. the transferred assets, our own signature creation flow) might have also been at fault. It was rash of me to single them out as a possible vector for the issue, especially given their track record and the fact that they were ultimately blameless.

Along the same lines, I believe I took to public lines of communication (e.g. Twitter, Discord) far too early, and while perhaps well-intentioned, this did more harm than good, as we hadn't fully understood the issue at the time. This led to some quick whiplash when we did identify the problem less than an hour later (as something different than what we first guessed).

In the future, I want to be better about opening private lines of communication with the protocols we work with. When possible, I'll start with smaller escalations–this time, I assumed the worst case without fully understanding the situation, and this led to additional fallout and concern. Many thanks again to the 0x team for their assistance and support in fully resolving this issue.

Apologies again for any issues caused by our interruption in service, and I hope this clarifies the incident.