scanner: reject duplicate BIP21 parameter keys (#63)

src/modules/scanner/implementation.rs

@@ -308,28 +308,56 @@ impl Scanner {
         })
     }
 
-    fn decode_onchain(invoice_str: &str) -> Result<Self, DecodingError> {
-        let parts: Vec<&str> = invoice_str
-            .strip_prefix("bitcoin:")
-            .unwrap_or(invoice_str)
-            .split('?')
-            .collect();
-
-        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::<HashMap<String, String>>()
+    pub(crate) fn decode_onchain(invoice_str: &str) -> Result<Self, DecodingError> {
+        let without_prefix = if invoice_str
+            .get(..8)
+            .is_some_and(|prefix| prefix.eq_ignore_ascii_case("bitcoin:"))
+        {
+            &invoice_str[8..]
         } else {
-            HashMap::new()
+            invoice_str
+        };
+
+        let (address_part, query_part) = match without_prefix.split_once('?') {
+            Some((addr, query)) => (addr, Some(query)),
+            None => (without_prefix, None),
         };
 
+        let address = address_part.to_string();
+
+        let mut params = HashMap::new();
+        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 singleton_key = match lower_k.as_str() {
+                    "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,
+                };
+
+                if let Some(canonical) = singleton_key {
+                    if !seen_singletons.insert(canonical) {
+                        return Err(DecodingError::InvalidFormat);
+                    }
+                }
+
+                params.insert(lower_k, v.to_string());
+            }
+        }
+
         let amount_satoshis = params
             .get("amount")
             .and_then(|amount| parse_amount_as_satoshis(amount).ok())

src/modules/scanner/tests.rs

@@ -207,6 +207,265 @@ mod tests {
         ));
     }
 
+    #[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_singleton_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_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_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"),
+        }
+    }
+
+    #[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";
+        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 =
+            "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&Custom=first&custom=second"
+                .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"));
+                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"),
+        }
+    }
+
     #[tokio::test]
     async fn test_floating_point_amount_precision() {
         let invoice =