From 911dc0c1534eea0570f5e940df670a6a6489d794 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Wed, 2 Sep 2026 18:37:43 +0530 Subject: [PATCH 1/3] scanner: reject duplicate BIP21 parameter keys (#63) --- src/modules/scanner/implementation.rs | 23 +++++++++++------------ src/modules/scanner/tests.rs | 13 ++++++++++++- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/modules/scanner/implementation.rs b/src/modules/scanner/implementation.rs index 72e47a19..f9c65f86 100644 --- a/src/modules/scanner/implementation.rs +++ b/src/modules/scanner/implementation.rs @@ -317,18 +317,17 @@ impl Scanner { let address = parts[0].to_string(); - let params = if parts.len() > 1 { - parts[1] - .split('&') - .filter_map(|param| { - param - .split_once('=') - .map(|(k, v)| (k.to_string(), v.to_string())) - }) - .collect::>() - } else { - HashMap::new() - }; + let mut params = HashMap::new(); + if parts.len() > 1 { + for param in parts[1].split('&') { + if let Some((k, v)) = param.split_once('=') { + if params.contains_key(k) { + return Err(DecodingError::InvalidFormat); + } + params.insert(k.to_string(), v.to_string()); + } + } + } let amount_satoshis = params .get("amount") diff --git a/src/modules/scanner/tests.rs b/src/modules/scanner/tests.rs index ea18f1f3..62092fd3 100644 --- a/src/modules/scanner/tests.rs +++ b/src/modules/scanner/tests.rs @@ -199,8 +199,19 @@ mod tests { } #[tokio::test] - async fn test_invalid_lightning_invoice() { + fn test_invalid_lightning_invoice_sync() { let invoice = "lnbc1invalid".to_string(); + assert!(matches!( + Scanner::decode_onchain(&invoice), + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_bip21_params_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.000035&amount=0.00005" + .to_string(); assert!(matches!( Scanner::decode(invoice).await, Err(DecodingError::InvalidFormat) From 27b3d92105561f93afe59efacadc55ba8affbadc Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 24 Sep 2026 14:53:16 +0530 Subject: [PATCH 2/3] fix(scanner): enforce BIP321 singleton duplicate policy and reject concatenated URIs (#63) Signed-off-by: Tyagiquamar --- src/modules/scanner/implementation.rs | 57 ++++++++++++--- src/modules/scanner/tests.rs | 101 +++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/src/modules/scanner/implementation.rs b/src/modules/scanner/implementation.rs index f9c65f86..06ffa32e 100644 --- a/src/modules/scanner/implementation.rs +++ b/src/modules/scanner/implementation.rs @@ -309,23 +309,58 @@ impl Scanner { } fn decode_onchain(invoice_str: &str) -> Result { - let parts: Vec<&str> = invoice_str - .strip_prefix("bitcoin:") - .unwrap_or(invoice_str) - .split('?') - .collect(); + let without_prefix = + if invoice_str.len() >= 8 && invoice_str[..8].eq_ignore_ascii_case("bitcoin:") { + &invoice_str[8..] + } else { + invoice_str + }; + + // Reject if there is a second "bitcoin:" URI prefix anywhere in the remainder (issue #63) + if without_prefix.to_ascii_lowercase().contains("bitcoin:") { + return Err(DecodingError::InvalidFormat); + } + + let (address_part, query_part) = match without_prefix.split_once('?') { + Some((addr, query)) => { + // If there's an additional '?' character in the query, it represents malformed/concatenated URIs + if query.contains('?') { + return Err(DecodingError::InvalidFormat); + } + (addr, Some(query)) + } + None => (without_prefix, None), + }; - let address = parts[0].to_string(); + let address = address_part.to_string(); let mut params = HashMap::new(); - if parts.len() > 1 { - for param in parts[1].split('&') { - if let Some((k, v)) = param.split_once('=') { - if params.contains_key(k) { + let mut seen_singletons = std::collections::HashSet::new(); + + if let Some(query) = query_part { + for param in query.split('&') { + if param.is_empty() { + continue; + } + let (k, v) = match param.split_once('=') { + Some((key, val)) => (key, val), + None => (param, ""), + }; + + let lower_k = k.to_ascii_lowercase(); + let is_singleton = matches!( + lower_k.as_str(), + "amount" | "label" | "message" | "pop" | "req-pop" + ); + + if is_singleton { + if seen_singletons.contains(&lower_k) { return Err(DecodingError::InvalidFormat); } - params.insert(k.to_string(), v.to_string()); + seen_singletons.insert(lower_k.clone()); } + + params.insert(lower_k, v.to_string()); } } diff --git a/src/modules/scanner/tests.rs b/src/modules/scanner/tests.rs index 62092fd3..44cb48fa 100644 --- a/src/modules/scanner/tests.rs +++ b/src/modules/scanner/tests.rs @@ -199,16 +199,26 @@ mod tests { } #[tokio::test] - fn test_invalid_lightning_invoice_sync() { + async fn test_invalid_lightning_invoice() { let invoice = "lnbc1invalid".to_string(); assert!(matches!( - Scanner::decode_onchain(&invoice), + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_issue_63_concatenated_bip21_uris_fails() { + // Exact regression test payload from https://github.com/synonymdev/bitkit-core/issues/63 + let invoice = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&message=Bitkitbitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&message=Bitkit".to_string(); + assert!(matches!( + Scanner::decode(invoice).await, Err(DecodingError::InvalidFormat) )); } #[tokio::test] - async fn test_duplicate_bip21_params_fails() { + async fn test_duplicate_singleton_amount_fails() { let invoice = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.000035&amount=0.00005" .to_string(); @@ -218,6 +228,91 @@ mod tests { )); } + #[tokio::test] + async fn test_duplicate_singleton_case_insensitive_amount_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?Amount=0.000035&amount=0.00005" + .to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_singleton_label_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?label=one&LABEL=two".to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_singleton_message_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?message=first&Message=second" + .to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_singleton_pop_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?pop=http1&pop=http2".to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_allowed_duplicate_payment_instruction_keys_succeeds() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?pj=https://endpoint1&pj=https://endpoint2" + .to_string(); + let decoded = Scanner::decode(invoice).await.unwrap(); + match decoded { + Scanner::OnChain { invoice } => { + assert!(invoice.params.is_some()); + } + _ => assert!(false, "Should be an OnChain invoice"), + } + } + + #[tokio::test] + async fn test_repeated_unknown_query_key_succeeds() { + let invoice = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?custom=val1&custom=val2" + .to_string(); + let decoded = Scanner::decode(invoice).await.unwrap(); + match decoded { + Scanner::OnChain { invoice } => { + assert!(invoice.params.is_some()); + } + _ => assert!(false, "Should be an OnChain invoice"), + } + } + + #[tokio::test] + async fn test_query_key_case_insensitivity_parsed() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?AMOUNT=0.000035&LABEL=MyLabel&MESSAGE=MyMessage" + .to_string(); + let decoded = Scanner::decode(invoice).await.unwrap(); + match decoded { + Scanner::OnChain { invoice } => { + assert_eq!(invoice.amount_satoshis, 3500); + assert_eq!(invoice.label.as_deref(), Some("MyLabel")); + assert_eq!(invoice.message.as_deref(), Some("MyMessage")); + } + _ => assert!(false, "Should be an OnChain invoice"), + } + } + #[tokio::test] async fn test_floating_point_amount_precision() { let invoice = From 7fcb909ced280ac7edd3a097f97a692ba91d7b27 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 24 Sep 2026 23:12:34 +0530 Subject: [PATCH 3/3] fix(scanner): canonicalize pop/req-pop singletons and allow question mark query data (#63) Signed-off-by: Tyagiquamar --- src/modules/scanner/implementation.rs | 26 +++++------- src/modules/scanner/tests.rs | 61 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/modules/scanner/implementation.rs b/src/modules/scanner/implementation.rs index 06ffa32e..19418fa3 100644 --- a/src/modules/scanner/implementation.rs +++ b/src/modules/scanner/implementation.rs @@ -308,7 +308,7 @@ impl Scanner { }) } - fn decode_onchain(invoice_str: &str) -> Result { + pub(crate) fn decode_onchain(invoice_str: &str) -> Result { let without_prefix = if invoice_str.len() >= 8 && invoice_str[..8].eq_ignore_ascii_case("bitcoin:") { &invoice_str[8..] @@ -322,13 +322,7 @@ impl Scanner { } let (address_part, query_part) = match without_prefix.split_once('?') { - Some((addr, query)) => { - // If there's an additional '?' character in the query, it represents malformed/concatenated URIs - if query.contains('?') { - return Err(DecodingError::InvalidFormat); - } - (addr, Some(query)) - } + Some((addr, query)) => (addr, Some(query)), None => (without_prefix, None), }; @@ -348,16 +342,18 @@ impl Scanner { }; let lower_k = k.to_ascii_lowercase(); - let is_singleton = matches!( - lower_k.as_str(), - "amount" | "label" | "message" | "pop" | "req-pop" - ); + let singleton_key = match lower_k.as_str() { + "amount" => Some("amount"), + "label" => Some("label"), + "message" => Some("message"), + "pop" | "req-pop" => Some("pop"), + _ => None, + }; - if is_singleton { - if seen_singletons.contains(&lower_k) { + if let Some(canonical) = singleton_key { + if !seen_singletons.insert(canonical) { return Err(DecodingError::InvalidFormat); } - seen_singletons.insert(lower_k.clone()); } params.insert(lower_k, v.to_string()); diff --git a/src/modules/scanner/tests.rs b/src/modules/scanner/tests.rs index 44cb48fa..73680ae8 100644 --- a/src/modules/scanner/tests.rs +++ b/src/modules/scanner/tests.rs @@ -270,6 +270,67 @@ mod tests { )); } + #[tokio::test] + async fn test_duplicate_singleton_pop_req_pop_mixed_fails() { + // BIP 321 invalid URI: Multiple proof of payment URIs must not appear, even if prefixed with req- + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?pop=callback%3a&req-pop=callback%3a" + .to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_singleton_req_pop_then_pop_fails() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?req-pop=callback1&pop=callback2" + .to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_duplicate_singleton_req_pop_fails() { + let invoice = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?req-pop=cb1&REQ-POP=cb2" + .to_string(); + assert!(matches!( + Scanner::decode(invoice).await, + Err(DecodingError::InvalidFormat) + )); + } + + #[test] + fn test_bip321_spec_invalid_pop_req_pop_example_decode_onchain() { + // Exact BIP 321 specification example from section "Invalid URIs": + // "Multiple proof of payment URIs must not appear, even if they are sometimes prefixed with req-: + // bitcoin:175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W?pop=callback%3a&req-pop=callback%3a" + let invoice = + "bitcoin:175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W?pop=callback%3a&req-pop=callback%3a"; + assert!(matches!( + Scanner::decode_onchain(invoice), + Err(DecodingError::InvalidFormat) + )); + } + + #[tokio::test] + async fn test_query_containing_question_mark_data_succeeds() { + let invoice = + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?message=Why?&amount=0.000035" + .to_string(); + let decoded = Scanner::decode(invoice).await.unwrap(); + match decoded { + Scanner::OnChain { invoice } => { + assert_eq!(invoice.amount_satoshis, 3500); + assert_eq!(invoice.message.as_deref(), Some("Why?")); + } + _ => assert!(false, "Should be an OnChain invoice"), + } + } + #[tokio::test] async fn test_allowed_duplicate_payment_instruction_keys_succeeds() { let invoice =