fix: prevent certain offset values to address out of hashBytes (#23)

Author: szabolcsnagy

index.js

@@ -37,7 +37,7 @@ const DEFAULT_PERIOD = 30
  * @param {string} [options.charSet='0123456789'] - The character set to use, defaults to the numbers 0-9.
  * @returns {Promise<string>} The generated HOTP.
  */
-async function generateHOTP(
+export async function generateHOTP(
 	secret,
 	{
 		counter = 0,
@@ -56,28 +56,31 @@ async function generateHOTP(
 	)
 	const signature = await crypto.subtle.sign('HMAC', key, byteCounter)
   const hashBytes = new Uint8Array(signature)
-  // offset is always the last 4 bits of the signature; 0-15
-	const offset = hashBytes[hashBytes.length-1] & 0xf
+  // offset is always the last 4 bits of the signature; its value: 0-15
+  const offset = hashBytes[hashBytes.length - 1] & 0xf
 
   let hotpVal = 0n
   if (digits === 6) {
     // stay compatible with the authenticator apps and only use the bottom 32 bits of BigInt
     hotpVal = 0n |
-    BigInt((hashBytes[offset] & 0x7f)) << 24n |
-    BigInt((hashBytes[offset + 1])) << 16n |
-    BigInt((hashBytes[offset + 2])) << 8n |
+    BigInt(hashBytes[offset] & 0x7f) << 24n |
+    BigInt(hashBytes[offset + 1]) << 16n |
+    BigInt(hashBytes[offset + 2]) << 8n |
     BigInt(hashBytes[offset + 3])
   } else {
     // otherwise create a 64bit value from the hashBytes
     hotpVal = 0n |
-    BigInt((hashBytes[offset] & 0x7f)) << 56n |
-    BigInt((hashBytes[offset + 1])) << 48n |
-    BigInt((hashBytes[offset + 2])) << 40n |
-    BigInt((hashBytes[offset + 3])) << 32n |
-    BigInt((hashBytes[offset + 4])) << 24n |
-    BigInt((hashBytes[offset + 5])) << 16n |
-    BigInt((hashBytes[offset + 6])) << 8n |
-    BigInt(hashBytes[offset + 7])
+    BigInt(hashBytes[offset] & 0x7f) << 56n |
+    BigInt(hashBytes[offset + 1]) << 48n |
+    BigInt(hashBytes[offset + 2]) << 40n |
+    BigInt(hashBytes[offset + 3]) << 32n |
+    BigInt(hashBytes[offset + 4]) << 24n |
+    
+    // we have only 20 hashBytes; if offset is 15 these indexes are out of the hashBytes
+    // fallback to zero
+    BigInt(hashBytes[offset + 5] ?? 0n) << 16n |
+    BigInt(hashBytes[offset + 6] ?? 0n) << 8n |
+    BigInt(hashBytes[offset + 7] ?? 0n)
   }
 
 	let hotp = ''

index.test.js

@@ -1,7 +1,8 @@
 import assert from 'node:assert'
 import { test } from 'node:test'
 import base32Encode from 'base32-encode'
-import { generateTOTP, getTOTPAuthUri, verifyTOTP } from './index.js'
+import base32Decode from 'base32-decode'
+import { generateTOTP, getTOTPAuthUri, verifyTOTP, generateHOTP } from './index.js'
 
 test('OTP can be generated and verified', async () => {
 	const { secret, otp, algorithm, period, digits } = await generateTOTP()
@@ -145,3 +146,18 @@ test('OTP with digits > 6 should not pad with first character of charSet', async
 		)
 	}
 })
+
+
+test('generateHOTP works with maximum HMAC offset value',  async () => {
+  await assert.doesNotReject( async() => {
+    // These specific secret and counter values will cause offset to be 15
+    const secret = '6YY3NUMNTQ73NRH3';
+    const counter = 57988074;
+      await generateHOTP(base32Decode(secret, 'RFC4648'),{
+        counter,
+        digits: 12, // trigger the use of the 64bit htopVal
+        algorithm: 'SHA-1',
+        charSet: '0123456789',
+      });
+    });
+})