From 7f12fab3ca456d15dccce376c3dd0f9da1941f25 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 30 Jun 2021 12:29:12 -0700 Subject: [PATCH] fix(isHashMatch): check that hash starts with $ Previously, we used argon2 to verify the hash with the password. If the hash didn't start with a $, then it would enter the catch block. Now we check the hash before trying to verify it and we also throw an Error if the verify fails. This makes the isHashMatch function more robust. --- src/node/util.ts | 5 ++--- test/unit/node/util.test.ts | 11 +++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/node/util.ts b/src/node/util.ts index 9129c7e84..5cb5e3cd9 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -166,14 +166,13 @@ export const hash = async (password: string): Promise => { * Used to verify if the password matches the hash */ export const isHashMatch = async (password: string, hash: string) => { - if (password === "" || hash === "") { + if (password === "" || hash === "" || !hash.startsWith("$")) { return false } try { return await argon2.verify(hash, password) } catch (error) { - logger.error(error) - return false + throw new Error(error) } } diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index 38534c228..8fae54b7d 100644 --- a/test/unit/node/util.test.ts +++ b/test/unit/node/util.test.ts @@ -189,6 +189,17 @@ describe("isHashMatch", () => { const actual = await util.isHashMatch(password, _hash) expect(actual).toBe(false) }) + it("should return false and not throw an error if the hash doesn't start with a $", async () => { + const password = "hellowpasssword" + const _hash = "n2i$v=19$m=4096,t=3,p=1$EAoczTxVki21JDfIZpTUxg$rkXgyrW4RDGoDYrxBFD4H2DlSMEhP4h+Api1hXnGnFY" + expect(async () => await util.isHashMatch(password, _hash)).not.toThrow() + expect(await util.isHashMatch(password, _hash)).toBe(false) + }) + it("should reject the promise and throw if error", async () => { + const password = "hellowpasssword" + const _hash = "$ar2i" + expect(async () => await util.isHashMatch(password, _hash)).rejects.toThrow() + }) }) describe("hashLegacy", () => {