fix(auth): reject ill-formed local passwords

This commit is contained in:
2026-08-16 19:56:43 +02:00
parent 5eed4f9449
commit 1ed1a34ae2
3 changed files with 30 additions and 3 deletions
+6
View File
@@ -53,6 +53,12 @@ function parsePHC(encoded: string): Argon2Parameters | undefined {
function passwordBytes(password: string): Buffer | undefined { function passwordBytes(password: string): Buffer | undefined {
if (typeof password !== "string") return 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"); const bytes = Buffer.from(password, "utf8");
return bytes.length >= MINIMUM_PASSWORD_BYTES && bytes.length <= MAXIMUM_PASSWORD_BYTES ? bytes : undefined; return bytes.length >= MINIMUM_PASSWORD_BYTES && bytes.length <= MAXIMUM_PASSWORD_BYTES ? bytes : undefined;
} }
+20 -2
View File
@@ -1,6 +1,14 @@
import { readFileSync } from "node:fs"; import { readFileSync } from "node:fs";
import { resolve } from "node:path"; 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<typeof import("node:crypto")>();
argon2SyncSpy.mockImplementation(actual.argon2Sync);
return { ...actual, argon2Sync: argon2SyncSpy };
});
import { verifyPassword } from "../src/auth/password.js"; import { verifyPassword } from "../src/auth/password.js";
interface Argon2Vector { 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", () => { test("rejects malformed and oversized PHC parameters before Argon2 allocation", () => {
const password = vectors[0].password; const password = vectors[0].password;
const digest = vectors[0].phc.split("$")[5]; const digest = vectors[0].phc.split("$")[5];
@@ -40,8 +57,9 @@ describe("local Argon2id password verification", () => {
]; ];
for (const phc of cases) { for (const phc of cases) {
expect(() => verifyPassword(password, phc)).not.toThrow(); argon2SyncSpy.mockClear();
expect(verifyPassword(password, phc)).toBe(false); expect(verifyPassword(password, phc)).toBe(false);
expect(argon2SyncSpy).not.toHaveBeenCalled();
} }
}); });
}); });
+4 -1
View File
@@ -2,7 +2,6 @@ import {
chmodSync, chmodSync,
existsSync, existsSync,
lstatSync, lstatSync,
mkdirSync,
renameSync, renameSync,
realpathSync, realpathSync,
symlinkSync, symlinkSync,
@@ -128,6 +127,10 @@ describe("local user registry", () => {
["duplicate normalized usernames", registryYaml(userYaml() + userYaml({ id: userId, username: "admin" }))], ["duplicate normalized usernames", registryYaml(userYaml() + userYaml({ id: userId, username: "admin" }))],
["duplicate IDs", registryYaml(userYaml() + userYaml({ username: "operator" }))], ["duplicate IDs", registryYaml(userYaml() + userYaml({ username: "operator" }))],
["unknown YAML fields", `${registryYaml(userYaml())}unexpected: true\n`], ["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) => { ])("rejects %s", async (_name, contents) => {
const fixture = writeRegistry(contents); const fixture = writeRegistry(contents);
await expectInvalid(createLocalUserRegistry(fixture.path).findByUsername("admin"), ["admin", passwordHash, fixture.path]); await expectInvalid(createLocalUserRegistry(fixture.path).findByUsername("admin"), ["admin", passwordHash, fixture.path]);