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

Open Beginner friendly
#275 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
rust
Domain
security

Research direction

Start in src/primitives/hrp.rs:323-335 and compare the case-insensitive equality implementation with the current Hash implementation. Review the existing ordering_and_hash test and the provided hash_consistent_with_eq case, then run the Rust test suite. Done means equal HRPs differing only in case produce identical hashes without breaking existing tests.

Written by the indexing model from the issue text.

Description

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.

Dominant language
Rust
Stars
100
Forks
74
Avg merge
2d 6h
Merged PRs (30d)
9

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rust-bitcoin/rust-bech32

All issues in rust-bitcoin/rust-bech32

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.