Merge pull request #407 from bsv-blockchain/TOB-21-Hex

Security: Enforce strict hex validation for Hash.toArray (TOB-21)

Author: ty-everett

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. The format
 ## Table of Contents
 
 - [Unreleased](#unreleased)
+- [1.9.21 - 2025-12-04](#1921---2025-12-04)
 - [1.9.20 - 2025-12-02](#1920---2025-12-02)
 - [1.9.19 - 2025-12-02](#1919---2025-12-02)
 - [1.9.18 - 2025-12-02](#1918---2025-12-02)
@@ -192,6 +193,26 @@ All notable changes to this project will be documented in this file. The format
 ### Security
 ---
 
+### [1.9.21] - 2025-12-04
+
+### Security
+
+- Implemented strict validation for hex string parsing across the codebase,
+  addressing TOB-21.  
+  Non-hex characters now cause an immediate error instead of being silently
+  discarded, preventing message-forgery scenarios.
+
+### Fixed
+
+- Rewrote the `toArray` and `hexToArray` conversion logic to enforce strict
+  hexadecimal input handling rather than permissive filtering.  
+- Corrected UTF-8 decoding behavior to ensure invalid byte sequences produce
+  a single `U+FFFD` replacement character as specified.  
+- Updated all internal hash and array conversion utilities to maintain consistent
+  behavior with strong input validation.
+
+---
+
 ### [1.9.20] - 2025-12-02
 
 ### Security

benchmarks/transaction-bench.js

@@ -21,9 +21,9 @@ async function deepInputChain () {
   const path = [
     [
       { offset: 0, hash: txid, txid: true, duplicate: false },
-      { offset: 1, hash: 'otherHash1', txid: false, duplicate: false }
+      { offset: 1, hash: 'aa'.repeat(32), txid: false, duplicate: false }
     ],
-    [{ offset: 1, hash: 'mergedHash1', txid: false, duplicate: false }]
+    [{ offset: 1, hash: 'bb'.repeat(32), txid: false, duplicate: false }]
   ]
   const merklePath = new MerklePath(blockHeight, path)
   tx.merklePath = merklePath
@@ -65,9 +65,9 @@ async function wideInputSet () {
     const path = [
       [
         { offset: 0, hash: txid, txid: true, duplicate: false },
-        { offset: 1, hash: 'otherHash1', txid: false, duplicate: false }
+        { offset: 1, hash: 'aa'.repeat(32), txid: false, duplicate: false }
       ],
-      [{ offset: 1, hash: 'mergedHash1', txid: false, duplicate: false }]
+      [{ offset: 1, hash: 'bb'.repeat(32), txid: false, duplicate: false }]
     ]
     const merklePath = new MerklePath(blockHeight, path)
     sourceTx.merklePath = merklePath
@@ -110,9 +110,9 @@ async function largeInputsOutputs () {
     const path = [
       [
         { offset: 0, hash: txid, txid: true, duplicate: false },
-        { offset: 1, hash: 'otherHash1', txid: false, duplicate: false }
+        { offset: 1, hash: 'aa'.repeat(32), txid: false, duplicate: false }
       ],
-      [{ offset: 1, hash: 'mergedHash1', txid: false, duplicate: false }]
+      [{ offset: 1, hash: 'bb'.repeat(32), txid: false, duplicate: false }]
     ]
     const merklePath = new MerklePath(blockHeight, path)
     sourceTx.merklePath = merklePath
@@ -158,9 +158,9 @@ async function nestedInputs () {
     const path = [
       [
         { offset: 0, hash: txid, txid: true, duplicate: false },
-        { offset: 1, hash: 'otherHash1', txid: false, duplicate: false }
+        { offset: 1, hash: 'aa'.repeat(32), txid: false, duplicate: false }
       ],
-      [{ offset: 1, hash: 'mergedHash1', txid: false, duplicate: false }]
+      [{ offset: 1, hash: 'bb'.repeat(32), txid: false, duplicate: false }]
     ]
     const merklePath = new MerklePath(blockHeight, path)
     baseTx.merklePath = merklePath

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@bsv/sdk",
-  "version": "1.9.19",
+  "version": "1.9.20",
   "type": "module",
   "description": "BSV Blockchain Software Development Kit",
   "main": "dist/cjs/mod.js",

package.json

@@ -12,6 +12,7 @@ import { SimplifiedFetchTransport } from '../../auth/transports/SimplifiedFetchT
 const certifierPrivKey = new PrivateKey(21)
 const alicePrivKey = new PrivateKey(22)
 const bobPrivKey = new PrivateKey(23)
+const DUMMY_REVOCATION_OUTPOINT_HEX = '00'.repeat(36)
 
 jest.mock('../../auth/utils/getVerifiableCertificates')
 
@@ -101,7 +102,7 @@ describe('Peer class mutual authentication and certificate exchange', () => {
         subjectPubKey,
         fields,
         certificateType,
-        async () => 'revocationOutpoint' // or any revocation outpoint logic you want
+        async () => DUMMY_REVOCATION_OUTPOINT_HEX
       )
 
     // For test consistency, you could override the auto-generated serialNumber:

src/auth/__tests/Peer.test.ts

@@ -232,7 +232,7 @@ export class MasterCertificate extends Certificate {
     certificateType: string,
     getRevocationOutpoint = async (_serial: string): Promise<string> => {
       void _serial // Explicitly acknowledge unused parameter
-      return 'Certificate revocation not tracked.'
+      return '00'.repeat(32)
     },
     serialNumber?: string
   ): Promise<MasterCertificate> {
@@ -246,11 +246,18 @@ export class MasterCertificate extends Certificate {
     // 3. Obtain a revocation outpoint
     const revocationOutpoint = await getRevocationOutpoint(finalSerialNumber)
 
+    let subjectIdentityKey: string
+      if (subject === 'self') {
+        subjectIdentityKey = (await certifierWallet.getPublicKey({ identityKey: true })).publicKey
+      } else {
+        subjectIdentityKey = subject
+      }
+
     // 4. Create new MasterCertificate instance
     const certificate = new MasterCertificate(
       certificateType,
       finalSerialNumber,
-      subject,
+      subjectIdentityKey,
       (await certifierWallet.getPublicKey({ identityKey: true })).publicKey,
       revocationOutpoint,
       certificateFields,

src/auth/certificates/MasterCertificate.ts

@@ -14,7 +14,8 @@ const verifierKey2 = new PrivateKey(81)
 
 // A mock revocation outpoint for testing
 const mockRevocationOutpoint =
-  'deadbeefdeadbeefdeadbeefdeadbeef00000000000000000000000000000000.1'
+  'deadbeefdeadbeefdeadbeefdeadbeef00000001'
+
 
 // Arbitrary certificate data (in plaintext)
 const plaintextFields = {
@@ -355,33 +356,69 @@ describe('MasterCertificate', () => {
         expect(newCert.fields[fieldName]).toMatch(/^[A-Za-z0-9+/]+=*$/)
       }
     })
+
     it('should allow issuing a self-signed certificate and decrypt it with the same wallet', async () => {
-      // In a self-signed scenario, the subject and certifier are the same
       const subjectWallet = new CompletedProtoWallet(subjectKey2)
 
-      // Some sample fields
       const selfSignedFields = {
         owner: 'Bob',
         organization: 'SelfCo'
       }
 
-      // Issue the certificate for "self"
+      const subjectIdentityKey = (
+        await subjectWallet.getPublicKey({ identityKey: true })
+      ).publicKey
+
+      // Issue the certificate: subject = actual identity key (valid hex)
       const selfSignedCert = await MasterCertificate.issueCertificateForSubject(
-        subjectWallet, // act as certifier
-        'self',
+        subjectWallet,        // acts as certifier
+        subjectIdentityKey,   // <-- was 'self', now real hex
         selfSignedFields,
         'SELF_SIGNED_TEST'
       )
 
-      // Now we attempt to decrypt the fields with the same wallet
+      // Decrypt with the same wallet
       const decrypted = await MasterCertificate.decryptFields(
         subjectWallet,
         selfSignedCert.masterKeyring,
         selfSignedCert.fields,
-        'self'
+        'self' // still fine here if decryptFields treats 'self' specially
       )
 
       expect(decrypted).toEqual(selfSignedFields)
     })
+
+    it('resolves subject === "self" to the certifier wallet identity key', async () => {
+      const certifierWallet = new CompletedProtoWallet(new PrivateKey(99))
+
+      const certifierIdentityKey = (
+        await certifierWallet.getPublicKey({ identityKey: true })
+      ).publicKey
+
+      const cert = await MasterCertificate.issueCertificateForSubject(
+        certifierWallet,
+        'self',
+        { name: 'Alice' },
+        'TEST_CERT'
+      )
+
+      expect(cert.subject).toBe(certifierIdentityKey)
+    })
+
+    it('uses provided subjectIdentityKey when subject is a valid hex string', async () => {
+      const certifierWallet = new CompletedProtoWallet(new PrivateKey(42))
+
+      const validPubkey =
+        '0279BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798'
+
+      const cert = await MasterCertificate.issueCertificateForSubject(
+        certifierWallet,
+        validPubkey,
+        { name: 'Alice' },
+        'TEST_CERT'
+      )
+
+      expect(cert.subject).toBe(validPubkey)
+    })
   })
-})
+})

src/auth/certificates/__tests/MasterCertificate.test.ts

@@ -1,6 +1,8 @@
 
 // @ts-nocheck
 /* eslint-disable @typescript-eslint/naming-convention */
+import { assertValidHex, normalizeHex } from './hex.js'
+
 const assert = (
   expression: unknown,
   message: string = 'Hash assertion failed'
@@ -169,6 +171,10 @@ abstract class BaseHash {
    */
   private _pad (): number[] {
     const len = this.pendingTotal
+    if (!Number.isSafeInteger(len) || len < 0) {
+      throw new Error('Message too long for this hash function')
+    }
+
     const bytes = this._delta8
     const k = bytes - ((len + this.padLength) % bytes)
     const res = new Array(k + this.padLength)
@@ -177,8 +183,6 @@ abstract class BaseHash {
     for (i = 1; i < k; i++) {
       res[i] = 0
     }
-
-    // Append length
     const lengthBytes = this.padLength
     const maxBits = 1n << BigInt(lengthBytes * 8)
     let totalBits = BigInt(len) * 8n
@@ -204,6 +208,7 @@ abstract class BaseHash {
         totalBits >>= 8n
       }
     }
+
     return res
   }
 }
@@ -262,10 +267,8 @@ export function toArray (
         }
       }
     } else {
-      msg = msg.replace(/[^a-z0-9]+/gi, '')
-      if (msg.length % 2 !== 0) {
-        msg = '0' + msg
-      }
+      assertValidHex(msg)
+      msg = normalizeHex(msg)
       for (let i = 0; i < msg.length; i += 2) {
         res.push(parseInt(msg[i] + msg[i + 1], 16))
       }

src/primitives/Hash.ts

@@ -48,11 +48,22 @@ describe('HMAC', function () {
       res: 'cf5ad5984f9e43917aa9087380dac46e410ddc8a7731859c84e9d0f31bd43655'
     })
 
+    function normalizeKey (key: string | number[]): string | number[] {
+      if (typeof key === 'string') {
+        // test-only helper: remove whitespace between hex groups
+        return key.replace(/\s+/g, '')
+      }
+      return key
+    }
+
     function test (opt): void {
       it(`should not fail at ${opt.name as string}`, function (): void {
-        let h = new SHA256HMAC(opt.key)
+        const key = normalizeKey(opt.key)
+
+        let h = new SHA256HMAC(key as any)
         expect(h.update(opt.msg, opt.msgEnc).digestHex()).toEqual(opt.res)
-        h = h = new SHA256HMAC(opt.key)
+
+        h = new SHA256HMAC(key as any)
         expect(
           h
             .update(opt.msg.slice(0, 10), opt.msgEnc)

src/primitives/__tests/HMAC.test.ts

@@ -3,6 +3,7 @@ import * as hash from '../../primitives/Hash'
 import * as crypto from 'crypto'
 import PBKDF2Vectors from './PBKDF2.vectors'
 import { toArray, toHex } from '../../primitives/utils'
+import { SHA1 } from '../..//primitives/Hash'
 
 describe('Hash', function () {
   function test (Hash, cases): void {
@@ -177,4 +178,27 @@ describe('Hash', function () {
       })
     }
   })
+
+  describe('Hash strict length validation (TOB-21)', () => {
+
+    it('throws when pendingTotal is not a safe integer', () => {
+      const h = new SHA1()
+
+      h.pendingTotal = Number.MAX_SAFE_INTEGER + 10
+
+      expect(() => {
+        h.digest()
+      }).toThrow('Message too long for this hash function')
+    })
+
+    it('throws when pendingTotal is negative', () => {
+      const h = new SHA1()
+
+      h.pendingTotal = -5
+
+      expect(() => {
+        h.digest()
+      }).toThrow('Message too long for this hash function')
+    })
+  })
 })