diff --git a/crates/cashu/src/nuts/nut10/spending_conditions.rs b/crates/cashu/src/nuts/nut10/spending_conditions.rs index 72835f4c00..8c4fc4ba32 100644 --- a/crates/cashu/src/nuts/nut10/spending_conditions.rs +++ b/crates/cashu/src/nuts/nut10/spending_conditions.rs @@ -110,7 +110,9 @@ impl TryFrom for SpendingConditions { conditions: secret .secret_data() .tags() - .and_then(|t| t.clone().try_into().ok()), + .cloned() + .map(Conditions::try_from) + .transpose()?, }), Kind::HTLC => Ok(Self::HTLCConditions { data: Sha256Hash::from_str(secret.secret_data().data()) @@ -118,7 +120,9 @@ impl TryFrom for SpendingConditions { conditions: secret .secret_data() .tags() - .and_then(|t| t.clone().try_into().ok()), + .cloned() + .map(Conditions::try_from) + .transpose()?, }), } } @@ -142,6 +146,7 @@ impl From for super::Secret { impl TryFrom for Secret { type Error = Error; fn try_from(conditions: SpendingConditions) -> Result { + conditions.validate()?; let secret: Nut10Secret = conditions.into(); Secret::try_from(secret) } @@ -176,6 +181,61 @@ pub struct Conditions { } impl Conditions { + fn validate(&self, primary_key_count: u64) -> Result<(), Error> { + if let Some(n) = self.num_sigs { + if n == 0 { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + + let available_keys = + primary_key_count + self.pubkeys.as_ref().map(Vec::len).unwrap_or(0) as u64; + if n > available_keys { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleMultisigConfiguration { + required: n, + available: available_keys, + }, + )); + } + } + + match (&self.refund_keys, self.num_sigs_refund) { + (Some(_), Some(0)) => { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + (Some(refund_keys), Some(required)) if required > refund_keys.len() as u64 => { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required, + available: refund_keys.len() as u64, + }, + )); + } + (None, Some(0)) => { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + (None, Some(required)) => { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required, + available: 0, + }, + )); + } + (Some(refund_keys), None) if refund_keys.is_empty() => { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required: 1, + available: 0, + }, + )); + } + _ => {} + } + + Ok(()) + } + /// Create new Spending [`Conditions`] pub fn new( locktime: Option, @@ -192,40 +252,39 @@ impl Conditions { ); } - if let Some(n) = num_sigs { - let available_keys = 1 + pubkeys.as_ref().map(Vec::len).unwrap_or(0); - if n > available_keys as u64 { - return Err(Error::NUT11( - crate::nut11::Error::ImpossibleMultisigConfiguration { - required: n, - available: available_keys as u64, - }, - )); - } - } - - if let Some(n) = num_sigs_refund { - let refund_key_count = refund_keys.as_ref().map(Vec::len).unwrap_or(0); - if n > refund_key_count as u64 { - return Err(Error::NUT11( - crate::nut11::Error::ImpossibleMultisigConfiguration { - required: n, - available: refund_key_count as u64, - }, - )); - } - } - - Ok(Self { + let conditions = Self { locktime, pubkeys, refund_keys, num_sigs, sig_flag: sig_flag.unwrap_or_default(), num_sigs_refund, - }) + }; + conditions.validate(1)?; + + Ok(conditions) } } + +impl SpendingConditions { + fn validate(&self) -> Result<(), Error> { + match self { + Self::P2PKConditions { conditions, .. } => { + if let Some(conditions) = conditions { + conditions.validate(1)?; + } + } + Self::HTLCConditions { conditions, .. } => { + if let Some(conditions) = conditions { + conditions.validate(0)?; + } + } + } + + Ok(()) + } +} + impl From for Vec> { fn from(conditions: Conditions) -> Vec> { let Conditions { @@ -251,7 +310,7 @@ impl From for Vec> { tags.push(Tag::NSigs(num_sigs).as_vec()); } - if let Some(refund_keys) = refund_keys { + if let Some(refund_keys) = refund_keys.filter(|keys| !keys.is_empty()) { tags.push(Tag::Refund(refund_keys).as_vec()) } @@ -311,6 +370,32 @@ impl TryFrom>> for Conditions { } } + if let Some(0) = num_sigs { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + if let Some(0) = num_sigs_refund { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + + if let Some(refund_keys) = &refund_keys { + let required = num_sigs_refund.unwrap_or(1); + if required > refund_keys.len() as u64 { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required, + available: refund_keys.len() as u64, + }, + )); + } + } else if let Some(required) = num_sigs_refund { + return Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required, + available: 0, + }, + )); + } + Ok(Conditions { locktime, pubkeys, @@ -362,4 +447,84 @@ mod tests { Some(vec![PublicKey::from_str(pk1).unwrap()]) ); } + + #[test] + fn test_empty_refund_tag_is_rejected() { + let tags = vec![ + vec!["refund".to_string()], + vec!["sigflag".to_string(), "SIG_INPUTS".to_string()], + ]; + + let result = Conditions::try_from(tags); + + assert!(matches!( + result, + Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required: 1, + available: 0 + } + )) + )); + } + + #[test] + fn test_empty_refund_keys_are_not_serialized() { + let conditions = Conditions { + locktime: Some(1), + pubkeys: None, + refund_keys: Some(vec![]), + num_sigs: None, + sig_flag: crate::SigFlag::default(), + num_sigs_refund: None, + }; + + let tags = Vec::>::from(conditions); + + assert!(!tags + .iter() + .any(|tag| tag.first() == Some(&"refund".to_string()))); + } + + #[test] + fn test_n_sigs_refund_without_refund_keys_is_rejected() { + let tags = vec![ + vec!["n_sigs_refund".to_string(), "1".to_string()], + vec!["sigflag".to_string(), "SIG_INPUTS".to_string()], + ]; + + let result = Conditions::try_from(tags); + + assert!(matches!( + result, + Err(Error::NUT11( + crate::nut11::Error::ImpossibleRefundMultisigConfiguration { + required: 1, + available: 0 + } + )) + )); + } + + #[test] + fn test_spending_conditions_try_from_propagates_invalid_tags() { + let pubkey = PublicKey::from_str( + "026562efcfadc8e86d44da6a8adf80633d974302e62c850774db1fb36ff4cc7198", + ) + .unwrap(); + let nut10_secret = Nut10Secret::new( + Kind::P2PK, + crate::nuts::nut10::SecretData::new( + pubkey.to_string(), + Some(vec![vec!["n_sigs".to_string(), "0".to_string()]]), + ), + ); + + let result = SpendingConditions::try_from(nut10_secret); + + assert!(matches!( + result, + Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)) + )); + } } diff --git a/crates/cashu/src/nuts/nut10/tag.rs b/crates/cashu/src/nuts/nut10/tag.rs index cd344499bb..8a4d8a051e 100644 --- a/crates/cashu/src/nuts/nut10/tag.rs +++ b/crates/cashu/src/nuts/nut10/tag.rs @@ -116,12 +116,17 @@ where TagKind::SigFlag => Ok(Tag::SigFlag(SigFlag::from_str( tag.get(1).ok_or(Error::TagValueNotFound)?.as_ref(), )?)), - TagKind::NSigs => Ok(Tag::NSigs( - tag.get(1) + TagKind::NSigs => { + let sigs = tag + .get(1) .ok_or(Error::TagValueNotFound)? .as_ref() - .parse()?, - )), + .parse()?; + if sigs == 0 { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + Ok(Tag::NSigs(sigs)) + } TagKind::Locktime => Ok(Tag::LockTime( tag.get(1) .ok_or(Error::TagValueNotFound)? @@ -146,12 +151,17 @@ where Ok(Self::PubKeys(pubkeys)) } - TagKind::NSigsRefund => Ok(Tag::NSigsRefund( - tag.get(1) + TagKind::NSigsRefund => { + let sigs = tag + .get(1) .ok_or(Error::TagValueNotFound)? .as_ref() - .parse()?, - )), + .parse()?; + if sigs == 0 { + return Err(Error::NUT11(crate::nut11::Error::ZeroSignaturesRequired)); + } + Ok(Tag::NSigsRefund(sigs)) + } TagKind::Custom(name) => { let tags = tag .iter() diff --git a/crates/cashu/src/nuts/nut11/mod.rs b/crates/cashu/src/nuts/nut11/mod.rs index 26b7165d6d..766628a9e7 100644 --- a/crates/cashu/src/nuts/nut11/mod.rs +++ b/crates/cashu/src/nuts/nut11/mod.rs @@ -82,6 +82,9 @@ pub enum Error { /// SIG_ALL not supported in this context #[error("SIG_ALL proofs must be verified using a different method")] SigAllNotSupportedHere, + /// Number of required signatures cannot be zero + #[error("Number of required signatures must be 1 or greater")] + ZeroSignaturesRequired, /// Serde Json error #[error(transparent)] SerdeJsonError(#[from] serde_json::Error), @@ -2178,7 +2181,7 @@ mod tests { assert!( matches!( err, - crate::nut10::Error::NUT11(Error::ImpossibleMultisigConfiguration { + crate::nut10::Error::NUT11(Error::ImpossibleRefundMultisigConfiguration { required: 3, available: 1, }) @@ -2222,7 +2225,7 @@ mod tests { assert!( matches!( err, - crate::nut10::Error::NUT11(Error::ImpossibleMultisigConfiguration { + crate::nut10::Error::NUT11(Error::ImpossibleRefundMultisigConfiguration { required: 1, available: 0, }) @@ -2264,4 +2267,150 @@ mod tests { ); assert!(result.is_err(), "4-of-3 should be impossible"); } + + #[test] + fn test_p2pk_num_sigs_zero_bypasses_signature_requirement() { + // n_sigs=0 makes verify_p2pk() evaluate `valid_sig_count >= required_sigs` + // as `0 >= 0` which is trivially true. An empty P2PKWitness therefore + // passes verification with no cryptographic check at all. + // + // Per bkb's record of NUT-11: n_sigs must be "a positive integer". + // n_sigs=0 is out-of-spec and produces an anyone-can-spend P2PK proof. + // + // FAILS on HEAD (verify_p2pk returns Ok). PASSES once fixed. + let data_pubkey = PublicKey::from_str( + "026562efcfadc8e86d44da6a8adf80633d974302e62c850774db1fb36ff4cc7198", + ) + .unwrap(); + + let conditions = Conditions { + locktime: None, + pubkeys: None, + refund_keys: None, + num_sigs: Some(0), // invalid: not a positive integer per NUT-11 + sig_flag: SigFlag::SigInputs, + num_sigs_refund: None, + }; + + let result: Result = + SpendingConditions::new_p2pk(data_pubkey, Some(conditions)).try_into(); + + assert!(matches!( + result, + Err(crate::nut10::Error::NUT11(Error::ZeroSignaturesRequired)) + )); + } + + #[test] + fn test_sig_all_p2pk_nsigs_zero_bypasses_sig_check() { + let data_pubkey = PublicKey::from_str( + "026562efcfadc8e86d44da6a8adf80633d974302e62c850774db1fb36ff4cc7198", + ) + .unwrap(); + let extra_pubkey = PublicKey::from_str( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(); + + // Note: We bypass Conditions::new() by constructing it directly, + // to test the low-level verify routine. But because we implemented + // ZeroSignaturesRequired inside TryFrom for Conditions too, + // Secret::try_into() will fail on building the conditions! + // To make the test actually exercise verify_sig_all_p2pk on a "bad" Secret, + // we have to assert that either creation fails or verify_sig_all_p2pk fails. + // Given our fix prevents creation entirely, the test should check that. + let nut10_secret = Nut10Secret::new( + crate::nuts::nut10::Kind::P2PK, + crate::nuts::nut10::SecretData::new( + data_pubkey.to_string(), + Some(vec![ + vec!["pubkeys".to_string(), extra_pubkey.to_string()], + vec!["n_sigs".to_string(), "0".to_string()], + vec!["sigflag".to_string(), "SIG_ALL".to_string()], + ]), + ), + ); + let conditions_res = crate::nuts::nut10::Conditions::try_from( + nut10_secret.secret_data().tags().cloned().unwrap(), + ); + assert!( + conditions_res.is_err(), + "Conditions should fail to parse due to n_sigs=0" + ); + } + + #[test] + fn test_refund_path_num_sigs_refund_zero_bypasses_signature_check() { + // When n_sigs_refund=Some(0) and refund_keys are present, + // get_pubkeys_and_required_sigs sets required_sigs=0 for the RefundPath. + // In verify_p2pk the guard + // if refund_path.required_sigs == 0 { return Ok(()); } + // fires once locktime has passed, letting anyone spend with an empty + // witness and no valid refund-key signature. + // + // FAILS on HEAD (verify_p2pk returns Ok). PASSES once the fix is applied. + + let data_pubkey = PublicKey::from_str( + "026562efcfadc8e86d44da6a8adf80633d974302e62c850774db1fb36ff4cc7198", + ) + .unwrap(); + + let refund_pubkey = PublicKey::from_str( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(); + + let nut10_secret = Nut10Secret::new( + crate::nuts::nut10::Kind::P2PK, + crate::nuts::nut10::SecretData::new( + data_pubkey.to_string(), + Some(vec![ + vec!["locktime".to_string(), "1".to_string()], + vec!["refund".to_string(), refund_pubkey.to_string()], + vec!["n_sigs".to_string(), "1".to_string()], + vec!["n_sigs_refund".to_string(), "0".to_string()], + ]), + ), + ); + let conditions_res = crate::nuts::nut10::Conditions::try_from( + nut10_secret.secret_data().tags().cloned().unwrap(), + ); + assert!( + conditions_res.is_err(), + "Conditions should fail to parse due to n_sigs=0" + ); + } + + #[test] + fn test_sig_all_p2pk_nsigs_refund_zero_bypasses_refund_sig_check() { + let data_pubkey = PublicKey::from_str( + "026562efcfadc8e86d44da6a8adf80633d974302e62c850774db1fb36ff4cc7198", + ) + .unwrap(); + let refund_pubkey = PublicKey::from_str( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(); + + let nut10_secret = Nut10Secret::new( + crate::nuts::nut10::Kind::P2PK, + crate::nuts::nut10::SecretData::new( + data_pubkey.to_string(), + Some(vec![ + vec!["locktime".to_string(), "1".to_string()], + vec!["refund".to_string(), refund_pubkey.to_string()], + vec!["n_sigs".to_string(), "1".to_string()], + vec!["n_sigs_refund".to_string(), "0".to_string()], + vec!["sigflag".to_string(), "SIG_ALL".to_string()], + ]), + ), + ); + let conditions_res = crate::nuts::nut10::Conditions::try_from( + nut10_secret.secret_data().tags().cloned().unwrap(), + ); + assert!( + conditions_res.is_err(), + "Conditions should fail to parse due to n_sigs_refund=0" + ); + } } diff --git a/crates/cashu/src/nuts/nut14/mod.rs b/crates/cashu/src/nuts/nut14/mod.rs index b9d5877ce1..41fee8a35c 100644 --- a/crates/cashu/src/nuts/nut14/mod.rs +++ b/crates/cashu/src/nuts/nut14/mod.rs @@ -527,6 +527,63 @@ mod tests { assert!(matches!(result.unwrap_err(), Error::InvalidHash)); } + #[test] + fn test_htlc_num_sigs_zero_bypasses_signature_requirement() { + let pubkey = crate::nuts::nut01::PublicKey::from_hex( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(); + + let preimage_bytes = [42u8; 32]; + let hash = Sha256Hash::hash(&preimage_bytes); + let hash_str = hash.to_string(); + + let tags = vec![ + vec!["pubkeys".to_string(), pubkey.to_string()], + vec!["n_sigs".to_string(), "0".to_string()], + ]; + + let nut10_secret = Nut10Secret::new(Kind::HTLC, SecretData::new(hash_str, Some(tags))); + // Since we patched deserialization itself (TryFrom), let's verify that + // extracting the conditions out of a constructed secret will error + let conditions_res = crate::nuts::nut10::Conditions::try_from( + nut10_secret.secret_data().tags().cloned().unwrap(), + ); + assert!( + conditions_res.is_err(), + "Conditions should fail to parse due to n_sigs=0" + ); + } + + #[test] + fn test_verify_sig_all_htlc_nsigs_zero_bypasses_sig_check() { + let preimage_bytes = [42u8; 32]; + let hash = Sha256Hash::hash(&preimage_bytes); + let hash_str = hash.to_string(); + + let required_pubkey = crate::nuts::nut01::PublicKey::from_hex( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(); + + // SIG_ALL HTLC with pubkeys but n_sigs=0 + let tags = vec![ + vec!["pubkeys".to_string(), required_pubkey.to_string()], + vec!["n_sigs".to_string(), "0".to_string()], + vec!["sigflag".to_string(), "SIG_ALL".to_string()], + ]; + + let nut10_secret = Nut10Secret::new(Kind::HTLC, SecretData::new(hash_str, Some(tags))); + + let conditions_res = crate::nuts::nut10::Conditions::try_from( + nut10_secret.secret_data().tags().cloned().unwrap(), + ); + assert!( + conditions_res.is_err(), + "Conditions should fail to parse due to n_sigs=0" + ); + } + /// Tests that verify_htlc correctly rejects an HTLC with the wrong witness type. /// /// This test ensures that the verification function checks that the witness is @@ -691,4 +748,84 @@ mod tests { "Should fail when using refund path with refund keys but no signature" ); } + + #[test] + fn test_htlc_generated_empty_refund_keys_are_omitted() { + use crate::nuts::nut10::Conditions; + + let preimage_bytes = [42u8; 32]; + let hash = Sha256Hash::hash(&preimage_bytes); + let hash_str = hash.to_string(); + + let conditions = Conditions { + locktime: Some(1), + pubkeys: None, + refund_keys: Some(vec![]), + num_sigs: None, + sig_flag: crate::nuts::nut11::SigFlag::default(), + num_sigs_refund: None, + }; + + let nut10_secret = + Nut10Secret::new(Kind::HTLC, SecretData::new(hash_str, Some(conditions))); + let secret: SecretString = nut10_secret.try_into().unwrap(); + + let htlc_witness = HTLCWitness { + preimage: hex::encode([0xffu8; 32]), + signatures: None, + }; + + let proof = Proof { + amount: crate::Amount::from(1), + keyset_id: crate::nuts::nut02::Id::from_str("00deadbeef123456").unwrap(), + secret, + c: crate::nuts::nut01::PublicKey::from_hex( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(), + witness: Some(Witness::HTLCWitness(htlc_witness)), + dleq: None, + p2pk_e: None, + }; + + assert!(proof.verify_htlc().is_ok()); + } + + #[test] + fn test_htlc_empty_refund_tag_is_rejected() { + let preimage_bytes = [42u8; 32]; + let hash = Sha256Hash::hash(&preimage_bytes); + let hash_str = hash.to_string(); + + let tags = vec![ + vec!["locktime".to_string(), "1".to_string()], + vec!["refund".to_string()], + ]; + + let nut10_secret = Nut10Secret::new(Kind::HTLC, SecretData::new(hash_str, Some(tags))); + let secret: SecretString = nut10_secret.try_into().unwrap(); + + let htlc_witness = HTLCWitness { + preimage: hex::encode([0xffu8; 32]), + signatures: None, + }; + + let proof = Proof { + amount: crate::Amount::from(1), + keyset_id: crate::nuts::nut02::Id::from_str("00deadbeef123456").unwrap(), + secret, + c: crate::nuts::nut01::PublicKey::from_hex( + "02a9acc1e48c25eeeb9289b5031cc57da9fe72f3fe2861d264bdc074209b107ba2", + ) + .unwrap(), + witness: Some(Witness::HTLCWitness(htlc_witness)), + dleq: None, + p2pk_e: None, + }; + + assert!(matches!( + proof.verify_htlc(), + Err(Error::SpendConditionsNotMet) + )); + } }