From 198532018692aeb7b5eab60a73567a405e9aaa5c Mon Sep 17 00:00:00 2001
From: Afri <58883403+q9f@users.noreply.github.com>
Date: Mon, 31 Oct 2022 13:46:39 +0100
Subject: [PATCH] eth/signature: fix allowing ledger v values of 0 (#155)

---
 lib/eth/chain.rb           |  9 +++++++++
 lib/eth/signature.rb       |  5 ++++-
 spec/eth/chain_spec.rb     | 11 +++++++++++
 spec/eth/signature_spec.rb | 10 +++++++---
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/lib/eth/chain.rb b/lib/eth/chain.rb
index 5d0afa60..24850ebf 100644
--- a/lib/eth/chain.rb
+++ b/lib/eth/chain.rb
@@ -92,6 +92,15 @@ class ReplayProtectionError < StandardError; end
     # Chain ID for the geth private network preset.
     PRIVATE_GETH = 1337.freeze
 
+    # Indicates wether the given `v` indicates a legacy chain value
+    # used by ledger wallets without EIP-155 replay protection.
+    #
+    # @param v [Integer] the signature's `v` value.
+    # @return [Boolean] true if ledger'#'s legacy value.
+    def is_ledger?(v)
+      [0, 1].include? v
+    end
+
     # Indicates wether the given `v` indicates a legacy chain value
     # without EIP-155 replay protection.
     #
diff --git a/lib/eth/signature.rb b/lib/eth/signature.rb
index f80e6d4d..1fb67433 100644
--- a/lib/eth/signature.rb
+++ b/lib/eth/signature.rb
@@ -70,7 +70,10 @@ def recover(blob, signature, chain_id = Chain::ETHEREUM)
       context = Secp256k1::Context.new
       r, s, v = dissect signature
       v = v.to_i(16)
-      raise SignatureError, "Invalid signature v byte #{v} for chain ID #{chain_id}!" if v < chain_id
+      if !Chain.is_ledger? v and !Chain.is_legacy? v
+        min_v = 2 * chain_id + 35
+        raise SignatureError, "Invalid signature v byte #{v} for chain ID #{chain_id}!" if v < min_v
+      end
       recovery_id = Chain.to_recovery_id v, chain_id
       signature_rs = Util.hex_to_bin "#{r}#{s}"
       recoverable_signature = context.recoverable_signature_from_compact signature_rs, recovery_id
diff --git a/spec/eth/chain_spec.rb b/spec/eth/chain_spec.rb
index 290878ca..de4e811c 100644
--- a/spec/eth/chain_spec.rb
+++ b/spec/eth/chain_spec.rb
@@ -34,6 +34,17 @@
     end
   end
 
+  describe ".is_ledger" do
+    it "can detect ledger values for v" do
+      expect(Chain.is_ledger? 0).to be_truthy
+      expect(Chain.is_ledger? 1).to be_truthy
+      expect(Chain.is_ledger? 27).not_to be_truthy
+      expect(Chain.is_ledger? 28).not_to be_truthy
+      expect(Chain.is_ledger? 37).not_to be_truthy
+      expect(Chain.is_ledger? 38).not_to be_truthy
+    end
+  end
+
   describe ".is_legacy" do
     it "can detect legacy values for v" do
       expect(Chain.is_legacy? 0).not_to be_truthy
diff --git a/spec/eth/signature_spec.rb b/spec/eth/signature_spec.rb
index 508a2425..ba027f42 100644
--- a/spec/eth/signature_spec.rb
+++ b/spec/eth/signature_spec.rb
@@ -110,6 +110,10 @@
 
     it "can recover a public key from a signature generated with ledger" do
       message = "test"
+      signature = "0x5c433983b23738940ce256c59d5bc6a3d5fd12c5bc9bdbf0ffdffb7be1a09d1815ca1db167c61a10945837f3fb4821086d6656b4fa6ede9c4d1aeaf07e2b0adf00"
+      public_hex = "044f8f3af2e1f0106544ae3d2d08d78ca8b68d258fcd8065dc193aefb7ccf1210d65044101488e8d68dbdfadb5adc2044b21fd60e1513bca65bcfc4e36927766d9"
+      expect(Signature.personal_recover message, signature).to eq public_hex
+
       signature = "0x5c433983b23738940ce256c59d5bc6a3d5fd12c5bc9bdbf0ffdffb7be1a09d1815ca1db167c61a10945837f3fb4821086d6656b4fa6ede9c4d1aeaf07e2b0adf01"
       public_hex = "04e51ff5abc511f2fda0f893c10054123e92527b5e69e24cca538e74edbd604508259e1b265b54628bc8024fb791e459f67adb770b20962eb38fabe8b86f2aebaa"
       expect(Signature.personal_recover message, signature).to eq public_hex
@@ -130,9 +134,9 @@
 
     it "raises argument errors if signature is invalid" do
       message = "This is proof that I, user A, have access to this address."
-      signature_invalid_v = "0x4e1ce8ea60bc6dfd4068a35462612495850cb645a1c9f475eb969bff21d0b0fb414112aaf13f01dd18a3527cb648cdd51b618ae49d4999112c33f86b7b26e97300"
+      signature_invalid_v = "0x4e1ce8ea60bc6dfd4068a35462612495850cb645a1c9f475eb969bff21d0b0fb414112aaf13f01dd18a3527cb648cdd51b618ae49d4999112c33f86b7b26e97302"
       signature_invalid_size = "0x4e1ce8ea60bc6dfd4068a35462612495850cb645a1c9f475eb969bff21d0b0fb414112aaf13f01dd18a3527cb648cdd51b618ae49d4999112c33f86b7b26e973"
-      expect { Signature.personal_recover(message, signature_invalid_v) }.to raise_error Signature::SignatureError, "Invalid signature v byte 0 for chain ID 1!"
+      expect { Signature.personal_recover(message, signature_invalid_v) }.to raise_error Signature::SignatureError, "Invalid signature v byte 2 for chain ID 1!"
       expect { Signature.personal_recover(message, signature_invalid_size) }.to raise_error Signature::SignatureError, "Unknown signature length 128!"
     end
   end
@@ -167,7 +171,7 @@
     it "raises argument errors if signature is invalid" do
       signature_invalid_v = "f6cda8eaf5137e8cc15d48d03a002b0512446e2a7acbc576c01cfbe40ad9345663ccda8884520d98dece9a8bfe38102851bdae7f69b3d8612b9808e63378016024"
       signature_invalid_size = "f6cda8eaf5137e8cc15d48d03a002b0512446e2a7acbc576c01cfbe40ad9345663ccda8884520d98dece9a8bfe38102851bdae7f69b3d8612b9808e6337801602"
-      expect { Signature.recover_typed_data(test_data, signature_invalid_v) }.to raise_error Chain::ReplayProtectionError, "Invalid v 36 value for chain ID 1. Invalid chain ID?"
+      expect { Signature.recover_typed_data(test_data, signature_invalid_v) }.to raise_error Signature::SignatureError, "Invalid signature v byte 36 for chain ID 1!"
       expect { Signature.recover_typed_data(test_data, signature_invalid_size) }.to raise_error Signature::SignatureError, "Unknown signature length 129!"
     end
   end