Merge pull request #846 from onflow/mpeter/guard-against-zero-chain-id-transactions

Fix endpoint crashes due to non-replay protected EVM transactions

Author: m-Peter

eth/types/types.go

@@ -359,10 +359,14 @@ func NewTransaction(
 		V:        (*hexutil.Big)(v),
 		R:        (*hexutil.Big)(r),
 		S:        (*hexutil.Big)(s),
-		ChainID:  (*hexutil.Big)(networkID),
 		size:     tx.Size(),
 	}
 
+	// if a legacy transaction has an EIP-155 chain id, include it explicitly
+	if id := tx.ChainId(); id.Sign() != 0 {
+		result.ChainID = (*hexutil.Big)(id)
+	}
+
 	// After the Pectra hard-fork, the full list of supported tx types is:
 	// LegacyTxType     = 0x00
 	// AccessListTxType = 0x01
@@ -391,6 +395,7 @@ func NewTransaction(
 		yparity := hexutil.Uint64(v.Sign())
 		result.Accesses = &al
 		result.YParity = &yparity
+		result.ChainID = (*hexutil.Big)(tx.ChainId())
 	}
 
 	if tx.Type() > types.AccessListTxType {
@@ -399,17 +404,20 @@ func NewTransaction(
 		// Since BaseFee is `0`, this is the max gas price
 		// the sender is willing to pay.
 		result.GasPrice = (*hexutil.Big)(tx.GasFeeCap())
+		result.ChainID = (*hexutil.Big)(tx.ChainId())
 	}
 
 	if tx.Type() > types.DynamicFeeTxType {
 		result.MaxFeePerBlobGas = (*hexutil.Big)(tx.BlobGasFeeCap())
 		result.BlobVersionedHashes = tx.BlobHashes()
+		result.ChainID = (*hexutil.Big)(tx.ChainId())
 	}
 
 	// The `AuthorizationList` field became available with the introduction
 	// of https://eip7702.io/#specification, under the `SetCodeTxType`
 	if tx.Type() > types.BlobTxType {
 		result.AuthorizationList = tx.SetCodeAuthorizations()
+		result.ChainID = (*hexutil.Big)(tx.ChainId())
 	}
 
 	return result, nil

models/transaction.go

@@ -44,6 +44,7 @@ type Transaction interface {
 	GasFeeCap() *big.Int
 	GasTipCap() *big.Int
 	GasPrice() *big.Int
+	ChainId() *big.Int
 	BlobGas() uint64
 	BlobGasFeeCap() *big.Int
 	BlobHashes() []common.Hash
@@ -117,6 +118,10 @@ func (dc DirectCall) GasPrice() *big.Int {
 	return BaseFeePerGas
 }
 
+func (dc DirectCall) ChainId() *big.Int {
+	return big.NewInt(0)
+}
+
 func (dc DirectCall) BlobGas() uint64 {
 	return 0
 }
@@ -160,10 +165,7 @@ func (tc TransactionCall) Hash() common.Hash {
 }
 
 func (tc TransactionCall) From() (common.Address, error) {
-	return gethTypes.Sender(
-		gethTypes.LatestSignerForChainID(tc.ChainId()),
-		tc.Transaction,
-	)
+	return DeriveTxSender(tc.Transaction)
 }
 
 func (tc TransactionCall) MarshalBinary() ([]byte, error) {
@@ -305,3 +307,22 @@ func ValidateTransaction(
 
 	return nil
 }
+
+// DeriveTxSender returns the address derived from the signature (V, R, S)
+// using secp256k1 elliptic curve and an error if it failed deriving or
+// upon an incorrect signature.
+func DeriveTxSender(tx *gethTypes.Transaction) (common.Address, error) {
+	var signer gethTypes.Signer
+	if chainID := tx.ChainId(); chainID.Sign() != 0 {
+		signer = gethTypes.LatestSignerForChainID(chainID)
+	} else {
+		signer = gethTypes.HomesteadSigner{}
+	}
+
+	from, err := gethTypes.Sender(signer, tx)
+	if err != nil {
+		return common.Address{}, fmt.Errorf("failed to derive the sender: %w", err)
+	}
+
+	return from, nil
+}

services/requester/batch_tx_pool.go

@@ -120,10 +120,11 @@ func (t *BatchTxPool) Add(
 	t.txMux.Lock()
 	defer t.txMux.Unlock()
 
-	from, err := gethTypes.Sender(gethTypes.LatestSignerForChainID(tx.ChainId()), tx)
+	from, err := models.DeriveTxSender(tx)
 	if err != nil {
-		return fmt.Errorf("failed to derive the sender: %w", err)
+		return err
 	}
+
 	txData, err := tx.MarshalBinary()
 	if err != nil {
 		return err

services/requester/requester.go

@@ -208,9 +208,9 @@ func (e *EVM) SendRawTransaction(ctx context.Context, data []byte) (common.Hash,
 		return common.Hash{}, err
 	}
 
-	from, err := types.Sender(types.LatestSignerForChainID(tx.ChainId()), tx)
+	from, err := models.DeriveTxSender(tx)
 	if err != nil {
-		return common.Hash{}, fmt.Errorf("failed to derive the sender: %w", err)
+		return common.Hash{}, err
 	}
 
 	if e.config.TxRequestLimit > 0 {

tests/fixtures/multicall3.byte

@@ -421,7 +421,6 @@ describe('eth_getFilterChanges', async () => {
             transactionIndex: '0x1',
             value: '0x388fb0',
             type: '0x0',
-            chainId: '0x286',
             v: '0xff',
             r: '0x30000000000000000',
             s: '0x3'

tests/web3js/eth_filter_endpoints_test.js

@@ -1,29 +1,71 @@
+const fs = require('fs')
+const utils = require('web3-utils')
 const { assert } = require('chai')
 const conf = require('./config')
 const helpers = require('./helpers')
 const web3 = conf.web3
 
-it('deploy contract and interact', async () => {
-    let deployed = await helpers.deployContract("storage")
+// Address that will deploy the multicall3 contract (must have been pre-funded)
+const MULTICALL3_DEPLOYER = '0x05f32B3cC3888453ff71B01135B34FF8e41263F2'
+
+it('deploys multicall3 contract and interacts', async () => {
+    let deployed = await helpers.deployContract('storage')
     let contractAddress = deployed.receipt.contractAddress
 
     // get the default deployed value on contract
     const initValue = 1337
-    let callRetrieve = await deployed.contract.methods.retrieve().encodeABI()
-    result = await web3.eth.call({ to: contractAddress, data: callRetrieve }, "latest")
+    let callRetrieve = deployed.contract.methods.retrieve().encodeABI()
+    result = await web3.eth.call({ to: contractAddress, data: callRetrieve }, 'latest')
     assert.equal(result, initValue)
 
-    let multicall3 = await helpers.deployContract("multicall3")
-    let multicall3Address = multicall3.receipt.contractAddress
+    // Fund the multicall3 deployer address
+    let transfer = await helpers.signAndSend({
+        from: conf.eoa.address,
+        to: MULTICALL3_DEPLOYER,
+        value: utils.toWei('1.0', 'ether'),
+        gasPrice: conf.minGasPrice,
+        gasLimit: 55_000,
+    })
+    assert.equal(transfer.receipt.status, conf.successStatus)
+
+    // Deploy multicall3 using pre-signed raw transaction
+    // Note: The transaction in multicall3.byte must be signed
+    // by the address defined in the `MULTICALL3_DEPLOYER` const
+    let multicall3DeploymentTx = await fs.promises.readFile(`${__dirname}/../fixtures/multicall3.byte`, 'utf8')
+    let response = await helpers.callRPCMethod(
+        'eth_sendRawTransaction',
+        [multicall3DeploymentTx]
+    )
+    assert.equal(200, response.status)
+    assert.isDefined(response.body.result, 'Transaction hash should be returned')
+
+    let txHash = response.body.result
+
+    // wait for the transaction receipt to become available
+    await new Promise((res, _) => setTimeout(() => res(), 1500))
+    let receipt = await web3.eth.getTransactionReceipt(txHash)
+    let multicall3Address = receipt.contractAddress
 
     // make sure deploy results are correct
-    assert.equal(multicall3.receipt.status, conf.successStatus)
-    assert.isString(multicall3.receipt.transactionHash)
+    assert.equal(receipt.status, conf.successStatus)
+    assert.equal(receipt.from, MULTICALL3_DEPLOYER)
+    assert.isString(receipt.transactionHash)
     assert.isString(multicall3Address)
 
-    let callSum20 = await deployed.contract.methods.sum(10, 10).encodeABI()
-    let callSum50 = await deployed.contract.methods.sum(10, 40).encodeABI()
-    let callAggregate3 = await multicall3.contract.methods.aggregate3(
+    let tx = await web3.eth.getTransaction(txHash)
+    assert.equal(tx.from, MULTICALL3_DEPLOYER)
+    assert.isUndefined(tx.chainId)
+
+    let block = await web3.eth.getBlock(receipt.blockNumber, true)
+    assert.equal(block.transactions[0].from, MULTICALL3_DEPLOYER)
+    assert.isUndefined(block.transactions[0].chainId)
+
+    let multicall3ABI = require('../fixtures/multicall3ABI.json')
+    let multicall3 = new web3.eth.Contract(multicall3ABI, multicall3Address, { handleReverted: true })
+
+    let callSum20 = deployed.contract.methods.sum(10, 10).encodeABI()
+    let callSum50 = deployed.contract.methods.sum(10, 40).encodeABI()
+    let callAggregate3 = multicall3.methods.aggregate3(
         [
             {
                 target: contractAddress,
@@ -43,7 +85,7 @@ it('deploy contract and interact', async () => {
             to: multicall3Address,
             data: callAggregate3
         },
-        "latest"
+        'latest'
     )
     let decodedResult = web3.eth.abi.decodeParameter(
         {