From c82e6d5ab0ff4b57a70fc12e4fd0dcbb1a4f082a Mon Sep 17 00:00:00 2001
From: Taksh Kothari <takshkothari09@gmail.com>
Date: Thu, 24 Sep 2026 13:04:25 +0530
Subject: [PATCH] fix: reject a signed byte pair in Data(hexString:) (#1687)

* fix: reject a signed byte pair in Data(hexString:)

UInt8(_:radix:) accepts a leading sign, so the two-character chunks this
parser feeds it let "+f" through as 0x0f. The doc comment already promises nil
for a string containing invalid hex characters, and it did not deliver that.

The cost is a second spelling for every identity-bearing hex string the app
parses -- Noise keys, Nostr pubkeys, fingerprints, signatures, routing IDs.
A 64-character peer ID of "+b" pairs passed PeerID.isValid, reported isHex
false and isNoiseKeyHex true at the same time, and resolved to the same
32-byte key and the same short routing ID as the genuine peer it shadowed.

Require two hex digits before parsing the pair.

* test: cover hex parsing and the PeerID validity it backs

DataHexTests covers what the parser accepts (plain, uppercase, 0x-prefixed,
whitespace-padded, every byte value round-tripped) alongside what it must
refuse, including the signed pairs that used to alias a genuine key.

PeerIDTests gains the 64-character negatives it was missing -- the short form
already had them -- and asserts isHex and isNoiseKeyHex agree.
---
 .../Sources/BitFoundation/Data+Hex.swift      |  7 ++-
 .../BitFoundationTests/DataHexTests.swift     | 62 +++++++++++++++++++
 .../BitFoundationTests/PeerIDTests.swift      | 11 ++++
 3 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 localPackages/BitFoundation/Tests/BitFoundationTests/DataHexTests.swift

diff --git a/localPackages/BitFoundation/Sources/BitFoundation/Data+Hex.swift b/localPackages/BitFoundation/Sources/BitFoundation/Data+Hex.swift
index e847b2e2..8b60e0b1 100644
--- a/localPackages/BitFoundation/Sources/BitFoundation/Data+Hex.swift
+++ b/localPackages/BitFoundation/Sources/BitFoundation/Data+Hex.swift
@@ -45,7 +45,12 @@ public extension Data {
 
         for _ in 0..<len {
             let nextIndex = hex.index(index, offsetBy: 2)
-            guard let byte = UInt8(String(hex[index..<nextIndex]), radix: 16) else {
+            let pair = hex[index..<nextIndex]
+            // UInt8(_:radix:) accepts a leading sign, so "+f" parses as 0x0f.
+            // That gives an identity-bearing hex string a second spelling which
+            // decodes to the same bytes, so require two hex digits first.
+            guard pair.allSatisfy(\.isHexDigit),
+                  let byte = UInt8(pair, radix: 16) else {
                 return nil
             }
             data.append(byte)
diff --git a/localPackages/BitFoundation/Tests/BitFoundationTests/DataHexTests.swift b/localPackages/BitFoundation/Tests/BitFoundationTests/DataHexTests.swift
new file mode 100644
index 00000000..9940fb50
--- /dev/null
+++ b/localPackages/BitFoundation/Tests/BitFoundationTests/DataHexTests.swift
@@ -0,0 +1,62 @@
+//
+// DataHexTests.swift
+// bitchatTests
+//
+// This is free and unencumbered software released into the public domain.
+// For more information, see <https://unlicense.org>
+//
+
+import Testing
+import Foundation
+import BitFoundation
+
+struct DataHexTests {
+    // MARK: - Accepted forms
+
+    @Test func decodes_plain_hex() {
+        #expect(Data(hexString: "0a1b2c")?.hexEncodedString() == "0a1b2c")
+    }
+
+    @Test func decodes_uppercase_hex() {
+        #expect(Data(hexString: "0A1B2C")?.hexEncodedString() == "0a1b2c")
+    }
+
+    @Test func decodes_prefixed_and_padded_hex() {
+        #expect(Data(hexString: "0x0a1b")?.hexEncodedString() == "0a1b")
+        #expect(Data(hexString: "0X0a1b")?.hexEncodedString() == "0a1b")
+        #expect(Data(hexString: "  0a1b  ")?.hexEncodedString() == "0a1b")
+    }
+
+    @Test func round_trips_encoded_bytes() {
+        let bytes = Data((0...255).map { UInt8($0) })
+        #expect(Data(hexString: bytes.hexEncodedString()) == bytes)
+    }
+
+    // MARK: - Rejected forms
+
+    @Test func rejects_odd_length() {
+        #expect(Data(hexString: "0a1") == nil)
+    }
+
+    @Test func rejects_non_hex_characters() {
+        #expect(Data(hexString: "zz") == nil)
+        #expect(Data(hexString: "0a zz") == nil)
+    }
+
+    /// UInt8(_:radix:) accepts a leading sign, so "+f" parses as 0x0f unless the
+    /// digits are checked first. Two spellings of one identity-bearing hex
+    /// string decoding to the same bytes is the whole problem.
+    @Test func rejects_signed_byte_pairs() {
+        #expect(Data(hexString: "+a") == nil)
+        #expect(Data(hexString: "-a") == nil)
+        #expect(Data(hexString: "0a+b") == nil)
+        #expect(Data(hexString: "+a+b") == nil)
+    }
+
+    @Test func a_signed_pair_does_not_alias_a_genuine_key() {
+        let genuine = String(repeating: "0b", count: 32)
+        let signed = String(repeating: "+b", count: 32)
+        #expect(Data(hexString: genuine) != nil)
+        #expect(Data(hexString: signed) == nil)
+    }
+}
diff --git a/localPackages/BitFoundation/Tests/BitFoundationTests/PeerIDTests.swift b/localPackages/BitFoundation/Tests/BitFoundationTests/PeerIDTests.swift
index def8e89a..05e2b687 100644
--- a/localPackages/BitFoundation/Tests/BitFoundationTests/PeerIDTests.swift
+++ b/localPackages/BitFoundation/Tests/BitFoundationTests/PeerIDTests.swift
@@ -362,6 +362,10 @@ struct PeerIDTests {
     @Test func rejects_invalid_characters() {
         #expect(!PeerID(str: "peer!@#").isValid)
         #expect(!PeerID(str: "gggggggggggggggg").isValid) // not hex for short form
+        // A 64-character id is only valid because it decodes to a Noise key, so
+        // it has to be rejected on the same grounds a short one is.
+        #expect(!PeerID(str: String(repeating: "+b", count: 32)).isValid)
+        #expect(!PeerID(str: String(repeating: "gg", count: 32)).isValid)
     }
     
     @Test func rejects_too_long() {
@@ -388,6 +392,13 @@ struct PeerIDTests {
         let badPeerID = PeerID(str: bad)
         #expect(!badPeerID.isNoiseKeyHex)
         #expect(badPeerID.noiseKey == nil)
+
+        // isHex and isNoiseKeyHex have to agree: a signed pair is not hex, so it
+        // must not decode to a Noise key either.
+        let signedPeerID = PeerID(str: String(repeating: "+b", count: 32))
+        #expect(!signedPeerID.isHex)
+        #expect(!signedPeerID.isNoiseKeyHex)
+        #expect(signedPeerID.noiseKey == nil)
     }
     
     @Test func prefixes() {