Code comparison needs to be in constant time for security

[Demuxx]

2fa/lib/2FA.js

Line 66 in f42eef5

if (TFA.generateCode(key, i, opts) === code) return true;

The string comparison of the hotp code against the user supplied code needs to be secure against timing attacks. There are various methods to addressing this, including those in this stackoverflow:

https://stackoverflow.com/questions/31095905/whats-the-difference-between-a-secure-compare-and-a-simple

Here is a secure implementation with proof code that the timing issue exists.

const crypto = require("crypto");

function constantTimeCompare(a, b) {
  if (a.length !== b.length) {
    return false;
  }

  let diff = 0;
  for (let i = 0; i < a.length; i++) {
    diff |= a.charCodeAt(i) ^ b.charCodeAt(i);
  }

  return diff === 0;
}

function timeComparison(a, b, comparisonFunction) {
  const start = process.hrtime.bigint();
  comparisonFunction(a, b);
  const end = process.hrtime.bigint();

  return end - start;
}

const stringLength = 1000;
const almostSameString1 = "a".repeat(stringLength - 1) + "b";
const almostSameString2 = "a".repeat(stringLength);
const differentString1 = "b" + "a".repeat(stringLength - 1);
const differentString2 = "a".repeat(stringLength);

let constantTimeTotal1 = BigInt(0);
let constantTimeTotal2 = BigInt(0);
let nonConstantTimeTotal1 = BigInt(0);
let nonConstantTimeTotal2 = BigInt(0);
const iterations = 100000;

for (let i = 0; i < iterations; i++) {
  constantTimeTotal1 += timeComparison(
    almostSameString1,
    almostSameString2,
    constantTimeCompare
  );
  constantTimeTotal2 += timeComparison(
    differentString1,
    differentString2,
    constantTimeCompare
  );
  nonConstantTimeTotal1 += timeComparison(
    almostSameString1,
    almostSameString2,
    (a, b) => a === b
  );
  nonConstantTimeTotal2 += timeComparison(
    differentString1,
    differentString2,
    (a, b) => a === b
  );
}

const constantTimeAverage1 = constantTimeTotal1 / BigInt(iterations);
const constantTimeAverage2 = constantTimeTotal2 / BigInt(iterations);
const nonConstantTimeAverage1 = nonConstantTimeTotal1 / BigInt(iterations);
const nonConstantTimeAverage2 = nonConstantTimeTotal2 / BigInt(iterations);

console.log(
  "Average time for constant-time comparison when strings differ at the end:",
  constantTimeAverage1.toString()
);
console.log(
  "Average time for constant-time comparison when strings differ at the start:",
  constantTimeAverage2.toString()
);
console.log(
  "Average time for non-constant-time comparison when strings differ at the end:",
  nonConstantTimeAverage1.toString()
);
console.log(
  "Average time for non-constant-time comparison when strings differ at the start:",
  nonConstantTimeAverage2.toString()
);

[Demuxx]

Note that this leaks information about the length of the hotp code, but this is OK since all codes are 6 digits always.