From 0c468a616d7e56126dc468150f6a5a92e530b8e4 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 13 Jul 2022 11:28:55 +0200 Subject: [PATCH] Merge pull request from GHSA-376v-xgjx-7mfr * Correctly use crypto.timingSafeEqual Signed-off-by: Matteo Collina Co-Authored-By: Uzlopak * apply requested change Co-authored-by: Uzlopak --- plugin.js | 19 ++++++++++++++----- test/decorate.test.js | 8 ++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/plugin.js b/plugin.js index 41efc69..93f0fbb 100644 --- a/plugin.js +++ b/plugin.js @@ -18,6 +18,13 @@ function verifyBearerAuthFactory (options) { if (_options.keys instanceof Set) _options.keys = Array.from(_options.keys) const { keys, errorResponse, contentType, bearerType, auth, addHook = true, verifyErrorLogLevel = 'error' } = _options + for (let i = 0, il = keys.length; i < il; ++i) { + if (typeof keys[i] !== 'string') { + throw new Error('options.keys has to contain only string entries') + } + keys[i] = Buffer.from(keys[i]) + } + return function verifyBearerAuth (request, reply, done) { const header = request.raw.headers.authorization if (!header) { @@ -89,17 +96,19 @@ function verifyBearerAuthFactory (options) { } function authenticate (keys, key) { - return keys.findIndex((a) => compare(a, key)) !== -1 + const b = Buffer.from(key) + return keys.findIndex((a) => compare(a, b)) !== -1 } // perform constant-time comparison to prevent timing attacks function compare (a, b) { - try { - // may throw if they have different length, can't convert to Buffer, etc... - return crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b)) - } catch { + if (a.length !== b.length) { + // Delay return with cryptographically secure timing check. + crypto.timingSafeEqual(a, a) return false } + + return crypto.timingSafeEqual(a, b) } function plugin (fastify, options, done) { diff --git a/test/decorate.test.js b/test/decorate.test.js index c7c56d0..5dd2ac0 100644 --- a/test/decorate.test.js +++ b/test/decorate.test.js @@ -20,3 +20,11 @@ test('verifyBearerAuthFactory', (t) => { t.ok(fastify.verifyBearerAuthFactory) }) }) + +test('verifyBearerAuthFactory', (t) => { + t.plan(1) + fastify.ready(() => { + const keys = { keys: new Set([123456]) } + t.throws(() => fastify.verifyBearerAuthFactory(keys), /keys has to contain only string entries/) + }) +})