fix: 🐛 Add timing-safe comparison tests and update API docs

Author: damusix

API.md

@@ -1,11 +1,24 @@
 
+
 ## Methods
 
-### `randomString(<Number> size)`
+
+### `randomString(size: number): string`
+
 Returns a cryptographically strong pseudo-random data string. Takes a size argument for the length of the string.
 
-### `randomAlphanumString(<Number> size)`
+### `randomAlphanumString(size: number): string`
+
 Returns a cryptographically strong pseudo-random alphanumeric data string. Takes a size argument for the length of the string.
 
-### `randomDigits(<Number> size)`
-Returns a cryptographically strong pseudo-random data string consisting of only numerical digits (0-9). Takes a size argument for the length of the string.
+### `randomDigits(size: number): string`
+
+Returns a cryptographically strong pseudo-random data string consisting of only numerical digits (0-9). Takes a size argument for the length of the string.
+
+### `randomBits(bits: number): Buffer`
+
+Returns a Buffer of cryptographically strong pseudo-random bits. Takes a bits argument for the number of bits to generate.
+
+### `fixedTimeComparison(a: string, b: string): boolean`
+
+Performs a constant-time comparison of two strings to prevent timing attacks. Returns `true` if the strings are equal, `false` otherwise. Safe to use with strings of different lengths.

lib/index.js

@@ -73,17 +73,20 @@ exports.randomBits = function (bits) {
     return internals.random(bytes);
 };
 
-
 exports.fixedTimeComparison = function (a, b) {
 
-    try {
-        return Crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b));
-    }
-    catch (err) {
+    const bufA = Buffer.from(a);
+    const bufB = Buffer.from(b);
+
+    if (bufA.length !== bufB.length) {
+
+        Crypto.timingSafeEqual(bufA, bufA);
+
         return false;
     }
-};
 
+    return Crypto.timingSafeEqual(bufA, bufB);
+};
 
 internals.random = function (bytes) {
 

test/index.js

@@ -90,4 +90,52 @@ describe('fixedTimeComparison()', () => {
         expect(Cryptiles.fixedTimeComparison('', '')).to.be.true();
         expect(Cryptiles.fixedTimeComparison('asdas', 'asdasd')).to.be.false();
     });
+
+    it('should not throw if buffer size differs', () => {
+
+        expect(() => Cryptiles.fixedTimeComparison('a', 'ab')).to.not.throw();
+        expect(() => Cryptiles.fixedTimeComparison('abc', 'a')).to.not.throw();
+        expect(() => Cryptiles.fixedTimeComparison('', 'a')).to.not.throw();
+        expect(() => Cryptiles.fixedTimeComparison('a', '')).to.not.throw();
+    });
+
+    it('should provide constant time regardless of the size of the right-most argument', { timeout: 10000 },  () => {
+
+        // Test that comparison time is based on left argument, not right
+        // When lengths differ, we compare left to itself (constant time based on left)
+        const largeLeft = 'a'.repeat(100000);
+        const smallLeft = 'b'.repeat(10);
+        const smallRight = 'x'.repeat(10);
+        const largeRight = 'y'.repeat(100000);
+
+        const iterations = 10000;
+
+        // Warm up
+        for (let i = 0; i < 1000; ++i) {
+            Cryptiles.fixedTimeComparison(largeLeft, smallRight);
+            Cryptiles.fixedTimeComparison(smallLeft, largeRight);
+        }
+
+        // Measure large left + small right (timing should be based on large left)
+        const startLargeLeft = process.hrtime.bigint();
+        for (let i = 0; i < iterations; ++i) {
+            Cryptiles.fixedTimeComparison(largeLeft, smallRight);
+        }
+
+        const endLargeLeft = process.hrtime.bigint();
+        const largeLeftTime = Number(endLargeLeft - startLargeLeft);
+
+        // Measure small left + large right (timing should be based on small left)
+        const startSmallLeft = process.hrtime.bigint();
+        for (let i = 0; i < iterations; ++i) {
+            Cryptiles.fixedTimeComparison(smallLeft, largeRight);
+        }
+
+        const endSmallLeft = process.hrtime.bigint();
+        const smallLeftTime = Number(endSmallLeft - startSmallLeft);
+
+        // Large left should take longer than small left, proving timing is based on left
+        // Even though small left has a much larger right argument
+        expect(largeLeftTime).to.be.above(smallLeftTime);
+    });
 });