refactor: clean code and improve input sanityzing

Author: coratgerl

.github/workflows/release.yml

@@ -28,8 +28,8 @@ jobs:
           registry-url: "https://registry.npmjs.org"
 
       - run: bun install
-      - run: bun ci
       - run: bun run build
+      - run: bun run ci
 
       - name: "Get Previous tag"
         id: previousTag
@@ -56,9 +56,9 @@ jobs:
           echo "next_version=$next_version" >> $GITHUB_ENV
 
       # We publish on NPM before releasing on GitHub to avoid releasing a version that is not published
-      - run: npm publish --access=public
+      - run: bun publish --access public
         env:
-          NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}
+          NPM_CONFIG_TOKEN: ${{secrets.NPM_TOKEN}}
 
       - name: Create tag
         uses: actions/github-script@v7

bun.lock

@@ -1,6 +1,6 @@
 import { BigInteger } from 'jsbn'
 import { getRandomValues } from './crypto'
-import { bufferToHex } from './utils'
+import { bufferToHex, sanitizeHex } from './utils'
 
 const bi = Symbol('big-int')
 
@@ -17,7 +17,20 @@ export class SRPInt {
   static ZERO = new SRPInt(new BigInteger('0'), null)
 
   static fromHex(hex: string): SRPInt {
-    const sanitized = hex.replace(/\s+/g, '').toLowerCase()
+    const sanitized = sanitizeHex(hex)
+
+    if (sanitized.length === 0) {
+      throw new RangeError(
+        'Expected string to contain at least one hexadecimal character',
+      )
+    }
+
+    if (!/^[\da-f]+$/.test(sanitized)) {
+      throw new RangeError(
+        'Expected string to only contain hexadecimal characters',
+      )
+    }
+
     return new SRPInt(new BigInteger(sanitized, 16), sanitized.length)
   }
 

src/SRPInt.ts

@@ -3,13 +3,16 @@ import { getParams } from './params'
 import { SRPError } from './SRPError'
 import { SRPInt } from './SRPInt'
 import type { Ephemeral, HashAlgorithm, PrimeGroup, Session } from './types'
-import { bufferToHex, hexToBuffer } from './utils'
+import { bufferToHex, constantTimeEqualHex, hexToBuffer } from './utils'
 
 export const createSRPClient = (
   hashAlgorithm: HashAlgorithm,
   primeGroup: PrimeGroup,
 ) => {
-  const { N, g, k, H, PAD, hashBytes } = getParams(hashAlgorithm, primeGroup)
+  const { N, g, k, H, PAD, hashBytes, ephemeralSecretBytes } = getParams(
+    hashAlgorithm,
+    primeGroup,
+  )
 
   return {
     generateSalt: (): string => {
@@ -53,7 +56,7 @@ export const createSRPClient = (
     },
 
     generateEphemeral: (): Ephemeral => {
-      const a = SRPInt.getRandom(hashBytes)
+      const a = SRPInt.getRandom(ephemeralSecretBytes)
 
       // A = g^a  (a = random number)
       const A = g.modPow(a, N)
@@ -115,10 +118,9 @@ export const createSRPClient = (
       const K = SRPInt.fromHex(clientSession.key) // Shared, strong session key
 
       // H(A, M, K)
-      const expected = await H(A, M, K)
-      const actual = SRPInt.fromHex(serverSessionProof)
+      const expected = (await H(A, M, K)).toHex()
 
-      if (!actual.equals(expected)) {
+      if (!constantTimeEqualHex(serverSessionProof, expected)) {
         throw new SRPError('server', 'InvalidSessionProof')
       }
     },

src/client.ts

@@ -22,7 +22,11 @@ const pbkdf2Iterations: Record<HashAlgorithm, number> = {
 }
 
 export const getRandomValues = (array: Uint8Array): void => {
-  webcrypto?.getRandomValues(array)
+  if (!webcrypto) {
+    throw new Error(unavailableErrorMessage)
+  }
+
+  webcrypto.getRandomValues(array)
 }
 
 export const digest = (

src/crypto.ts

@@ -165,6 +165,7 @@ export const getParams = (
   const g = SRPInt.fromHex(group.g)
 
   const paddedHexLength = N.hexLength ?? 0 // N.hexLength is never null
+  const ephemeralSecretBytes = Math.max(hashBytes[hashAlgorithm], 32)
 
   const H = (...input: (SRPInt | string)[]) => hash(hashAlgorithm, ...input)
   const PAD = (integer: SRPInt) => integer.pad(paddedHexLength)
@@ -177,5 +178,6 @@ export const getParams = (
     H, // One-way hash function
     PAD, // Pad function to have the same number of bytes as N
     hashBytes: hashBytes[hashAlgorithm], // Hash function output length
+    ephemeralSecretBytes, // Random secret length for ephemeral exponents
   }
 }

src/params.ts

@@ -2,19 +2,23 @@ import { getParams } from './params'
 import { SRPError } from './SRPError'
 import { SRPInt } from './SRPInt'
 import type { Ephemeral, HashAlgorithm, PrimeGroup, Session } from './types'
+import { constantTimeEqualHex } from './utils'
 
 export const createSRPServer = (
   hashAlgorithm: HashAlgorithm,
   primeGroup: PrimeGroup,
 ) => {
-  const { N, g, k, H, PAD, hashBytes } = getParams(hashAlgorithm, primeGroup)
+  const { N, g, k, H, PAD, ephemeralSecretBytes } = getParams(
+    hashAlgorithm,
+    primeGroup,
+  )
 
   return {
     generateEphemeral: async (verifier: string): Promise<Ephemeral> => {
       const v = SRPInt.fromHex(verifier) // Password verifier
 
       // B = kv + g^b  (b = random number)
-      const b = SRPInt.getRandom(hashBytes)
+      const b = SRPInt.getRandom(ephemeralSecretBytes)
       const B = (await k()).multiply(v).add(g.modPow(b, N)).mod(N)
 
       return {
@@ -56,10 +60,9 @@ export const createSRPServer = (
       const [K, HN, Hg, HI] = await Promise.all([H(S), H(N), H(g), H(I)])
       const M = await H(HN.xor(Hg), HI, s, A, B, K)
 
-      const expected = M
-      const actual = SRPInt.fromHex(clientSessionProof)
+      const expected = M.toHex()
 
-      if (!actual.equals(expected)) {
+      if (!constantTimeEqualHex(clientSessionProof, expected)) {
         throw new SRPError('client', 'InvalidSessionProof')
       }
 

src/server.ts

@@ -1,4 +1,8 @@
 export const encodeUtf8 = TextEncoder.prototype.encode.bind(new TextEncoder())
+const hexPattern = /^[\da-f]+$/i
+
+export const sanitizeHex = (hex: string): string =>
+  hex.replace(/\s+/g, '').toLowerCase()
 
 export const bufferToHex = (buffer: ArrayBuffer): string => {
   const array = new Uint8Array(buffer)
@@ -16,15 +20,52 @@ export const bufferToHex = (buffer: ArrayBuffer): string => {
 }
 
 export const hexToBuffer = (hex: string): ArrayBuffer => {
-  if (hex.length % 2 !== 0) {
+  const sanitized = sanitizeHex(hex)
+
+  if (sanitized.length % 2 !== 0) {
     throw new RangeError('Expected string to be an even number of characters')
   }
 
-  const array = new Uint8Array(hex.length / 2)
+  if (sanitized.length > 0 && !hexPattern.test(sanitized)) {
+    throw new RangeError(
+      'Expected string to only contain hexadecimal characters',
+    )
+  }
+
+  const array = new Uint8Array(sanitized.length / 2)
 
-  for (let i = 0; i < hex.length; i += 2) {
-    array[i / 2] = Number.parseInt(hex.substring(i, i + 2), 16)
+  for (let i = 0; i < sanitized.length; i += 2) {
+    array[i / 2] = Number.parseInt(sanitized.substring(i, i + 2), 16)
   }
 
   return array.buffer
 }
+
+export const constantTimeEqualHex = (
+  leftHex: string,
+  rightHex: string,
+): boolean => {
+  const left = sanitizeHex(leftHex)
+  const right = sanitizeHex(rightHex)
+
+  if (
+    left.length % 2 !== 0 ||
+    right.length % 2 !== 0 ||
+    !hexPattern.test(left) ||
+    !hexPattern.test(right)
+  ) {
+    return false
+  }
+
+  const leftBytes = new Uint8Array(hexToBuffer(left))
+  const rightBytes = new Uint8Array(hexToBuffer(right))
+  const maxLength = Math.max(leftBytes.length, rightBytes.length)
+
+  let mismatch = leftBytes.length ^ rightBytes.length
+
+  for (let i = 0; i < maxLength; i++) {
+    mismatch |= (leftBytes[i] ?? 0) ^ (rightBytes[i] ?? 0)
+  }
+
+  return mismatch === 0
+}

src/utils.ts

@@ -12,3 +12,9 @@ test('SRPInt should keep padding when going back and forth', () => {
   expect(SRPInt.fromHex('0000000a').toHex()).toStrictEqual('0000000a')
   expect(SRPInt.fromHex('00000000a').toHex()).toStrictEqual('00000000a')
 })
+
+test('SRPInt should reject malformed hex input', () => {
+  expect(() => SRPInt.fromHex('')).toThrow(RangeError)
+  expect(() => SRPInt.fromHex('gg')).toThrow(RangeError)
+  expect(() => SRPInt.fromHex('-01')).toThrow(RangeError)
+})