Sep 25, 2023, I found an issue in a widely used implementation of the ERC1271 pattern that affected 15+ teams. This blog describes the sequence of events regarding how it all came together. If you are a wallet team or a security researcher, consider reading this. Severity of the issue is subjective to the wallet's use case, ranging from High to Medium.

• Chapter 1 - Spark

• Chapter 2 - Discussions: Who’s responsible, Wallets or External Applications ?

• Chapter 3 - Search and Report

• Chapter 4 - Fix, Learnings, Shoutouts and Bounties

Chapter 1: Spark

Sep 25: I was looking into Cowswap and found that they don't include the user address in their order struct, unlike most other intent-based projects such as Uniswap X.

Thinking more on this, I realised that this could lead to smart contract signatures being replayed between multiple accounts if those accounts have a common owner.

If you are unaware, smart contract signatures are implemented by ERC1271, where a caller calls a smart contract and the smart contract checks if the given digest was signed by one of the owners.

Reference Implementation from EIP

function isValidSignature(
    bytes32 _hash,
    bytes calldata _signature
) external override view returns (bytes4) {
    // Validate signatures
    if (recoverSigner(_hash, _signature) == owner) {
      return 0x1626ba7e;
    } else {
      return 0xffffffff;
    }
}

Now, what if the given signed digest (_hash) is independent of the smart contract account?

Can a smart contract signature signed by a smart account with owner A be replayed on any other smart account owned by A? More broadly, can a signature signed by EOA A be replayed on a smart account owned by A?

It turns out the answer is YES if the signed digest doesn't include the smart account address. That's the crux of the issue.

You can find the initial report to Cow here https://gist.github.com/0xcuriousapple/f68f63ab25f463f8f9fb5759209ab497
Since they don't include the signer (user address) inside the order struct and thereby in the signed digest, CowSwap orders could be replayed for smart contract accounts having the same owners. For Cow, it's more severe since their nonce handling is unique. Basically, they mark orders as filled or not based on the order hash. Therefore, for this to occur on Cow, you don't even need nonces to be in sync.

The Cow team was diligent throughout the report and agreed that it exists. They assured that they will monitor such orders on their own and start hashing the user address as an extra parameter moving forward.

Note that not all wallets were vulnerable to this line of attack.

Some wallets, like Safe or Sequence, hash the account address on their own, nullifying this attack vector.

Hence, the Cow team also pointed out that ideally, wallets should handle this like a SAFE.

Chapter 2 : Discussions: Who’s responsible, Wallets or External Applications ?

Initially, I thought that this was a Cowswap-related issue, so I only raised it with them. But the argument that wallets should ideally handle this was gaining traction.

Out of all the wallets I could index, only Safe, Sequence, and BlocTo were the non-vulnerable ones.

I also found that there are other protocols like CowSwap that expect wallets to handle this case. For example, Uniswap's Permit 2, Arcade, Lens and ETH Attestation.

Hence, now I was unsure whose responsibility this really is? wallets or external applications ?

The Cow team put up a conversation with the Safe team, who pointed out that ideally wallets should handle this and not rely on the example implementation of ERC1271.

I consulted with some other relevant parties, including the authors of ERC4337, other application developers, and Francisco Giordano (@frangio) (one of the authors of ERC1271).

The opinions differed on whose responsibility this is, but in the end, there was a majority consensus that wallets should handle this.

The main argument for this being a wallets issue is that, from the external application's point of view, this case creates a distinction between smart accounts and EOAs.

An external application should have measures in place to prevent signature replay in general cases, such as chain.id. However, in the case of a common owner of smart accounts, it should ideally be blocked at the source, i.e. in wallets code, since it's specific to them.

Frangio also took on the task of making the necessary amendments in the official EIP itself and has been leading conversations there 🫡

However, it's been nearly 3 months, and it has been lagging due to multiple points of view. Issue in the EIP reference implementation sparked a wider discussion about security disclosures for EIPs, which remains unresolved. You can follow its progress here

Moving ahead, I decided to report it to all affected wallets

Chapter 3 : Search and Report

It was time for the endgame: finding and reporting to all affected wallets. I used all tools at my disposal:

• Alchemy's list of wallets

• GitHub

• Codeslaw

• TinTin's sanctuary

For days, my schedule was searching for isValidSignature implementations, determining if it was affected, finding contact information, writing a custom issue report for each team, and report.

Personal rant: It was pure pain, solely due to the nature of this issue. Multiple teams were affected, and there was no easy way to reach all of them at once. It was a widely adopted implementation, with thousands of instances on direct search. Therefore, you had to be creative and persistent with your searches. Moreover, finding a vulnerable implementation was just the beginning; finding contact information and establishing communication was a bigger problem.

In parallel, Howy, an engineer at Alchemy, also found this issue around October 27th on his own for their LightAccount and reported it to Uniswap first due to its wider use. You can find his proof of concept for Permit 2 here: https://github.com/omgwiNNING/replay-sig-poc
He created a Telegram group and added the Uniswap team and affected wallets. Since I was in touch with the affected wallets, I also joined the group. The wallet community and its members have been having discussions there since that point onwards.

The group is pretty dormant now, but if you would like to join, reach out to me or Howy.

When it was all said and done, the following were the wallets that accepted the issue.

1. Ambire

2. ZeroDev

3. Biconomy (Resolved from V2 onwards, and is still present in V1)

4. Soul Wallet

5. Etherspot

6. Openfort

7. Thirdweb

8. Unipass

9. Instadapp

10. Enso

11. OKX

12. Light Account (Alchemy)

13. Hats Wallet

14. Fuse

15. Solady

Ambire's report for reference: Ambire's Report

There were some more wallets to above list, 5-6 if I recall correctly. Some discarded the issue, some never responded, while others clarified why it doesn't impact them, like Argent, for example. Even though Argent uses the same reference implementation, this doesn't apply to them because the signer of the account in their case cannot be used separately as an EOA. It's generated on the device and not exposed to users.

Chapter 4: Fix, Learnings, Shoutouts and Bounties

Fix:

This issue could be fixed by adding the account address inside the signed digest. You can refer to Safe's implementation for the solution.

However, in the direct implementation like Safe, you will find that it results in opaque signatures for users.

Hence, we (Ivo, Frangio, Vectorised) have been having discussions to solve it in a way that doesn't result in opaque signatures for users (such that they can verify them on their hardware wallets).

Both Frangio and Vectorised have put together their solutions on their github.

• frangio/eip712-wrapper-for-eip1271

• Vectorized/solady pull request #688

Note that none of the above has been audited as of now and is just for reference.

Learnings:

1. Regarding EIP Reference Implementations : The broader question here is: Should you consider EIP reference implementations secure by default or not? Should EIP authors take responsibility and have their implementations audited? Or should they leave out the reference implementation part altogether, given the lack of a standard disclosure process and the inability to make amendments?

   For now, you shouldn't rely on and copy reference EIP implementations as they are. They are only provided for reference and have not been audited.

2. Regarding Signatures: Be cautious about signatures. They can be tricky and will always remain tricky simply because someone has the ability to execute them on your behalf. Any leak there and you're done.

3. Please, please include your security contact information in your deployed contract source. This will make it easier for anyone to reach out to you.

Shoutout to the people who helped during this, in no particular order:

• Frangio (Co-author of ERC1271), @frangio_

• Ivo (Ambire), @Ivshti

• Felix (Cow), @fleupold

• Taek (Zerodev), @leekt216

• Vectorised (Solady), @optimizoor

• Howy (Alchemy), @howydev

• and many others

Shoutout to the teams who paid bounty for this, in no particular order:

• Ambire (Immunefi)

• Instadapp (Immunefi)

• Biconomy (Their Own)

• Cow (Immunefi)

Thanks,
@0xcuriousapple