hectorm / otpauth

One Time Password (HOTP/TOTP) library for Node.js, Deno, Bun and browsers.
https://hectorm.github.io/otpauth/
MIT License
971 stars 56 forks source link

base32Decode throws error when stumble upon spaces in secret #547

Closed jodaka closed 1 month ago

jodaka commented 1 month ago

I've been using this library for a while and it worked perfectly fine. However recently I imported few more OTP accounts and one of them had spaces in secret. It lead to errors being thrown in base32Decode. I fixed it by simply removing all spaces. Maybe I should have placed spaces removal code in fromBase32 or somewhere else. Anyways, I thought that it might help someone, so here' s modified code:

/**
 * RFC 4648 base32 alphabet without pad.
 * @type {string}
 */ const ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
/**
 * Converts a base32 string to an Uint8Array (RFC 4648).
 * @see [LinusU/base32-decode](https://github.com/LinusU/base32-decode)
 * @param {string} str Base32 string.
 * @returns {Uint8Array} Uint8Array.
 */ const base32Decode = (str) => {
  // remove spaces
  str = str.replaceAll(" ", "");
  // Canonicalize to all upper case and remove padding if it exists.
  let end = str.length;
  while (str[end - 1] === "=") --end;
  const cstr = (end < str.length ? str.substring(0, end) : str).toUpperCase();
  const buf = new ArrayBuffer(((cstr.length * 5) / 8) | 0);
  const arr = new Uint8Array(buf);
  let bits = 0;
  let value = 0;
  let index = 0;
  for (let i = 0; i < cstr.length; i++) {
    const idx = ALPHABET.indexOf(cstr[i]);
    if (idx === -1) throw new TypeError(`Invalid character found in ${str}: ${cstr[i]}`);
    value = (value << 5) | idx;
    bits += 5;
    if (bits >= 8) {
      bits -= 8;
      arr[index++] = value >>> bits;
    }
  }
  return arr;
};
hectorm commented 1 month ago

Although spaces are not allowed by the spec, I'm aware that some issuers add them to improve readability. Some programs like Bitwarden or KeePassXC are permissive and automatically remove spaces in the secret field, I think we could do the same.