Keyang / htoken

Node.JS CLI(and Lib) for HOTP (RFC 4226) google authenticator token
MIT License
2 stars 1 forks source link

Year 2038 problem #2

Open swansontec opened 6 years ago

swansontec commented 6 years ago

Doing a security audit of the code, it appears to be subject to the year 2038 problem. Specifically, the following lines are only capable of dealing with 32-bit values:

var counterBuf = new Buffer(8);
var c = counter;
for (var i = 0; i < 8; i++) {
  counterBuf[7 - i] = c & 255;
  c = c >> 8;
}

This is because the JS bitwise operators (>>) always truncate their values to 32 bits.

swansontec commented 6 years ago

Ok, this is not a problem after all - the seconds since epoch get divided by 30 before going into this algorithm, so it should be good for a few hundred years.

Keyang commented 6 years ago

Thanks for the feedback. Let me know if it can be closed.

swansontec commented 6 years ago

This isn't a problem In practice when using the HTOP algorithm as a TOTP, since the numbers will fit nicely within 2^32 for the near future. So yes, you can close this if you like.

Nevertheless, I have decided to re-implement the rfc4226 algorithm in my own project, rather than trust NPM. Here is my solution to number-conversion problem:

export function numberToBe64 (number: number): Uint8Array {
  const high = Math.floor(number / 4294967296)
  return new Uint8Array([
    (high >> 24) & 0xff,
    (high >> 16) & 0xff,
    (high >> 8) & 0xff,
    high & 0xff,
    (number >> 24) & 0xff,
    (number >> 16) & 0xff,
    (number >> 8) & 0xff,
    number & 0xff
  ])
}

Here are some test vectors:

const cases = [
  // Powers of 2, plus 1:
  [1, [0, 0, 0, 0, 0, 0, 0, 1]],
  [257, [0, 0, 0, 0, 0, 0, 1, 1]],
  [65537, [0, 0, 0, 0, 0, 1, 0, 1]],
  [16777217, [0, 0, 0, 0, 1, 0, 0, 1]],
  [4294967297, [0, 0, 0, 1, 0, 0, 0, 1]],
  [1099511627777, [0, 0, 1, 0, 0, 0, 0, 1]],
  [281474976710657, [0, 1, 0, 0, 0, 0, 0, 1]],
  [72057594037927937, [1, 0, 0, 0, 0, 0, 0, 0]], // Truncated
  // The edge of the representable integers:
  [9007199254740991, [0, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]],
  [9007199254740992, [0, 0x20, 0, 0, 0, 0, 0, 0]],
  [9007199254740993, [0, 0x20, 0, 0, 0, 0, 0, 0]], // Truncated
  [9007199254740994, [0, 0x20, 0, 0, 0, 0, 0, 2]],
  // Fractions:
  [0.75, [0, 0, 0, 0, 0, 0, 0, 0]],
  [1.75, [0, 0, 0, 0, 0, 0, 0, 1]],
  // Negative numbers:
  [-1, [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]],
  [-256, [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0]],
  [-257, [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff]],
  [-4294967296, [0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0]],
  [-4294967297, [0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, 0xff]],
  [-9007199254740992, [0xff, 0xe0, 0, 0, 0, 0, 0, 0]]
]

for (const [number, array] of cases) {
  expect(numberToBe64(number)).to.deep.equal(new Uint8Array(array))
}

As you can see, this handles a much larger range than 2^32, right up to the limits of the double-precision floating-point format Javascript uses. Feel free to use this if you like.