ethers.js icon indicating copy to clipboard operation
ethers.js copied to clipboard

"TypeError: non-canonical s" for valid transaction

Open ethissue opened this issue 2 years ago • 7 comments

Ethers Version

6.6.2

Search Terms

signature, yParity

Describe the Problem

A valid transaction on the Ethereum mainnet is not parseable by ethers V6. The same code snippet below will run perfectly fine with ethers v5.7.2.

Also, go-ethereum crashes on this TX in particular as well, will open an issue on their repo tomorrow too unless we figure out what's happening.

Worth noting that the TX right before and right after parse just fine!

Code Snippet

#!/bin/sh

# SET TMP PROJECT PATH
TMP_PATH=/tmp/ethissue

# SET VERSION TO BE TESTED (5.7.2 WORKS!)
VERSION="6.6.2"

# CREATE CLEAN TMP PROJECT WITH VERSION TO BE TESTED
rm -rf $TMP_PATH
mkdir -p $TMP_PATH
cd $TMP_PATH
npm init -y
npm install ethers@$VERSION

# WRITE TEST SCRIPT
cat <<EOF > "${TMP_PATH}/index.js"
const ethers = require('ethers');

const provider = ethers.getDefaultProvider();

(async function(){
    let txHash;

    //txHash = '0x6d837b8496d4ae6bfad610e6647cc66faa2d1f3676f2fca24174c1c7cc1bb233'; // block height 100,019
    txHash = '0xce8a2163e2563c7fc783371985fff9fd8194d7e27c4be1adb06520ca2e816e9a'; // block height 100,023
    //txHash = '0x92fd0b59813c1ed4ee485414a9545edfb701fc1e81ed252e00484895637c4a3f'; // block height 100,031

    const tx = await provider.getTransaction(txHash);

    console.log(tx);
})();
EOF

# RUN AND INSPECT OUTPUT
node "${TMP_PATH}/index.js"

Contract ABI

No response

Errors

TypeError: non-canonical s (argument="signature" ...)
{
  ...
  code: 'INVALID_ARGUMENT',
  argument: 'signature',
  ...
}

Environment

Ethereum (mainnet/ropsten/rinkeby/goerli), node.js (v12 or newer)

Environment (Other)

No response

ethissue avatar Jul 07 '23 23:07 ethissue

Opened an issue at go-ethereum as well. Exact same outcome can be reproduced with the very same transactions.

ethissue avatar Jul 13 '23 03:07 ethissue

FWIW, the issue is that this particular tx is signed with the Frontier rules, where 2 signatures were valid (same tx diff hash), which was forbidden in Homestead (https://eips.ethereum.org/EIPS/eip-2)

Allowing transactions with any s value with 0 < s < secp256k1n, as is currently the case, opens a transaction malleability concern, as one can take any transaction, flip the s value from s to secp256k1n - s, flip the v value (27 -> 28, 28 -> 27), and the resulting signature would still be valid. This is not a serious security flaw, especially since Ethereum uses addresses and not transaction hashes as the input to an ether value transfer or other transaction, but it nevertheless creates a UI inconvenience as an attacker can cause the transaction that gets confirmed in a block to have a different hash from the transaction that any user sends, interfering with user interfaces that use transaction hashes as tracking IDs. Preventing high s values removes this problem.

To recover the signature of Frontier txs (at least the weird flipped ones), you need a Frontier signer, not a Homestead one. I'm unsure how this is done in Ethers.js, in go-ethereum you can explicitly construct such a signer.

PS:

go-ethereum crashes on this TX in particular

go-ethereum does not crash, it returns an error, which is correct given that you tried to validate the signatures with Homestead rules, where those are invalid

karalabe avatar Jul 13 '23 06:07 karalabe

FWIW, the issue is that this particular tx is signed with the Frontier rules, where 2 signatures were valid (same tx diff hash), which was forbidden in Homestead (https://eips.ethereum.org/EIPS/eip-2)

Allowing transactions with any s value with 0 < s < secp256k1n, as is currently the case, opens a transaction malleability concern, as one can take any transaction, flip the s value from s to secp256k1n - s, flip the v value (27 -> 28, 28 -> 27), and the resulting signature would still be valid. This is not a serious security flaw, especially since Ethereum uses addresses and not transaction hashes as the input to an ether value transfer or other transaction, but it nevertheless creates a UI inconvenience as an attacker can cause the transaction that gets confirmed in a block to have a different hash from the transaction that any user sends, interfering with user interfaces that use transaction hashes as tracking IDs. Preventing high s values removes this problem.

To recover the signature of Frontier txs (at least the weird flipped ones), you need a Frontier signer, not a Homestead one. I'm unsure how this is done in Ethers.js, in go-ethereum you can explicitly construct such a signer.

PS:

go-ethereum crashes on this TX in particular

go-ethereum does not crash, it returns an error, which is correct given that you tried to validate the signatures with Homestead rules, where those are invalid

Thank you for the insight @karalabe .

By crashing I didn't mean that the go-ethereum does an unclean exit, but that it produces an error when used intuitively. Like if you use go-ethereum to fetch block #100,023 and then parse its transactions then it will fail with an error.

Same is true currently for ethers.js .

ethissue avatar Jul 13 '23 22:07 ethissue

Thanks @karalabe. I didn’t realize there were frontier blocks since launch; I thought that was a pre-launch things.

This change will require a minor bump, and a few non-trivial changes:

  • I’ll add an option to Signature to allow nonCanonicalS (by default it will be false)
  • Add a read-only property to the Signature class to indicate whether the S is non-canonical
  • Add a ProviderPlugin to allow a Network to indicate nonCanonicalS range
  • Add that plugin by default to the Mainnet Network with the correct block number
  • on detection of that plugin, pass that along to the Formatter to allow non-canonical S when parsing the Signature

I think that is all that is necessary… :)

ricmoo avatar Jul 17 '23 03:07 ricmoo

Thanks @karalabe. I didn’t realize there were frontier blocks since launch; I thought that was a pre-launch things.

This change will require a minor bump, and a few non-trivial changes:

  • I’ll add an option to Signature to allow nonCanonicalS (by default it will be false)
  • Add a read-only property to the Signature class to indicate whether the S is non-canonical
  • Add a ProviderPlugin to allow a Network to indicate nonCanonicalS range
  • Add that plugin by default to the Mainnet Network with the correct block number
  • on detection of that plugin, pass that along to the Formatter to allow non-canonical S when parsing the Signature

I think that is all that is necessary… :)

Sounds like a well structured solution, thank you for your great work as always @ricmoo !

ethissue avatar Jul 26 '23 20:07 ethissue

Hi !

I also get this error when I try to fetch the 1000110 block with the Infura provider. Any progress on the issue?

mahsumurebe avatar Aug 21 '23 21:08 mahsumurebe

is this still being worked on? also seeing this for 62162 on 6.12.1

mz- avatar May 08 '24 20:05 mz-