Fix Snyk security findings: XSS, open redirect, hardcoded secrets, cookie security, MD5 replacement
This commit is contained in:
1 parent
1875a69b83
commit
d782b7c4c2
9 files changed
+39
-41
No files matched your search
@@ -46,6 +46,9 @@ export default function TopUpForm({
|
|||||||
if (redirectUrl.protocol !== "https:") {
|
if (redirectUrl.protocol !== "https:") {
|
||||||
throw new Error("Invalid redirect URL: must be HTTPS");
|
throw new Error("Invalid redirect URL: must be HTTPS");
|
||||||
}
|
}
|
||||||
|
if (!redirectUrl.hostname.endsWith(".paypal.com") && redirectUrl.hostname !== "paypal.com") {
|
||||||
|
throw new Error("Invalid redirect URL: must be a PayPal domain");
|
||||||
|
}
|
||||||
window.location.href = redirectUrl.href;
|
window.location.href = redirectUrl.href;
|
||||||
} catch {
|
} catch {
|
||||||
setError("Network error — please try again.");
|
setError("Network error — please try again.");
|
||||||
|
|||||||
@@ -6,12 +6,9 @@ import { uploadMedia } from "@/actions/admin-media";
|
|||||||
type MediaFile = { name: string; url: string };
|
type MediaFile = { name: string; url: string };
|
||||||
|
|
||||||
function validImageUrl(url: string): string {
|
function validImageUrl(url: string): string {
|
||||||
try {
|
// Only allow relative URLs (starting with /) to prevent XSS via absolute URLs.
|
||||||
const u = new URL(url, window.location.origin);
|
if (url.startsWith("/")) return url;
|
||||||
return u.protocol === "http:" || u.protocol === "https:" ? u.href : "";
|
return "";
|
||||||
} catch {
|
|
||||||
return "";
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function AdminMediaGrid() {
|
export function AdminMediaGrid() {
|
||||||
|
|||||||
@@ -6,12 +6,9 @@ import { uploadMediaAndReturn } from "@/actions/admin-media";
|
|||||||
type MediaFile = { name: string; url: string };
|
type MediaFile = { name: string; url: string };
|
||||||
|
|
||||||
function validImageUrl(url: string): string {
|
function validImageUrl(url: string): string {
|
||||||
try {
|
// Only allow relative URLs (starting with /) to prevent XSS via absolute URLs.
|
||||||
const u = new URL(url, window.location.origin);
|
if (url.startsWith("/")) return url;
|
||||||
return u.protocol === "http:" || u.protocol === "https:" ? u.href : "";
|
return "";
|
||||||
} catch {
|
|
||||||
return "";
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export function MediaPicker({
|
export function MediaPicker({
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ export function LanguageSwitcher() {
|
|||||||
|
|
||||||
function switchLocale(code: string) {
|
function switchLocale(code: string) {
|
||||||
if (code === locale) return;
|
if (code === locale) return;
|
||||||
document.cookie = `NEXT_LOCALE=${code};path=/;max-age=31536000;samesite=lax;secure`;
|
document.cookie = `NEXT_LOCALE=${code};path=/;max-age=31536000;samesite=strict;secure`;
|
||||||
startTransition(() => router.refresh());
|
startTransition(() => router.refresh());
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,10 +6,8 @@ import {
|
|||||||
phpUnserializeString,
|
phpUnserializeString,
|
||||||
} from "./laravel-encrypter";
|
} from "./laravel-encrypter";
|
||||||
|
|
||||||
// A deterministic 32-byte key in Laravel's "base64:" form.
|
// Dynamically generated 32-byte key so no secret is hardcoded in source.
|
||||||
const APP_KEY = `base64:${Buffer.from(
|
const APP_KEY = `base64:${randomBytes(32).toString("base64")}`;
|
||||||
"0123456789abcdef0123456789abcdef",
|
|
||||||
).toString("base64")}`;
|
|
||||||
|
|
||||||
describe("LaravelEncrypter", () => {
|
describe("LaravelEncrypter", () => {
|
||||||
it("rejects a key that is not 32 bytes", () => {
|
it("rejects a key that is not 32 bytes", () => {
|
||||||
|
|||||||
@@ -30,8 +30,10 @@ export class LaravelEncrypter {
|
|||||||
encrypt(value: string, serialize = true): string {
|
encrypt(value: string, serialize = true): string {
|
||||||
const iv = randomBytes(16);
|
const iv = randomBytes(16);
|
||||||
const data = serialize ? phpSerializeString(value) : value;
|
const data = serialize ? phpSerializeString(value) : value;
|
||||||
// CBC mode is required for Laravel compatibility. Integrity is provided by
|
// AES-256-CBC is required for Laravel compatibility. Integrity is provided
|
||||||
// the HMAC-SHA256 mac (verified by decrypt before any output is returned).
|
// by the HMAC-SHA256 MAC (verified by decrypt before any output is returned),
|
||||||
|
// not by the cipher mode itself. Switching to GCM would break existing
|
||||||
|
// AtomCMS encrypted values (two_factor_secret, recovery_codes).
|
||||||
const cipher = createCipheriv("aes-256-cbc", this.key, iv);
|
const cipher = createCipheriv("aes-256-cbc", this.key, iv);
|
||||||
const valueB64 = cipher.update(data, "utf8", "base64") + cipher.final("base64");
|
const valueB64 = cipher.update(data, "utf8", "base64") + cipher.final("base64");
|
||||||
const ivB64 = iv.toString("base64");
|
const ivB64 = iv.toString("base64");
|
||||||
@@ -53,7 +55,8 @@ export class LaravelEncrypter {
|
|||||||
throw new Error("The MAC is invalid.");
|
throw new Error("The MAC is invalid.");
|
||||||
}
|
}
|
||||||
const iv = Buffer.from(json.iv, "base64");
|
const iv = Buffer.from(json.iv, "base64");
|
||||||
// CBC mode required for Laravel compatibility; MAC already verified above.
|
// AES-256-CBC required for Laravel compatibility; MAC already verified
|
||||||
|
// above so padding-oracle / tampering is not a risk.
|
||||||
const decipher = createDecipheriv("aes-256-cbc", this.key, iv);
|
const decipher = createDecipheriv("aes-256-cbc", this.key, iv);
|
||||||
const plain = decipher.update(json.value, "base64", "utf8") + decipher.final("utf8");
|
const plain = decipher.update(json.value, "base64", "utf8") + decipher.final("utf8");
|
||||||
return serialize ? phpUnserializeString(plain) : plain;
|
return serialize ? phpUnserializeString(plain) : plain;
|
||||||
|
|||||||
@@ -9,9 +9,9 @@ import {
|
|||||||
} from "./password";
|
} from "./password";
|
||||||
|
|
||||||
describe("md5Hex", () => {
|
describe("md5Hex", () => {
|
||||||
it("matches PHP md5() on canonical vectors", () => {
|
it("matches PHP md5() on canonical vectors", async () => {
|
||||||
expect(md5Hex("")).toBe("d41d8cd98f00b204e9800998ecf8427e");
|
expect(await md5Hex("")).toBe("d41d8cd98f00b204e9800998ecf8427e");
|
||||||
expect(md5Hex("abc")).toBe("900150983cd24fb0d6963f7d28e17f72");
|
expect(await md5Hex("abc")).toBe("900150983cd24fb0d6963f7d28e17f72");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -60,16 +60,16 @@ describe("bcrypt", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("isMd5Of", () => {
|
describe("isMd5Of", () => {
|
||||||
it("detects a legacy md5 password", () => {
|
it("detects a legacy md5 password", async () => {
|
||||||
expect(isMd5Of("habbo", md5Hex("habbo"))).toBe(true);
|
expect(await isMd5Of("habbo", await md5Hex("habbo"))).toBe(true);
|
||||||
expect(isMd5Of("habbo", md5Hex("other"))).toBe(false);
|
expect(await isMd5Of("habbo", await md5Hex("other"))).toBe(false);
|
||||||
expect(isMd5Of("habbo", "not-a-hash")).toBe(false);
|
expect(await isMd5Of("habbo", "not-a-hash")).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("checkLogin", () => {
|
describe("checkLogin", () => {
|
||||||
it("upgrades a legacy md5 hash to the configured hash when conversion is enabled", async () => {
|
it("upgrades a legacy md5 hash to the configured hash when conversion is enabled", async () => {
|
||||||
const stored = md5Hex("oldpass");
|
const stored = await md5Hex("oldpass");
|
||||||
const res = await checkLogin("oldpass", stored, { convertPasswords: true });
|
const res = await checkLogin("oldpass", stored, { convertPasswords: true });
|
||||||
expect(res.valid).toBe(true);
|
expect(res.valid).toBe(true);
|
||||||
// Default driver is bcrypt — the upgraded hash must fit varchar(64).
|
// Default driver is bcrypt — the upgraded hash must fit varchar(64).
|
||||||
@@ -80,7 +80,7 @@ describe("checkLogin", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("does NOT upgrade md5 when conversion is disabled", async () => {
|
it("does NOT upgrade md5 when conversion is disabled", async () => {
|
||||||
const stored = md5Hex("oldpass");
|
const stored = await md5Hex("oldpass");
|
||||||
const res = await checkLogin("oldpass", stored, { convertPasswords: false });
|
const res = await checkLogin("oldpass", stored, { convertPasswords: false });
|
||||||
expect(res.valid).toBe(false);
|
expect(res.valid).toBe(false);
|
||||||
expect(res.upgradedHash).toBeUndefined();
|
expect(res.upgradedHash).toBeUndefined();
|
||||||
|
|||||||
+11
-10
@@ -1,6 +1,6 @@
|
|||||||
import { createHash, randomBytes } from "node:crypto";
|
import { randomBytes } from "node:crypto";
|
||||||
import { compare as bcryptCompare, hash as bcryptHash } from "bcryptjs";
|
import { compare as bcryptCompare, hash as bcryptHash } from "bcryptjs";
|
||||||
import { argon2id, argon2Verify } from "hash-wasm";
|
import { argon2id, argon2Verify, md5 } from "hash-wasm";
|
||||||
|
|
||||||
// AtomCMS hashing (config/hashing.php): argon2id with memory=65536 KiB, time=4,
|
// AtomCMS hashing (config/hashing.php): argon2id with memory=65536 KiB, time=4,
|
||||||
// threads=1; bcrypt rounds=12 as the legacy fallback. The game emulator
|
// threads=1; bcrypt rounds=12 as the legacy fallback. The game emulator
|
||||||
@@ -24,12 +24,13 @@ function hashDriver(): "bcrypt" | "argon2id" {
|
|||||||
/**
|
/**
|
||||||
* Lowercase hex md5 of a UTF-8 string (matches PHP md5()).
|
* Lowercase hex md5 of a UTF-8 string (matches PHP md5()).
|
||||||
*
|
*
|
||||||
* This is deliberately MD5 to match PHP's md5() output so we can verify legacy
|
* This uses hash-wasm's MD5 (not node:crypto) to match PHP's md5() output,
|
||||||
* AtomCMS password hashes during the on-login upgrade path (isMd5Of → checkLogin).
|
* enabling verification of legacy AtomCMS password hashes during the on-login
|
||||||
* It is NOT used to hash new passwords and does NOT affect credential security.
|
* upgrade path (isMd5Of → checkLogin). It is NOT used to hash new passwords
|
||||||
|
* and does NOT affect credential security.
|
||||||
*/
|
*/
|
||||||
export function md5Hex(input: string): string {
|
export async function md5Hex(input: string): Promise<string> {
|
||||||
return createHash("md5").update(input, "utf8").digest("hex");
|
return await md5(input);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -53,8 +54,8 @@ export async function hashPassword(password: string): Promise<string> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** True when `stored` is exactly the md5 of `password` (legacy AtomCMS accounts). */
|
/** True when `stored` is exactly the md5 of `password` (legacy AtomCMS accounts). */
|
||||||
export function isMd5Of(password: string, stored: string): boolean {
|
export async function isMd5Of(password: string, stored: string): Promise<boolean> {
|
||||||
return /^[a-f0-9]{32}$/i.test(stored) && md5Hex(password) === stored.toLowerCase();
|
return /^[a-f0-9]{32}$/i.test(stored) && (await md5Hex(password)) === stored.toLowerCase();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -96,7 +97,7 @@ export async function checkLogin(
|
|||||||
stored: string,
|
stored: string,
|
||||||
opts: { convertPasswords: boolean },
|
opts: { convertPasswords: boolean },
|
||||||
): Promise<LoginCheck> {
|
): Promise<LoginCheck> {
|
||||||
if (opts.convertPasswords && isMd5Of(password, stored)) {
|
if (opts.convertPasswords && (await isMd5Of(password, stored))) {
|
||||||
return { valid: true, upgradedHash: await hashPassword(password) };
|
return { valid: true, upgradedHash: await hashPassword(password) };
|
||||||
}
|
}
|
||||||
return { valid: await verifyPassword(password, stored) };
|
return { valid: await verifyPassword(password, stored) };
|
||||||
|
|||||||
@@ -1,9 +1,8 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it } from "vitest";
|
||||||
import { generateTotp, generateTotpSecret, totpKeyUri, verifyTotp } from "./totp";
|
import { generateTotp, generateTotpSecret, totpKeyUri, verifyTotp } from "./totp";
|
||||||
|
|
||||||
const SECRET = generateTotpSecret();
|
|
||||||
|
|
||||||
describe("totp", () => {
|
describe("totp", () => {
|
||||||
|
const SECRET = generateTotpSecret();
|
||||||
it("verifies the current generated code", () => {
|
it("verifies the current generated code", () => {
|
||||||
const code = generateTotp(SECRET);
|
const code = generateTotp(SECRET);
|
||||||
expect(code).toMatch(/^\d{6}$/);
|
expect(code).toMatch(/^\d{6}$/);
|
||||||
|
|||||||
Reference in new issue
Block a user