Skip to content

Hrp Hash impl is case-sensitive while PartialEq is case-insensitive #275

Description

@tcharding

Cloned from Project Loupe - excuse the verbosity.

Finding

  • repo: rust-bitcoin/rust-bech32 (https://github.com/rust-bitcoin/rust-bech32.git)
  • title: Hrp Hash impl is case-sensitive while PartialEq is case-insensitive
  • severity: medium
  • location: src/primitives/hrp.rs:323-335
  • cwe: CWE-697
  • scanner: llm-code-review
  • fingerprint: 74a106c2abfc71d3decdd8142b2b3129a01d0d489400c7fac70733633edc63d3

Description

Hrp::eq (line 325) compares via lowercase_byte_iter().eq(...), so two HRPs that differ only in case (e.g. bc and BC) compare equal. The Hash impl (line 334), however, hashes self.buf directly, which preserves the originally-parsed case. This violates the core Hash/Eq invariant from core::hash::Hash: "If two keys are equal, their hashes must also be equal."

Concretely: Hrp::parse_unchecked("BC") == Hrp::parse_unchecked("bc") is true, but their Hash outputs differ. Any HashMap<Hrp, _> / HashSet<Hrp> therefore exhibits silent logical failure — inserting one case and looking up the equivalent other case can miss, and inserting both can produce two entries that Hrp::eq claims are the same key. Because Hrp is public API, downstream code that uses it as a hash key (e.g. allow-lists keyed by HRP, dedup sets, caches keyed on the network identifier) is exposed to lookup misses that, depending on call-site semantics, become authorization bypasses or correctness vulnerabilities (the attacker-controlled half of the comparison can be uppercase, the trusted entry lowercase, or vice versa).

Fix: make Hash consume self.lowercase_byte_iter() (or hash a normalized prefix of buf[..size]) so equal HRPs always hash identically. The existing test ordering_and_hash only asserts non-zero hash and does not catch this contract violation.

Proof of Concept

--- a/src/primitives/hrp.rs
+++ b/src/primitives/hrp.rs
@@ -770,6 +770,35 @@ mod tests {
         let hrp = Hrp::parse_unchecked("!\x7f~");
         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_consistent_with_eq() {
+        // The Hash trait requires: if a == b, then hash(a) == hash(b).
+        // Hrp::eq is case-insensitive (via lowercase_byte_iter) but Hrp's
+        // Hash impl hashes the raw buf, so equal HRPs that differ in case
+        // hash to different values. This breaks HashMap/HashSet correctness.
+        use core::hash::{Hash, Hasher};
+
+        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 a = Hrp::parse_unchecked("ABC");
+        let b = Hrp::parse_unchecked("abc");
+        assert_eq!(a, b);
+
+        let mut h1 = Simple(0);
+        let mut h2 = Simple(0);
+        a.hash(&mut h1);
+        b.hash(&mut h2);
+        assert_eq!(h1.finish(), h2.finish());
+    }
 }

This finding was discovered by Project Loupe.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions