CheckedHrpstring::validate_segwit accepts witness version > 16

[tcharding]

Cloned from Project Loupe - excuse the verbosity.

Finding

• repo: rust-bitcoin/rust-bech32 (https://github.com/rust-bitcoin/rust-bech32.git)
• title: CheckedHrpstring::validate_segwit accepts witness version > 16
• severity: low
• location: src/primitives/decode.rs:462-479
• cwe: CWE-20
• scanner: llm-code-review
• fingerprint: a814a455915a07a8d3a90f5f75ee5a5112c5829206152420a7a8e429da113b4a

Description

CheckedHrpstring::validate_segwit (lines 462–479) reads the first data character as a witness version but does not enforce the BIP-173 / BIP-350 rule that segwit witness versions are 0..=16:

let witness_version = Fe32::from_char(self.ascii[0].into()).unwrap();
self.ascii = &self.ascii[1..];

self.validate_segwit_padding()?;
self.validate_witness_program_length(witness_version)?;

Ok(SegwitHrpstring { hrp: self.hrp(), witness_version, ascii: self.ascii })

Fe32 values run 0..=31, but only 0..=16 are valid segwit witness versions. validate_witness_program_length (in primitives/segwit.rs) only checks length bounds and the v0-only-20/32 rule; it never checks version <= 16. The two other public entry points that build a SegwitHrpstring — SegwitHrpstring::new (line 583) and SegwitHrpstring::new_bech32 (line 614) — both explicitly reject witness_version.to_u8() > 16, but validate_segwit does not.

A caller using the low-level path CheckedHrpstring::new::<Bech32m>(addr)?.validate_segwit()? therefore obtains a SegwitHrpstring whose witness_version is e.g. 17 (data char '3') or 31 ('l'), with a valid 2–40-byte program. This contradicts the type-level doc on SegwitHrpstring ("had the witness version validated") and can mislead downstream consumers that trust the type's invariants when interpreting witness_version() for script construction or policy decisions.

Fix: add if witness_version.to_u8() > 16 { return Err(SegwitHrpstringError::InvalidWitnessVersion(witness_version)); } mirroring SegwitHrpstring::new.

Proof of Concept

diff --git a/src/primitives/decode.rs b/src/primitives/decode.rs
--- a/src/primitives/decode.rs
+++ b/src/primitives/decode.rs
@@ -1454,6 +1454,18 @@
         invalid_segwit_address_2, "invalid witness version", "bc14r0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq";
         invalid_segwit_address_3, "invalid checksum length", "bc1q5mdq";
         invalid_segwit_address_4, "missing data", "bc1qwf5mdq";
         invalid_segwit_address_5, "invalid program length", "bc14r0srrr7xfkvy5l643lydnw9rewf5mdq";
     }
+
+    #[test]
+    fn validate_segwit_rejects_invalid_witness_version() {
+        // 'l' is Fe32(31), which is > 16 and therefore not a valid segwit witness version.
+        // 32 trailing 'q' chars decode to 20 zero bytes (a valid program length).
+        let checked = CheckedHrpstring {
+            hrp: Hrp::parse_unchecked("bc"),
+            ascii: b"lqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq",
+            hrpstring_length: 36,
+        };
+        assert!(checked.validate_segwit().is_err(), "validate_segwit must reject witness version > 16");
+    }
 }

This finding was discovered by Project Loupe.