FIX - pw security issue

Author: MeMMa137

package-lock.json

@@ -7,8 +7,10 @@ import {
   getUserById,
   setPasswordForExistingUser,
   toPublicUser,
+  updateUserPasswordHash,
   updateUserRoleFromEmail,
 } from "./users.js";
+import { hashPasswordForStorage, verifyPasswordForLogin } from "./passwordHash.js";
 
 const passwordLoginLimiter = rateLimit({
   windowMs: 10 * 60 * 1000,
@@ -57,21 +59,6 @@ function isValidEmail(email) {
   return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
 }
 
-function hashPassword(password) {
-  const salt = crypto.randomBytes(16).toString("hex");
-  const hash = crypto.scryptSync(password, salt, 64).toString("hex");
-  return `scrypt:${salt}:${hash}`;
-}
-
-function verifyPassword(password, storedHash) {
-  const [algorithm, salt, hash] = String(storedHash || "").split(":");
-  if (algorithm !== "scrypt" || !salt || !hash) return false;
-
-  const submittedHash = crypto.scryptSync(password, salt, 64);
-  const storedBuffer = Buffer.from(hash, "hex");
-  return storedBuffer.length === submittedHash.length && crypto.timingSafeEqual(storedBuffer, submittedHash);
-}
-
 function signLocalUserId(userId) {
   const payload = String(userId);
   const signature = crypto.createHmac("sha256", localDevCookieSecret()).update(payload).digest("hex");
@@ -135,15 +122,19 @@ export function createAuthRouter() {
       let user;
 
       if (existingUser?.password_hash) {
-        if (!verifyPassword(password, existingUser.password_hash)) {
+        const outcome = verifyPasswordForLogin(password, existingUser.password_hash);
+        if (!outcome.valid) {
           res.status(401).json({ error: "Wrong email or password." });
           return;
         }
+        if (outcome.migrateToHash) {
+          await updateUserPasswordHash(existingUser.id, outcome.migrateToHash);
+        }
         user = await updateUserRoleFromEmail(existingUser.id, email);
       } else if (existingUser) {
-        user = await setPasswordForExistingUser(existingUser.id, email, hashPassword(password));
+        user = await setPasswordForExistingUser(existingUser.id, email, hashPasswordForStorage(password));
       } else {
-        user = await createUserFromEmailPassword(email, hashPassword(password));
+        user = await createUserFromEmailPassword(email, hashPasswordForStorage(password));
       }
 
       req.session.userId = user.id;

server/authRoutes.js

@@ -7,6 +7,7 @@
     "dev": "node index.js"
   },
   "dependencies": {
+    "bcryptjs": "^3.0.3",
     "cookie-parser": "^1.4.7",
     "dotenv": "^17.4.2",
     "express": "^4.21.2",

server/package.json

@@ -0,0 +1,43 @@
+import crypto from "crypto";
+import bcrypt from "bcryptjs";
+
+/** Work factor for bcrypt; OWASP suggests >= 10 for interactive logins. */
+const BCRYPT_COST = 12;
+
+/**
+ * Hash a password for storage (bcrypt).
+ * @param {string} plain
+ */
+export function hashPasswordForStorage(plain) {
+  return bcrypt.hashSync(plain, BCRYPT_COST);
+}
+
+function verifyLegacyScrypt(plain, storedHash) {
+  const [algorithm, salt, hash] = storedHash.split(":");
+  if (algorithm !== "scrypt" || !salt || !hash) return false;
+
+  /* Legacy `scrypt:` rows only; verifying with the same parameters used when they were stored.
+     New passwords use bcrypt. */
+  const submittedHash = crypto.scryptSync(plain, salt, 64); // lgtm[js/insufficient-password-hash]
+  const storedBuffer = Buffer.from(hash, "hex");
+  return storedBuffer.length === submittedHash.length && crypto.timingSafeEqual(storedBuffer, submittedHash);
+}
+
+/**
+ * Verify a password against the stored value. Supports legacy `scrypt:` rows
+ * and bcrypt (`$2a$` / `$2b$` / `$2y$`).
+ *
+ * @returns {{ valid: boolean, migrateToHash?: string }}
+ */
+export function verifyPasswordForLogin(plain, storedHash) {
+  const stored = String(storedHash || "");
+  if (!stored) return { valid: false };
+
+  if (stored.startsWith("scrypt:")) {
+    if (!verifyLegacyScrypt(plain, stored)) return { valid: false };
+    return { valid: true, migrateToHash: hashPasswordForStorage(plain) };
+  }
+
+  if (!bcrypt.compareSync(plain, stored)) return { valid: false };
+  return { valid: true };
+}

server/passwordHash.js

@@ -415,6 +415,27 @@ export async function setPasswordForExistingUser(id, email, passwordHash) {
   return result.rows[0];
 }
 
+export async function updateUserPasswordHash(id, passwordHash) {
+  if (!pool) {
+    throw new Error("DATABASE_URL is not set.");
+  }
+
+  const result = await pool.query(
+    `
+      UPDATE users
+      SET
+        password_hash = $1,
+        password_set_at = NOW(),
+        updated_at = NOW()
+      WHERE id = $2
+      RETURNING *
+    `,
+    [passwordHash, id]
+  );
+
+  return result.rows[0] ?? null;
+}
+
 export async function createUserFromEmailPassword(email, passwordHash) {
   if (!pool) {
     throw new Error("DATABASE_URL is not set.");