Skip to content

Commit

Permalink
eth/signature: fix allowing ledger v values of 0 (#155)
Browse files Browse the repository at this point in the history
  • Loading branch information
q9f authored Oct 31, 2022
1 parent 6848cec commit 1985320
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
9 changes: 9 additions & 0 deletions lib/eth/chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
5 changes: 4 additions & 1 deletion lib/eth/signature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions spec/eth/chain_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions spec/eth/signature_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1985320

Please sign in to comment.