From 5bd1931c8ee344e3d61405a70f3813f7c4e6cc26 Mon Sep 17 00:00:00 2001
From: David Murdoch <david@davidmurdoch.com>
Date: Mon, 14 Mar 2022 18:53:20 -0400
Subject: [PATCH] enforce eip-2 for impersonated account signatures

---
 src/chains/ethereum/ethereum/src/wallet.ts    |  5 ++--
 .../transaction/src/transaction-factory.ts    | 27 ++++++++++++++++---
 src/packages/secp256k1/index.ts               |  3 +++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/src/chains/ethereum/ethereum/src/wallet.ts b/src/chains/ethereum/ethereum/src/wallet.ts
index 31167f6fca..d90298f267 100644
--- a/src/chains/ethereum/ethereum/src/wallet.ts
+++ b/src/chains/ethereum/ethereum/src/wallet.ts
@@ -14,7 +14,7 @@ import {
   WEI
 } from "@ganache/utils";
 import { privateToAddress } from "ethereumjs-util";
-import secp256k1 from "@ganache/secp256k1";
+import secp256k1, { SECP256K1_MAX_PRIVATE_KEY } from "@ganache/secp256k1";
 import { mnemonicToSeedSync } from "bip39";
 import { alea } from "seedrandom";
 import crypto from "crypto";
@@ -24,7 +24,6 @@ import { EthereumInternalOptions } from "@ganache/ethereum-options";
 import { Address } from "@ganache/ethereum-address";
 
 const TWELVE_255s = Buffer.allocUnsafe(12).fill(255);
-const SECP256K1_MAX_PRIVATE_KEY = 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141n;
 
 //#region Constants
 const SCRYPT_PARAMS = {
@@ -663,7 +662,7 @@ export default class Wallet {
       // will. There are obviously many chances for a false positive, but the
       // next condition in the `while` loop will catch those.
       if (first12.compare(TWELVE_255s) === 0) {
-        while (BigInt(fakePrivateKey.toString()) >= SECP256K1_MAX_PRIVATE_KEY) {
+        while (BigInt(`0x${fakePrivateKey.toString("hex")}`) >= SECP256K1_MAX_PRIVATE_KEY) {
           // keccak returns a 32 byte hash of the input data, which is the exact
           // length we need for a private key.
           // note: if keccak can return it's own input as it's output, then this
diff --git a/src/chains/ethereum/transaction/src/transaction-factory.ts b/src/chains/ethereum/transaction/src/transaction-factory.ts
index 2b61c6be50..bee364065e 100644
--- a/src/chains/ethereum/transaction/src/transaction-factory.ts
+++ b/src/chains/ethereum/transaction/src/transaction-factory.ts
@@ -20,6 +20,20 @@ import { decode } from "@ganache/rlp";
 import { CodedError } from "@ganache/ethereum-utils";
 import { TypedTransaction } from "./transaction-types";
 import { EIP1559FeeMarketTransaction } from "./eip1559-fee-market-transaction";
+import { SECP256K1_MAX_PRIVATE_KEY_DIV_2 } from "@ganache/secp256k1";
+
+/**
+ * @param common 
+ * @param tx 
+ * @throws
+ */
+function assertValidTransactionSValue(common: Common, tx: LegacyTransaction | EIP2930AccessListTransaction | EIP1559FeeMarketTransaction) {
+  // Transaction signatures whose s-value is greater than secp256k1n/2 are
+  // invalid after the homestead hardfork. See: https://eips.ethereum.org/EIPS/eip-2
+  if (tx.s && tx.s.toBigInt() > SECP256K1_MAX_PRIVATE_KEY_DIV_2 && common.gteHardfork('homestead')) {
+    throw new Error("Invalid Signature: s-values greater than secp256k1n/2 are considered invalid")
+  }
+}
 
 const UNTYPED_TX_START_BYTE = 0xc0; // all txs with first byte >= 0xc0 are untyped
 
@@ -162,7 +176,9 @@ export class TransactionFactory {
   ) {
     const txType = this.typeOfRPC(txData);
 
-    return this._fromData(txData, txType, common, extra);
+    const tx = this._fromData(txData, txType, common, extra);
+    assertValidTransactionSValue(common, tx);
+    return tx;
   }
   /**
    * Create a transaction from a `txData` object
@@ -218,6 +234,7 @@ export class TransactionFactory {
     let data = Data.from(txData).toBuffer();
     const type = data[0];
     const txType = this.typeOf(type);
+    let tx: LegacyTransaction | EIP2930AccessListTransaction | EIP1559FeeMarketTransaction;
     if (common.isActivatedEIP(2718)) {
       let raw: TypedDatabasePayload;
       try {
@@ -227,7 +244,7 @@ export class TransactionFactory {
       } catch (e: any) {
         throw new Error("Could not decode transaction: " + e.message);
       }
-      return this._fromData(raw, txType, common);
+      tx = this._fromData(raw, txType, common);
     } else {
       let raw: TypedDatabasePayload;
       try {
@@ -235,8 +252,12 @@ export class TransactionFactory {
       } catch (e: any) {
         throw new Error("Could not decode transaction: " + e.message);
       }
-      return this._fromData(raw, TransactionType.Legacy, common);
+      tx = this._fromData(raw, TransactionType.Legacy, common);
     }
+
+    assertValidTransactionSValue(common, tx);
+
+    return tx;
   }
 
   private static typeOf(type: number) {
diff --git a/src/packages/secp256k1/index.ts b/src/packages/secp256k1/index.ts
index 0b27f0d48d..6c31a84c12 100644
--- a/src/packages/secp256k1/index.ts
+++ b/src/packages/secp256k1/index.ts
@@ -7,6 +7,9 @@
 
 import { dirname } from "path";
 
+export const SECP256K1_MAX_PRIVATE_KEY = 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141n;
+export const SECP256K1_MAX_PRIVATE_KEY_DIV_2 = SECP256K1_MAX_PRIVATE_KEY / 2n;
+
 let secp256k1: {
   ecdsaRecover: (
     output: Uint8Array,