Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

fix: return false rather than panicking on invalid ECDSA signatures #557

Closed
wants to merge 2 commits into from
Closed

fix: return false rather than panicking on invalid ECDSA signatures #557

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e99368
fix: return false rather than panicking on invalid ECDSA signatures
TomAFrench Sep 20, 2023
34a3b8a
chore: fix incorrect test input
TomAFrench Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
241 changes: 178 additions & 63 deletions blackbox_solver/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,27 @@ fn verify_secp256k1_ecdsa_signature(
};
// Convert the inputs into k256 data structures

let signature = Signature::try_from(signature.as_slice()).unwrap();
let Ok(signature) = Signature::try_from(signature.as_slice()) else {
// Signature `r` and `s` are forbidden from being zero.
return false;
};

let point = EncodedPoint::from_affine_coordinates(
public_key_x_bytes.into(),
public_key_y_bytes.into(),
true,
);
let pubkey = PublicKey::from_encoded_point(&point).unwrap();

let pubkey = PublicKey::from_encoded_point(&point);
let pubkey = if pubkey.is_some().into() {
pubkey.unwrap()
} else {
// Public key must sit on the Secp256k1 curve.
return false;
};

// Note: This is incorrect as it will panic if `hashed_msg >= k256::Secp256k1::ORDER`.
// In this scenario we should just take the leftmost bits from `hashed_msg` up to the group order length.
let z = Scalar::from_repr(*GenericArray::from_slice(hashed_msg)).unwrap();

// Finished converting bytes into data structures
Expand Down Expand Up @@ -173,15 +185,27 @@ fn verify_secp256r1_ecdsa_signature(

// Convert the inputs into k256 data structures

let signature = Signature::try_from(signature.as_slice()).unwrap();
let Ok(signature) = Signature::try_from(signature.as_slice()) else {
// Signature `r` and `s` are forbidden from being zero.
return false;
};

let point = EncodedPoint::from_affine_coordinates(
public_key_x_bytes.into(),
public_key_y_bytes.into(),
true,
);
let pubkey = PublicKey::from_encoded_point(&point).unwrap();

let pubkey = PublicKey::from_encoded_point(&point);
let pubkey = if pubkey.is_some().into() {
pubkey.unwrap()
} else {
// Public key must sit on the Secp256r1 curve.
return false;
};

// Note: This is incorrect as it will panic if `hashed_msg >= p256::NistP256::ORDER`.
// In this scenario we should just take the leftmost bits from `hashed_msg` up to the group order length.
let z = Scalar::from_repr(*GenericArray::from_slice(hashed_msg)).unwrap();

// Finished converting bytes into data structures
Expand Down Expand Up @@ -210,75 +234,166 @@ fn verify_secp256r1_ecdsa_signature(
}

#[cfg(test)]
mod test {
use super::{verify_secp256k1_ecdsa_signature, verify_secp256r1_ecdsa_signature};
mod secp256k1_tests {
use super::verify_secp256k1_ecdsa_signature;

// 0x3a73f4123a5cd2121f21cd7e8d358835476949d035d9c2da6806b4633ac8c1e2,
const HASHED_MESSAGE: [u8; 32] = [
0x3a, 0x73, 0xf4, 0x12, 0x3a, 0x5c, 0xd2, 0x12, 0x1f, 0x21, 0xcd, 0x7e, 0x8d, 0x35, 0x88,
0x35, 0x47, 0x69, 0x49, 0xd0, 0x35, 0xd9, 0xc2, 0xda, 0x68, 0x06, 0xb4, 0x63, 0x3a, 0xc8,
0xc1, 0xe2,
];
// 0xa0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7
const PUB_KEY_X: [u8; 32] = [
0xa0, 0x43, 0x4d, 0x9e, 0x47, 0xf3, 0xc8, 0x62, 0x35, 0x47, 0x7c, 0x7b, 0x1a, 0xe6, 0xae,
0x5d, 0x34, 0x42, 0xd4, 0x9b, 0x19, 0x43, 0xc2, 0xb7, 0x52, 0xa6, 0x8e, 0x2a, 0x47, 0xe2,
0x47, 0xc7,
];
// 0x893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7
const PUB_KEY_Y: [u8; 32] = [
0x89, 0x3a, 0xba, 0x42, 0x54, 0x19, 0xbc, 0x27, 0xa3, 0xb6, 0xc7, 0xe6, 0x93, 0xa2, 0x4c,
0x69, 0x6f, 0x79, 0x4c, 0x2e, 0xd8, 0x77, 0xa1, 0x59, 0x3c, 0xbe, 0xe5, 0x3b, 0x03, 0x73,
0x68, 0xd7,
];
// 0xe5081c80ab427dc370346f4a0e31aa2bad8d9798c38061db9ae55a4e8df454fd28119894344e71b78770cc931d61f480ecbb0b89d6eb69690161e49a715fcd55
const SIGNATURE: [u8; 64] = [
0xe5, 0x08, 0x1c, 0x80, 0xab, 0x42, 0x7d, 0xc3, 0x70, 0x34, 0x6f, 0x4a, 0x0e, 0x31, 0xaa,
0x2b, 0xad, 0x8d, 0x97, 0x98, 0xc3, 0x80, 0x61, 0xdb, 0x9a, 0xe5, 0x5a, 0x4e, 0x8d, 0xf4,
0x54, 0xfd, 0x28, 0x11, 0x98, 0x94, 0x34, 0x4e, 0x71, 0xb7, 0x87, 0x70, 0xcc, 0x93, 0x1d,
0x61, 0xf4, 0x80, 0xec, 0xbb, 0x0b, 0x89, 0xd6, 0xeb, 0x69, 0x69, 0x01, 0x61, 0xe4, 0x9a,
0x71, 0x5f, 0xcd, 0x55,
];

#[test]
fn verifies_valid_k1_signature_with_low_s_value() {
// 0x3a73f4123a5cd2121f21cd7e8d358835476949d035d9c2da6806b4633ac8c1e2,
let hashed_message: [u8; 32] = [
0x3a, 0x73, 0xf4, 0x12, 0x3a, 0x5c, 0xd2, 0x12, 0x1f, 0x21, 0xcd, 0x7e, 0x8d, 0x35,
0x88, 0x35, 0x47, 0x69, 0x49, 0xd0, 0x35, 0xd9, 0xc2, 0xda, 0x68, 0x06, 0xb4, 0x63,
0x3a, 0xc8, 0xc1, 0xe2,
];

// 0xa0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7
let pub_key_x: [u8; 32] = [
0xa0, 0x43, 0x4d, 0x9e, 0x47, 0xf3, 0xc8, 0x62, 0x35, 0x47, 0x7c, 0x7b, 0x1a, 0xe6,
0xae, 0x5d, 0x34, 0x42, 0xd4, 0x9b, 0x19, 0x43, 0xc2, 0xb7, 0x52, 0xa6, 0x8e, 0x2a,
0x47, 0xe2, 0x47, 0xc7,
];

// 0x893aba425419bc27a3b6c7e693a24c696f794c2ed877a1593cbee53b037368d7
let pub_key_y: [u8; 32] = [
0x89, 0x3a, 0xba, 0x42, 0x54, 0x19, 0xbc, 0x27, 0xa3, 0xb6, 0xc7, 0xe6, 0x93, 0xa2,
0x4c, 0x69, 0x6f, 0x79, 0x4c, 0x2e, 0xd8, 0x77, 0xa1, 0x59, 0x3c, 0xbe, 0xe5, 0x3b,
0x03, 0x73, 0x68, 0xd7,
];

// 0xe5081c80ab427dc370346f4a0e31aa2bad8d9798c38061db9ae55a4e8df454fd28119894344e71b78770cc931d61f480ecbb0b89d6eb69690161e49a715fcd55
let signature: [u8; 64] = [
0xe5, 0x08, 0x1c, 0x80, 0xab, 0x42, 0x7d, 0xc3, 0x70, 0x34, 0x6f, 0x4a, 0x0e, 0x31,
0xaa, 0x2b, 0xad, 0x8d, 0x97, 0x98, 0xc3, 0x80, 0x61, 0xdb, 0x9a, 0xe5, 0x5a, 0x4e,
0x8d, 0xf4, 0x54, 0xfd, 0x28, 0x11, 0x98, 0x94, 0x34, 0x4e, 0x71, 0xb7, 0x87, 0x70,
0xcc, 0x93, 0x1d, 0x61, 0xf4, 0x80, 0xec, 0xbb, 0x0b, 0x89, 0xd6, 0xeb, 0x69, 0x69,
0x01, 0x61, 0xe4, 0x9a, 0x71, 0x5f, 0xcd, 0x55,
];

fn verifies_valid_signature_with_low_s_value() {
let valid =
verify_secp256k1_ecdsa_signature(&hashed_message, &pub_key_x, &pub_key_y, &signature);
verify_secp256k1_ecdsa_signature(&HASHED_MESSAGE, &PUB_KEY_X, &PUB_KEY_Y, &SIGNATURE);

assert!(valid)
}

#[test]
fn verifies_valid_r1_signature_with_low_s_value() {
// 0x54705ba3baafdbdfba8c5f9a70f7a89bee98d906b53e31074da7baecdc0da9ad
let hashed_message = [
84, 112, 91, 163, 186, 175, 219, 223, 186, 140, 95, 154, 112, 247, 168, 155, 238, 152,
217, 6, 181, 62, 49, 7, 77, 167, 186, 236, 220, 13, 169, 173,
];
// 0x550f471003f3df97c3df506ac797f6721fb1a1fb7b8f6f83d224498a65c88e24
let pub_key_x = [
85, 15, 71, 16, 3, 243, 223, 151, 195, 223, 80, 106, 199, 151, 246, 114, 31, 177, 161,
251, 123, 143, 111, 131, 210, 36, 73, 138, 101, 200, 142, 36,
];
// 0x136093d7012e509a73715cbd0b00a3cc0ff4b5c01b3ffa196ab1fb327036b8e6
let pub_key_y = [
19, 96, 147, 215, 1, 46, 80, 154, 115, 113, 92, 189, 11, 0, 163, 204, 15, 244, 181,
192, 27, 63, 250, 25, 106, 177, 251, 50, 112, 54, 184, 230,
];

// 0x2c70a8d084b62bfc5ce03641caf9f72ad4da8c81bfe6ec9487bb5e1bef62a13218ad9ee29eaf351fdc50f1520c425e9b908a07278b43b0ec7b872778c14e0784
let signature: [u8; 64] = [
44, 112, 168, 208, 132, 182, 43, 252, 92, 224, 54, 65, 202, 249, 247, 42, 212, 218,
140, 129, 191, 230, 236, 148, 135, 187, 94, 27, 239, 98, 161, 50, 24, 173, 158, 226,
158, 175, 53, 31, 220, 80, 241, 82, 12, 66, 94, 155, 144, 138, 7, 39, 139, 67, 176,
236, 123, 135, 39, 120, 193, 78, 7, 132,
];
fn rejects_invalid_signature() {
// This signature is invalid as ECDSA specifies that `r` and `s` must be non-zero.
let invalid_signature: [u8; 64] = [0x00; 64];

let valid = verify_secp256k1_ecdsa_signature(
&HASHED_MESSAGE,
&PUB_KEY_X,
&PUB_KEY_Y,
&invalid_signature,
);
assert!(!valid);
}

#[test]
fn rejects_invalid_public_key() {
let invalid_pub_key_x: [u8; 32] = [0xff; 32];
let invalid_pub_key_y: [u8; 32] = [0xff; 32];

let valid = verify_secp256k1_ecdsa_signature(
&HASHED_MESSAGE,
&invalid_pub_key_x,
&invalid_pub_key_y,
&SIGNATURE,
);

assert!(!valid)
}

#[test]
#[ignore = "ECDSA verification does not currently handle long hashes correctly"]
fn trims_overly_long_hashes_to_correct_length() {
let mut long_hashed_message = HASHED_MESSAGE.to_vec();
long_hashed_message.push(0xff);

let valid = verify_secp256k1_ecdsa_signature(
&long_hashed_message,
&PUB_KEY_X,
&PUB_KEY_Y,
&SIGNATURE,
);

assert!(valid)
}
}

#[cfg(test)]
mod secp256r1_tests {
use super::verify_secp256r1_ecdsa_signature;

// 0x54705ba3baafdbdfba8c5f9a70f7a89bee98d906b53e31074da7baecdc0da9ad
const HASHED_MESSAGE: [u8; 32] = [
84, 112, 91, 163, 186, 175, 219, 223, 186, 140, 95, 154, 112, 247, 168, 155, 238, 152, 217,
6, 181, 62, 49, 7, 77, 167, 186, 236, 220, 13, 169, 173,
];
// 0x550f471003f3df97c3df506ac797f6721fb1a1fb7b8f6f83d224498a65c88e24
const PUB_KEY_X: [u8; 32] = [
85, 15, 71, 16, 3, 243, 223, 151, 195, 223, 80, 106, 199, 151, 246, 114, 31, 177, 161, 251,
123, 143, 111, 131, 210, 36, 73, 138, 101, 200, 142, 36,
];
// 0x136093d7012e509a73715cbd0b00a3cc0ff4b5c01b3ffa196ab1fb327036b8e6
const PUB_KEY_Y: [u8; 32] = [
19, 96, 147, 215, 1, 46, 80, 154, 115, 113, 92, 189, 11, 0, 163, 204, 15, 244, 181, 192,
27, 63, 250, 25, 106, 177, 251, 50, 112, 54, 184, 230,
];
// 0x2c70a8d084b62bfc5ce03641caf9f72ad4da8c81bfe6ec9487bb5e1bef62a13218ad9ee29eaf351fdc50f1520c425e9b908a07278b43b0ec7b872778c14e0784
const SIGNATURE: [u8; 64] = [
44, 112, 168, 208, 132, 182, 43, 252, 92, 224, 54, 65, 202, 249, 247, 42, 212, 218, 140,
129, 191, 230, 236, 148, 135, 187, 94, 27, 239, 98, 161, 50, 24, 173, 158, 226, 158, 175,
53, 31, 220, 80, 241, 82, 12, 66, 94, 155, 144, 138, 7, 39, 139, 67, 176, 236, 123, 135,
39, 120, 193, 78, 7, 132,
];

#[test]
fn verifies_valid_signature_with_low_s_value() {
let valid =
verify_secp256r1_ecdsa_signature(&hashed_message, &pub_key_x, &pub_key_y, &signature);
verify_secp256r1_ecdsa_signature(&HASHED_MESSAGE, &PUB_KEY_X, &PUB_KEY_Y, &SIGNATURE);

assert!(valid)
}

#[test]
fn rejects_invalid_signature() {
// This signature is invalid as ECDSA specifies that `r` and `s` must be non-zero.
let invalid_signature: [u8; 64] = [0x00; 64];

let valid = verify_secp256r1_ecdsa_signature(
&HASHED_MESSAGE,
&PUB_KEY_X,
&PUB_KEY_Y,
&invalid_signature,
);
assert!(!valid);
}

#[test]
fn rejects_invalid_public_key() {
let invalid_pub_key_x: [u8; 32] = [0xff; 32];
let invalid_pub_key_y: [u8; 32] = [0xff; 32];

let valid = verify_secp256r1_ecdsa_signature(
&HASHED_MESSAGE,
&invalid_pub_key_x,
&invalid_pub_key_y,
&SIGNATURE,
);

assert!(!valid)
}

#[test]
#[ignore = "ECDSA verification does not currently handle long hashes correctly"]
fn trims_overly_long_hashes_to_correct_length() {
let mut long_hashed_message = HASHED_MESSAGE.to_vec();
long_hashed_message.push(0xff);

let valid = verify_secp256r1_ecdsa_signature(
&long_hashed_message,
&PUB_KEY_X,
&PUB_KEY_Y,
&SIGNATURE,
);

assert!(valid)
}
Expand Down