From 911dc0c1534eea0570f5e940df670a6a6489d794 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Wed, 2 Sep 2026 18:37:43 +0530 Subject: [PATCH 1/5] 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/5] 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/5] 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 = From 08a781819583afe1c77b803f9bfd7823ce71acc7 Mon Sep 17 00:00:00 2001 From: Philipp Walter Date: Fri, 9 Oct 2026 13:31:53 +0200 Subject: [PATCH 4/5] fix: preserve bitcoin uri text in query values --- src/modules/scanner/implementation.rs | 5 --- src/modules/scanner/tests.rs | 60 +++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/modules/scanner/implementation.rs b/src/modules/scanner/implementation.rs index 19418fa3..ddd29ded 100644 --- a/src/modules/scanner/implementation.rs +++ b/src/modules/scanner/implementation.rs @@ -316,11 +316,6 @@ impl Scanner { 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)) => (addr, Some(query)), None => (without_prefix, None), diff --git a/src/modules/scanner/tests.rs b/src/modules/scanner/tests.rs index 73680ae8..a16d5d88 100644 --- a/src/modules/scanner/tests.rs +++ b/src/modules/scanner/tests.rs @@ -331,6 +331,66 @@ mod tests { } } + #[tokio::test] + async fn test_bitcoin_colon_in_query_values_succeeds() { + let address = "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn"; + for (key, value) in [ + ("message", "bitcoin:donation"), + ("label", "BITCOIN:donation"), + ("custom", "bitcoin:donation"), + ("custom", "https://example.com/bitcoin:donation"), + ("message", "bitcoin%3Adonation"), + ("message", "bitcoin:1BoatSLRHtKNngkdXEeobR76b53LETtpyT"), + ] { + let uri = format!("bitcoin:{address}?amount=0.000035&{key}={value}"); + let decoded = Scanner::decode(uri.clone()).await.unwrap(); + match decoded { + Scanner::OnChain { invoice } => { + assert_eq!(invoice.address, address, "{uri}"); + assert_eq!(invoice.amount_satoshis, 3500, "{uri}"); + assert_eq!( + invoice + .params + .as_ref() + .unwrap() + .get(key) + .map(String::as_str), + Some(value), + "{uri}" + ); + match key { + "message" => assert_eq!(invoice.message.as_deref(), Some(value), "{uri}"), + "label" => assert_eq!(invoice.label.as_deref(), Some(value), "{uri}"), + _ => {} + } + } + _ => panic!("Expected an on-chain invoice for '{uri}'"), + } + } + } + + #[tokio::test] + async fn test_duplicate_query_keys_still_fail_with_bitcoin_colon_values() { + let address = "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn"; + for query in [ + "message=bitcoin:donation&Message=other", + "label=BITCOIN:donation&label=other", + "pop=bitcoin:donation&req-pop=other", + ] { + assert!(matches!( + Scanner::decode(format!("bitcoin:{address}?{query}")).await, + Err(DecodingError::InvalidFormat) + )); + } + } + + #[tokio::test] + async fn test_concatenated_uri_in_address_is_rejected() { + let address = "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn"; + let uri = format!("bitcoin:{address}bitcoin:{address}?amount=0.000035"); + assert!(Scanner::decode(uri).await.is_err()); + } + #[tokio::test] async fn test_allowed_duplicate_payment_instruction_keys_succeeds() { let invoice = From f501fbb1b51812da2caa092f8935201e58252885 Mon Sep 17 00:00:00 2001 From: Philipp Walter Date: Fri, 9 Oct 2026 13:42:17 +0200 Subject: [PATCH 5/5] fix: make bitcoin uri prefix inspection utf-8 safe --- src/modules/scanner/implementation.rs | 15 +++++++----- src/modules/scanner/tests.rs | 34 ++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/modules/scanner/implementation.rs b/src/modules/scanner/implementation.rs index ddd29ded..918aada8 100644 --- a/src/modules/scanner/implementation.rs +++ b/src/modules/scanner/implementation.rs @@ -309,12 +309,14 @@ impl Scanner { } 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..] - } else { - invoice_str - }; + let without_prefix = if invoice_str + .get(..8) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("bitcoin:")) + { + &invoice_str[8..] + } else { + invoice_str + }; let (address_part, query_part) = match without_prefix.split_once('?') { Some((addr, query)) => (addr, Some(query)), @@ -341,6 +343,7 @@ impl Scanner { "amount" => Some("amount"), "label" => Some("label"), "message" => Some("message"), + // BIP321 explicitly aliases pop/req-pop; general required-parameter handling is separate. "pop" | "req-pop" => Some("pop"), _ => None, }; diff --git a/src/modules/scanner/tests.rs b/src/modules/scanner/tests.rs index a16d5d88..65792a98 100644 --- a/src/modules/scanner/tests.rs +++ b/src/modules/scanner/tests.rs @@ -331,6 +331,35 @@ mod tests { } } + #[test] + fn test_decode_onchain_non_ascii_prefix_does_not_panic() { + for input in ["", "€", "€€€", "1234567€", "💸💸💸"] { + match Scanner::decode_onchain(input).unwrap() { + Scanner::OnChain { invoice } => assert_eq!(invoice.address, input), + _ => panic!("Expected an on-chain invoice for '{input}'"), + } + } + } + + #[tokio::test] + async fn test_bare_query_keys_have_empty_values() { + let address = "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn"; + let uri = format!("bitcoin:{address}?message&label&custom"); + match Scanner::decode(uri).await.unwrap() { + Scanner::OnChain { invoice } => { + assert_eq!(invoice.address, address); + assert_eq!(invoice.message.as_deref(), Some("")); + assert_eq!(invoice.label.as_deref(), Some("")); + let params = invoice.params.unwrap(); + assert_eq!(params.len(), 3); + for key in ["message", "label", "custom"] { + assert_eq!(params.get(key).map(String::as_str), Some("")); + } + } + _ => panic!("Expected an on-chain invoice"), + } + } + #[tokio::test] async fn test_bitcoin_colon_in_query_values_succeeds() { let address = "mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn"; @@ -421,7 +450,7 @@ mod tests { #[tokio::test] async fn test_query_key_case_insensitivity_parsed() { let invoice = - "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?AMOUNT=0.000035&LABEL=MyLabel&MESSAGE=MyMessage" + "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?AMOUNT=0.000035&LABEL=MyLabel&MESSAGE=MyMessage&Custom=first&custom=second" .to_string(); let decoded = Scanner::decode(invoice).await.unwrap(); match decoded { @@ -429,6 +458,9 @@ mod tests { assert_eq!(invoice.amount_satoshis, 3500); assert_eq!(invoice.label.as_deref(), Some("MyLabel")); assert_eq!(invoice.message.as_deref(), Some("MyMessage")); + let params = invoice.params.unwrap(); + assert_eq!(params.get("custom").map(String::as_str), Some("second")); + assert!(!params.contains_key("Custom")); } _ => assert!(false, "Should be an OnChain invoice"), }