From 1ed1a34ae27d8612006dae78abf7095166299ca8 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 16 Aug 2026 19:56:43 +0200 Subject: [PATCH] fix(auth): reject ill-formed local passwords --- backend/src/auth/password.ts | 6 ++++++ backend/test/auth-password.test.ts | 22 ++++++++++++++++++++-- backend/test/local-registry.test.ts | 5 ++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/backend/src/auth/password.ts b/backend/src/auth/password.ts index 69db9af4..afc71c11 100644 --- a/backend/src/auth/password.ts +++ b/backend/src/auth/password.ts @@ -53,6 +53,12 @@ function parsePHC(encoded: string): Argon2Parameters | undefined { function passwordBytes(password: string): Buffer | undefined { if (typeof password !== "string") return undefined; + const typedPassword = password as string & { isWellFormed?: () => boolean }; + if (typeof typedPassword.isWellFormed === "function") { + if (!typedPassword.isWellFormed()) return undefined; + } else if (/[\uD800-\uDFFF]/.test(password)) { + return undefined; + } const bytes = Buffer.from(password, "utf8"); return bytes.length >= MINIMUM_PASSWORD_BYTES && bytes.length <= MAXIMUM_PASSWORD_BYTES ? bytes : undefined; } diff --git a/backend/test/auth-password.test.ts b/backend/test/auth-password.test.ts index d99f0f44..22318cb7 100644 --- a/backend/test/auth-password.test.ts +++ b/backend/test/auth-password.test.ts @@ -1,6 +1,14 @@ import { readFileSync } from "node:fs"; import { resolve } from "node:path"; -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi } from "vitest"; + +const { argon2SyncSpy } = vi.hoisted(() => ({ argon2SyncSpy: vi.fn() })); +vi.mock("node:crypto", async (importOriginal) => { + const actual = await importOriginal(); + argon2SyncSpy.mockImplementation(actual.argon2Sync); + return { ...actual, argon2Sync: argon2SyncSpy }; +}); + import { verifyPassword } from "../src/auth/password.js"; interface Argon2Vector { @@ -27,6 +35,15 @@ describe("local Argon2id password verification", () => { } }); + test("rejects ill-formed Unicode instead of authenticating as U+FFFD", () => { + const replacementPassword = "correct horse battery stap\uFFFD"; + const loneSurrogatePassword = "correct horse battery stap\uD800"; + const replacementPasswordHash = "$argon2id$v=19$m=65536,t=3,p=1$AAECAwQFBgcICQoLDA0ODw$+tAXzgaQVnNaonNvgevyG6UKlaKcwyRMi1mESNk0BvQ"; + + expect(verifyPassword(replacementPassword, replacementPasswordHash)).toBe(true); + expect(verifyPassword(loneSurrogatePassword, replacementPasswordHash)).toBe(false); + }); + test("rejects malformed and oversized PHC parameters before Argon2 allocation", () => { const password = vectors[0].password; const digest = vectors[0].phc.split("$")[5]; @@ -40,8 +57,9 @@ describe("local Argon2id password verification", () => { ]; for (const phc of cases) { - expect(() => verifyPassword(password, phc)).not.toThrow(); + argon2SyncSpy.mockClear(); expect(verifyPassword(password, phc)).toBe(false); + expect(argon2SyncSpy).not.toHaveBeenCalled(); } }); }); diff --git a/backend/test/local-registry.test.ts b/backend/test/local-registry.test.ts index 28c909c1..9f2717d8 100644 --- a/backend/test/local-registry.test.ts +++ b/backend/test/local-registry.test.ts @@ -2,7 +2,6 @@ import { chmodSync, existsSync, lstatSync, - mkdirSync, renameSync, realpathSync, symlinkSync, @@ -128,6 +127,10 @@ describe("local user registry", () => { ["duplicate normalized usernames", registryYaml(userYaml() + userYaml({ id: userId, username: "admin" }))], ["duplicate IDs", registryYaml(userYaml() + userYaml({ username: "operator" }))], ["unknown YAML fields", `${registryYaml(userYaml())}unexpected: true\n`], + ["no enabled admin", registryYaml(userYaml({ role: "user" }))], + ["duplicate roles", registryYaml(userYaml().replace(" - admin", " - admin\n - admin"))], + ["invalid password hash", registryYaml(userYaml().replace(passwordHash, "not-a-password-hash"))], + ["control character in display name", registryYaml(userYaml().replace("displayName: Admin", 'displayName: "Admin\\tUser"'))], ])("rejects %s", async (_name, contents) => { const fixture = writeRegistry(contents); await expectInvalid(createLocalUserRegistry(fixture.path).findByUsername("admin"), ["admin", passwordHash, fixture.path]);