Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions mm2src/coins/coin_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub enum ValidatePaymentError {
TimelockOverflow(TryFromIntError),
ProtocolNotSupported(String),
InvalidData(String),
CheckSignatureError(String),
}

impl From<SPVError> for ValidatePaymentError {
Expand Down
60 changes: 44 additions & 16 deletions mm2src/coins/utxo/utxo_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2152,20 +2152,26 @@ fn verify_p2pk_input_pubkey(
index: usize,
signature_version: SignatureVersion,
fork_id: u32,
) -> Result<bool, String> {
) -> Result<bool, ValidatePaymentError> {
// Extract the signature from the scriptSig.
let signature = script.extract_signature()?;
let signature = script
.extract_signature()
.map_err(|e| ValidatePaymentError::CheckSignatureError(format!("Signature parsing error: {}", e)))?;
// Validate the signature.
try_s!(SecpSignature::from_der(&signature[..signature.len() - 1]));
SecpSignature::from_der(&signature[..signature.len().saturating_sub(1)])
.map_err(|e| ValidatePaymentError::CheckSignatureError(format!("Signature parsing error: {}", e)))?;
let signature = signature.into();
// Make sure we have no more instructions. P2PK scriptSigs consist of a single instruction only containing the signature.
if script.get_instruction(1).is_some() {
return ERR!("Unexpected instruction at position 2 of script {:?}", script);
return Err(ValidatePaymentError::TxDeserializationError(format!(
"Unexpected instruction at position 2 of script {:?}",
script
)));
};
// Get the scriptPub for this input. We need it to get the transaction sig_hash to sign (but actually "to verify" in this case).
let pubkey = expected_pubkey
.to_secp256k1_pubkey()
.map_err(|e| ERRL!("Error converting plain pubkey to secp256k1 pubkey: {}", e))?;
.map_err(|e| ValidatePaymentError::InvalidData(format!("Parsing expected pubkey error: {}", e)))?;
// P2PK scriptPub has two valid possible formats depending on whether the public key is written in compressed or uncompressed form.
let possible_pubkey_scripts = [
Builder::build_p2pk(&Public::Compressed(pubkey.serialize().into())),
Expand All @@ -2182,14 +2188,24 @@ fn verify_p2pk_input_pubkey(
fork_id,
) {
Ok(hash) => hash,
Err(e) => return ERR!("Error calculating signature hash: {}", e),
Err(e) => {
return Err(ValidatePaymentError::CheckSignatureError(format!(
"Error calculating signature hash: {}",
e
)))
},
};
// Verify that the signature is valid for the transaction hash with respect to the expected public key.
return match expected_pubkey.verify(&hash, &signature) {
Ok(true) => Ok(true),
// The signature is invalid for this pubkey, try the other possible pubkey script.
Ok(false) => continue,
Err(e) => ERR!("Error verifying signature: {}", e),
Err(e) => {
return Err(ValidatePaymentError::CheckSignatureError(format!(
"Error verifying signature: {}",
e
)))
},
};
}

Expand All @@ -2202,15 +2218,12 @@ fn pubkey_from_script_sig(script: &Script) -> Result<H264, String> {
// Extract the signature from the scriptSig.
let signature = script.extract_signature()?;
// Validate the signature.
try_s!(SecpSignature::from_der(&signature[..signature.len() - 1]));
try_s!(SecpSignature::from_der(&signature[..signature.len().saturating_sub(1)]));

let pubkey = match script.get_instruction(1) {
Some(Ok(instruction)) => match instruction.opcode {
Opcode::OP_PUSHBYTES_33 => match instruction.data {
Some(bytes) => try_s!(PublicKey::from_slice(bytes)),
None => return ERR!("No data at instruction 1 of script {:?}", script),
},
_ => return ERR!("Unexpected opcode {:?}", instruction.opcode),
Some(Ok(instruction)) => match instruction.data {
Some(bytes) => try_s!(PublicKey::from_slice(bytes)),
Comment thread
mariocynicys marked this conversation as resolved.
None => return ERR!("No data at instruction 1 of script {:?}", script),
},
Some(Err(e)) => return ERR!("Error {} on getting instruction 1 of script {:?}", e, script),
None => return ERR!("None instruction 1 of script {:?}", script),
Expand Down Expand Up @@ -2298,8 +2311,7 @@ pub async fn check_all_utxo_inputs_signed_by_pub<T: UtxoCommonOps>(
idx,
coin.as_ref().conf.signature_version,
coin.as_ref().conf.fork_id,
)
.map_to_mm(ValidatePaymentError::TxDeserializationError)?;
)?;
if successful_verification {
// No pubkey extraction for P2PK inputs. Continue.
continue;
Expand Down Expand Up @@ -5674,6 +5686,22 @@ fn test_pubkey_from_script_sig() {
pubkey_from_script_sig(&script_sig_err).unwrap_err();
}

#[test]
fn test_pubkey_from_axe_script_sig() {
let script_sig = Script::from("45304202205fa91d3dc0c88b1b0c2b5ecdf08b49c0458b6f10ff6b758b82c1934210f367fc021e51a96cf672048a44fef3256ba9a061b408f842b6b523624c28d6b5bbd1680121023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289");
let expected_pub = H264::from("023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289");
let actual_pub = pubkey_from_script_sig(&script_sig).unwrap();
assert_eq!(expected_pub, actual_pub);
}

#[test]
fn test_pubkey_from_empty_script_sig() {
let script_sig = Script::from("");
assert!(pubkey_from_script_sig(&script_sig).is_err());
let script_sig = Script::from("00");
assert!(pubkey_from_script_sig(&script_sig).is_err());
}

#[test]
fn test_verify_p2pk_input_pubkey() {
// 65-byte (uncompressed) pubkey example.
Expand Down
9 changes: 3 additions & 6 deletions mm2src/mm2_bitcoin/script/src/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,9 @@ impl Script {
/// Usable for P2PK and P2PKH scripts.
pub fn extract_signature(&self) -> Result<Vec<u8>, String> {
match self.get_instruction(0) {
Some(Ok(instruction)) => match instruction.opcode {
Opcode::OP_PUSHBYTES_70 | Opcode::OP_PUSHBYTES_71 | Opcode::OP_PUSHBYTES_72 => match instruction.data {
Some(bytes) => Ok(bytes.to_vec()),
None => Err(format!("No data at instruction 0 of script {self:?}")),
},
opcode => Err(format!("Unexpected opcode {opcode:?}")),
Some(Ok(instruction)) => match instruction.data {
Some(bytes) if !bytes.is_empty() => Ok(bytes.to_vec()),
Comment thread
mariocynicys marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's not do the if !is_empty() check to be consistent with the error messages like the pubkey extraction counterpart.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have at least one byte in this data because we will send a slice of len-1 of it to SecpSignature::from_der fn.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh. nice catch. this -1 seems a lil bit dangerous. maybe we could use .saturating_sub in this situation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to saturating_sub, thanks

Some(_) | None => Err(format!("No data at instruction 0 of script {self:?}")),
},
Some(Err(e)) => Err(format!("Error {e} on getting instruction 0 of script {self:?}")),
None => Err(format!("None instruction 0 of script {self:?}")),
Expand Down
Loading