What the developer wrote
Authored by Peter D. Gray
little bug
What changed, and why it matters
This commit fixes a small parsing bug in how COLDCARD handles BIP-21 payment URLs (the 'bitcoin:...' links used to request payments). Previously, the code looked for a colon anywhere in the entire string to decide whether a URL protocol like 'bitcoin:' was present. Now it only checks the first 16 characters. The change also updates tests so they exercise both URLs with and without the 'bitcoin:' prefix, including one with a colon inside the label text. The bug could have caused a BIP-21 URL containing a colon later in the string—such as in a label like 'total due: 500'—to be split incorrectly, potentially misinterpreting the address or parameters.
Review whether the 16-character window is sufficient for all supported schemes and whether additional validation (e.g., requiring the scheme to match known protocols) is warranted. Verify that downstream callers handle the corrected parsing consistently and that no other code paths rely on the old, overly broad colon split.
Security signals we found
Input-parsing logic change in a payment-URL decoder
Potential misrouting or misidentification of BIP-21 payment address due to over-broad colon splitting
Test coverage added for colon-in-parameter-value scenario
Evidence from the diff
In shared/utils.py, decode_bip21_text() changed the protocol-detection guard from if ':' in got to if ':' in got[0:16]. The original logic would treat any colon anywhere in the input as a protocol separator, splitting the string at the first colon. A BIP-21 query parameter whose value legitimately contains a colon (e.g., URL-encoded label=total%20due:%20500) could therefore be incorrectly split before the actual scheme, producing a wrong proto and a malformed remainder. The patch constrains protocol detection to the first 16 characters, which is enough for ‘bitcoin:’ and avoids misinterpreting colons in parameter values. The test file now builds URLs by optionally prepending ‘bitcoin:’ rather than stripping it, and adds a label-with-colon case.
Changed components
shared/utils.py:decode_bip21_text()BIP-21 URL parsing / QR-code payment request handlingInspect captured patch +11 / −9
diff --git a/shared/utils.py b/shared/utils.py
index 816cdd0..cce7c86 100644
--- a/shared/utils.py
+++ b/shared/utils.py
@@ -645,7 +645,7 @@ def decode_bip21_text(got):
proto, args, addr = None, None, None
# remove URL protocol: if present
- if ':' in got:
+ if ':' in got[0:16]:
proto, got = got.split(':', 1)
# looks like BIP-21 payment URL
diff --git a/testing/test_decoders.py b/testing/test_decoders.py
index 69c2f96..50aa422 100644
--- a/testing/test_decoders.py
+++ b/testing/test_decoders.py
@@ -53,12 +53,13 @@ def test_detector_bin(fname, expect, encoding, try_decode):
@pytest.mark.parametrize('url', [
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R',
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?label=Luke-Jr',
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?amount=20.3&label=Luke-Jr',
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?amount=50&label=Luke-Jr&message=Donation%20for%20project%20xyz',
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?req-somethingyoudontunderstand=50&req-somethingelseyoudontget=999',
-'bitcoin:mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?somethingyoudontunderstand=50&somethingelseyoudontget=999',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?label=Luke-Jr',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?amount=20.3&label=Luke-Jr',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?amount=50&label=Luke-Jr&message=Donation%20for%20project%20xyz',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?req-somethingyoudontunderstand=50&req-somethingelseyoudontget=999',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?somethingyoudontunderstand=50&somethingelseyoudontget=999',
+'mtHSVByP9EYZmB26jASDdPVm19gvpecb5R?label=total%20due:%20500',
])
@pytest.mark.parametrize('bip21', range(2))
@pytest.mark.parametrize('addr_fmt', range(2))
@@ -66,8 +67,9 @@ def test_detector_url(url, bip21, addr_fmt, try_decode):
a1, a2 = ('mtHSVByP9EYZmB26jASDdPVm19gvpecb5R',
'BCRT1QUPYD58NDSH7LUT0ET0VTRQ432JVU9JTDX8FGYV')
- if not bip21:
- _, url = url.split(':', 1)
+ if bip21:
+ url = 'bitcoin:' + url
+
if addr_fmt:
url = url.replace(a1, a2)
expect_addr = a2.lower()
Why this scored 38/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.