Quick Context

• Web3 Company, ThirdWeb announced critical vulnerability in open source smart contracts - (12/04/2023)

• ThirdWeb offers SDK for smart contract development, thus vulnerabilities potentially impact many popular assets

ThirdWeb's announcement came with an unsettling implication. There could've been any number of digital assets at risk throughout Ethereum's ecosystem. If attackers discovered the critical vulnerability in any popular applications, one could imagine billions of user funds stolen at any moment.

I was hoping to uncover this vulnerability myself. What I discovered instead was a broader dilemma facing all open source software projects and their users.

The Announcement

At the time of initial disclosure, ThirdWeb opted to give sparse details about the vulnerability.

So, I distilled a few key details from the announcement to the following:

1. ThirdWeb learns of the vulnerability on November 20th, 2023 6pm PST,

2. It impacts pre-built smart contracts created before November 22nd, 2023 at 7pm PT

3. Some affected contract types include DropERC20, ERC721, ERC1155 (all versions), and AirdropERC20

Bullet #2 implied a fix was already published. A quick search showed that ThirdWeb hosted their smart contracts in a public repository on Github called "Contracts".

Easy, with this I went straight for the commit history.

Repository Analysis

I was looking for a commit that would lead me toward the undisclosed vulnerability. After a quick scan I thought to myself,

"That's odd.."

Here's what I saw:

Aside from "Add audit reports 14-15", I didn't see anything that would imply a vulnerability fix. I started my search with the audit reports on the off chance a postmortem was "soft" published without an official announcement. After having skimmed through the reports, I saw nothing indicating the vulnerability type and magnitude I was looking for. Plus, the reports were dated back to months prior.

Doubling back to the commit history, I considered the one labeled "Revert account factory..", but according to their announcement, ThirdWeb first became aware on Nov. 20th at 6pm PT. Upon inspecting the time stamp for commits on Nov. 20th, I noticed "Revert account factory..." was committed at 2:25pm PT. The only commits that fell within the timeline provided were titled as a dependency upgrade and minor clean up, and an updated gas benchmark; neither sounded promising.

Had they lied, or simply made a mistake? I didn't think so. I wanted to first assume that ThirdWeb was careful to give accurate information about the timeline given the criticality of reputation in the digital asset security space. Nevertheless, I clicked through a few of the older commits just in case. I noticed that many of them contained a nice summary of each update made with high level context.

I thought, "With that kind of detail it should be easy to spot a vulnerability patch".

Exactly!

I considered that if ThirdWeb was intentional about withholding details concerning the vulnerability itself, at least until their customers had time to remediate, then they might've also been careful to avoid plainly pushing a patch that would tip off those lurking about their repo looking for clues (not unlike myself).

A Second Look

Taking a step back, this highlighted a real challenge inherent to open source software. If you maintain a public repository, and catch wind of a vulnerability, then how might you limit exposing information surrounding its details that could put users at risk, while ensuring a timely fix is available?

With this in mind, I looked through the innocuous commits next. The first, "Update Gas Benchmark", was unmistakably unremarkable.

"Okay, no luck on this one"

I proceeded to tap the second commit, "Upgrade dependencies and minor clean up". The commit description was true to the title. Nothing notable.

The number of "changed files", however, was another story..

Where better to hide a critical fix than among +300 updated files, primarily consisting linter/format changes like this one?

As I scrolled down the changed files, this pattern continued. I jumped to where an update was made to the ERC1155 contract in a directory called "pre-built", and noticed more of the same - except for one change I nearly missed.

Line 39 shows a change away from an OpenZepplin dependency to one within ThirdWeb's own repository. Line 57 shows a change from this ERC1155 Contract extending the OpenZepplin MulticallUpgradable to ThirdWeb's Multicall. This could imply a significant change to the behavior of the contract, unlike what the commit description suggested.

Things were starting to get interesting.

I pulled up OpenZepplin's repository, and compared their MulticalUpgradeable with ThirdWeb's MultiCall.

The obvious change was that multicall would no longer be an upgradable method, but it still wasn't clear to me why that mattered. It was getting late, so I made a note to checkout the import on line 6, then put my phone away. As I drifted to sleep, I thought of how changing MultiCall's upgradablility might be connected to the vulnerability. I was excited that I might soon solve this puzzle!

Disclosure

The next day came, and I never got the chance. On December 7th, Dedaub, a security auditor dropped an article detailing the specifics of the vulnerability. It turned out that the issue did involve MultiCall; specifically with how attackers could call it on contracts with methods that inherited/implemented ERC2771 functionality in order to spoof their address.

Admittedly, I was a bit disappointed having not discovered more myself, but there was still the question of how ThirdWeb covertly patched this flaw. With upgradability as a component being out the window, I turned to investigating ThirdWeb's Address.sol import from their MultiCall contract.

It's here that the patch turned up.

Now, if you’re already familiar the definitions below (or don’t care for the details) skip ahead. Else, read the following as they are per-requisites for understanding the vulnerability:

The key detail to this vulnerability is that a trusted forwarder contract acts as a middle-man for the transaction. As a consequence, we want our recipient contract to think of the message sender as the user account that initiated the flow rather than the trusted forwarder itself. To do this, a ERC2771 supported method may see if it was called by a trusted forwarder contract. If so, then it will interpret the last 20 bytes of call data as the message sender (an encoding done by the trusted forwarder contract).

When looking at the original Address.sol from OpenZepplin (see bellow), I could see that functionDelegateCall works by calling delegatecall on the target contract (line 105). This was problematic in the case of ERC2771.

When an account makes a gasless transaction for invoking MultiCall, any ERC2771 supported methods will interpret the last 20 bytes of data as the initiator of the gasless transaction as expected. However, MultiCall will make a series of delegate calls using the bytes of data set by the initiator of the gasless transaction (as intended). The problem here is that while delegatecall uses this user account defined data, msg.sender is still the trusted forwader contract - which means those ERC2771 methods will interpret the message sender based on data specified as the initiator of the gasless transaction. In other words, this code allows for address spoofing.

All Patched Up

The new functionDelegateCall has an additional check to determine if the msg.sender, is a contract, and reverts the transaction if that is the case (line 95). While it means a contract based wallet or any other well intending contract code would be unable to use MultiCall to save on gas, it does seem to patch this vulnerability since trusted forwarders are contracts.

In the end, ThirdWeb reported that many of its customers were able to successfully remediate their applications without incident. The situation also surfaced a question that may impact doomsday prep for organizations, companies, and individuals that maintain, or plan to maintain open source repositories. In the event of a critical vulnerability, is there a full proof method to keep key vulnerability details away from attackers while providing patched software for users? Even if that’s infeasible, a conversation around trade offs and a decision on a response would certainly be prudent.

Thanks for reading!