Hrp Hash is case-sensitive while Eq is case-insensitive (Hash/Eq contract violation)

[tcharding]

Cloned from Project Loupe - excuse the verbosity.

Finding

• repo: rust-bitcoin/rust-bech32 (https://github.com/rust-bitcoin/rust-bech32.git)
• reviewed revision: 0853bae3b761e3b2f11b09a49b96b610d452665f
• title: Hrp Hash is case-sensitive while Eq is case-insensitive (Hash/Eq contract violation)
• severity: medium
• location: src/primitives/hrp.rs:332-335
• cwe: CWE-697
• scanner: llm-code-review
• fingerprint: 43c45917577cdb871f894ed5fdf4c56c9990687ce583e6f21a7baed7a075205b

Description

Hrp implements PartialEq/Eq/Ord case-insensitively (lines 309-330): equality compares lowercase_byte_iter() over the first size bytes. But the Hash impl (lines 332-335) hashes self.buf — the full fixed 83-byte array in its original (un-lowercased) case. This breaks the std Hash/Eq contract, which requires a == b ⟹ hash(a) == hash(b). Concretely, Hrp::parse_unchecked("BC") and Hrp::parse_unchecked("bc") are Eq (the crate's own comparison treats them as the same HRP) yet hash differently because their buf bytes differ ([66,67,…] vs [98,99,…]).

Impact: any consumer that uses Hrp as a key in a HashMap/HashSet — the reason Hash+Eq+Ord are all implemented — gets broken behavior. A case variant of a present key lands in a different bucket, so lookups miss. If a HashSet<Hrp> is used as a denylist of forbidden HRPs, an attacker submitting the uppercase form of a denied HRP bypasses the check (set.contains(&hrp) returns false); an allowlist conversely silently fails to match legitimate case variants. The existing ordering_and_hash test only asserts the hash is non-zero, so it never compares two equal-but-different-case HRPs and misses this.

Fix: hash the same bytes Eq compares — iterate lowercase_byte_iter() over the first size bytes. The regression test parses "BC"/"bc", asserts they are equal, then asserts equal hashes (using a simple summing hasher); it fails on HEAD and passes once the hash is made case-insensitive. Distinct from prior finding #359 (mixed-case parse_display), which is a separate bug.

Proof of Concept

--- a/src/primitives/hrp.rs
+++ b/src/primitives/hrp.rs
@@ -771,5 +771,32 @@ mod tests {
         assert_eq!(hrp.as_bytes()[0], b'!');
         assert_eq!(hrp.as_bytes()[1], b'X');
         assert_eq!(hrp.as_bytes()[2], b'~');
     }
+
+    #[test]
+    fn hash_matches_case_insensitive_eq() {
+        // `Hrp`'s `Eq`/`Ord` are case-insensitive, so "BC" and "bc" are equal.
+        // The `Hash`/`Eq` contract then requires them to hash identically, but
+        // the `Hash` impl hashes the raw `buf` in its original case, so equal
+        // values hash differently and break hash-based collections.
+        struct Simple(u64);
+        impl Hasher for Simple {
+            fn finish(&self) -> u64 { self.0 }
+            fn write(&mut self, bytes: &[u8]) {
+                for &b in bytes {
+                    self.0 = self.0.wrapping_mul(31).wrapping_add(b as u64);
+                }
+            }
+        }
+
+        let upper = Hrp::parse_unchecked("BC");
+        let lower = Hrp::parse_unchecked("bc");
+        assert_eq!(upper, lower);
+
+        let mut h1 = Simple(0);
+        let mut h2 = Simple(0);
+        upper.hash(&mut h1);
+        lower.hash(&mut h2);
+        assert_eq!(h1.finish(), h2.finish());
+    }
 }

This finding was discovered by Project Loupe.