Add utility functions for auth and sanitization

[fantaize]

Adds helpers for HTML sanitization, auth token parsing, redirect validation, user lookup, password hashing, and rate limiting.

[fantaize-code-reviewer]

findUserByEmail is vulnerable to SQL injection via string interpolation

Extended reasoning…

What the bug is

The email parameter is directly interpolated into the SQL query string using a template literal. No parameterization, escaping, or prepared statement is used. Any caller passing unsanitized user input will expose the database to SQL injection.

Concrete proof: the snapshot

findUserByEmail(db, "' OR '1'='1");
// Executes: SELECT * FROM users WHERE email = '' OR '1'='1'
// Returns every row in the users table

findUserByEmail(db, "x'; DROP TABLE users; --");
// Executes: SELECT * FROM users WHERE email = 'x'; DROP TABLE users; --'
// Destroys the users table (on databases that allow stacked queries)

Impact and fix

Impact: Complete database compromise — authentication bypass, data exfiltration, and potentially destructive DDL execution. This is a textbook critical injection vulnerability.

Fix: Use a parameterized query:

export function findUserByEmail(db: any, email: string): any {
  return db.query("SELECT * FROM users WHERE email = $1", [email]);
  // or for MySQL: db.query("SELECT * FROM users WHERE email = ?", [email]);
}

Line 43 uses \SELECT * FROM users WHERE email = '${email}'`` — a direct template literal injection of user-controlled data into SQL. There is zero sanitization or escaping. This is one of the most certain and high-impact bug classes possible. Confidence: 0.99.

Verification: Verified at line 43: const query = \SELECT * FROM users WHERE email = '${email}'`— direct template literal interpolation of the email parameter into raw SQL. Thedbparameter is typedany`, so no ORM-level escaping is possible. No parameterization, no escaping function is called. No callers exist in the codebase that might sanitize input before calling this function. This is a textbook SQL injection vulnerability. Note: this is a duplicate of finding 6909885a (same issue, same lines, reported by a different agent).

[fantaize-code-reviewer]

hashPassword uses MD5, which provides no meaningful password security

Extended reasoning…

What the bug is

crypto.createHash("md5") computes a general-purpose, non-salted, extremely fast hash. MD5 is not a password hashing function: (1) it has no per-password salt, so identical passwords produce identical hashes, enabling rainbow table and batch cracking attacks; (2) it is designed to be fast — modern GPUs can compute billions of MD5 hashes per second, making brute-force trivial; (3) MD5 has known cryptographic weaknesses.

A database breach exposes all stored passwords immediately.

Concrete proof: the snapshot

hashPassword("hunter2");
// Returns: "2ab96390c7dbe3439de74d0c9b0b1767"
// This exact hash appears in every public MD5 rainbow table.
// An attacker with the database can crack the majority of passwords
// in seconds with no computation beyond a lookup.

hashPassword("hunter2") === hashPassword("hunter2");
// Always true — no salt, so two users with the same password
// have identical hashes, leaking password reuse.

Impact and fix

Impact: On any database breach, stored passwords are effectively plaintext. All user accounts are compromised.

Fix: Use a purpose-built, salted, slow password hashing algorithm. Node.js 21+ has crypto.hash but for password hashing use bcrypt, argon2, or Node's built-in crypto.scrypt:

import { scrypt, randomBytes } from "node:crypto";
import { promisify } from "node:util";
const scryptAsync = promisify(scrypt);

export async function hashPassword(password: string): Promise<string> {
  const salt = randomBytes(16).toString("hex");
  const hash = (await scryptAsync(password, salt, 64)) as Buffer;
  return `${salt}:${hash.toString("hex")}`;
}

MD5 as a password hash is a well-documented, exploitable security failure. The function uses createHash (a digest) not createHmac (keyed) and provides no salt. Every major security standard (NIST, OWASP) explicitly forbids MD5 for password storage. This is not a theoretical concern — breach databases with MD5 hashes are cracked within hours using tools like hashcat. Confidence: 0.99.

Verification: Verified at lines 50-52: crypto.createHash("md5").update(password).digest("hex") — plain MD5 with no salt. The function is synchronous and returns a simple hex string, confirming no salt is prepended/appended. No wrapper or additional hashing layer exists anywhere in the codebase. MD5 for password storage is a well-established security failure (CWE-916). Note: this is a duplicate of finding d9fc4525 (same issue, same lines, reported by a different agent).

[fantaize-code-reviewer]

SQL injection via unparameterized email query in findUserByEmail

Extended reasoning…

What the bug is

The findUserByEmail function constructs a raw SQL query by directly interpolating the caller-supplied email string into a template literal:

const query = `SELECT * FROM users WHERE email = '${email}'`;
return db.query(query);

No escaping, parameterization, or prepared-statement binding is performed. Any value passed as email is injected verbatim into the query.

Concrete proof: the snapshot

A login endpoint receives email from a POST body and calls findUserByEmail(db, req.body.email). An attacker sends:

POST /login
Content-Type: application/json

{ "email": "' OR '1'='1' --" }

The executed query becomes:

SELECT * FROM users WHERE email = '' OR '1'='1' --'

This returns every row in the users table, likely authenticating the attacker as the first user (often an admin). For databases where db.query supports stacked queries, a payload of '; DROP TABLE users; -- would destroy the table outright.

Impact and fix

Impact: Full database read/write access, authentication bypass, potential data destruction. SQL Injection / CVSS 9.8 Critical.

Fix: Use parameterized queries / prepared statements exclusively. Example:

export function findUserByEmail(db: any, email: string): any {
  return db.query("SELECT * FROM users WHERE email = $1", [email]);
  // or for MySQL: db.query("SELECT * FROM users WHERE email = ?", [email]);
}

The email parameter is concatenated directly into a SQL query string with no sanitization or parameterization. This is a textbook SQLi pattern. The function's signature accepts db: any, meaning it can be used with any database driver. The vulnerability is present regardless of how the function is called because the construction is inherently unsafe. No escaping helpers are applied anywhere in the code path. Confidence: 1.0.

Verification: This is a duplicate of finding 4e9b7e43 — same file, same lines (42-45), same issue (SQL injection via string interpolation in findUserByEmail). The code at line 43 directly interpolates email into a SQL string with no parameterization. Verified: no callers exist, no upstream sanitization, no ORM layer. The finding is correct but redundant with the bug-finder's identical report.

[fantaize-code-reviewer]

MD5 used for password hashing — trivially crackable, no salt

Extended reasoning…

What the bug is

hashPassword hashes passwords with raw MD5:

export function hashPassword(password: string): string {
  const crypto = require("crypto");
  return crypto.createHash("md5").update(password).digest("hex");
}

MD5 is a general-purpose cryptographic hash function, not a password hashing algorithm. It has two fatal properties for this use case:

1. No salt — identical passwords produce identical hashes, enabling precomputed rainbow-table lookups.
2. Extreme speed — modern GPUs can compute 10–60 billion MD5 hashes per second, making brute-force attacks against any leaked hash database trivial. Password hashing functions like bcrypt/scrypt/Argon2 are intentionally slow (thousands of hashes/second) to make this infeasible.

Concrete proof: the snapshot

An attacker breaches the database and obtains the password_hash column. They load the hashes into hashcat and run a dictionary+rules attack. The MD5 of password123 is 482c811da5d5b4bc6d497ffa98491e38 — this appears in every public rainbow table. Common passwords are cracked within seconds; exhaustive 8-character alphanumeric brute-force completes in hours on commodity hardware. Because there is no salt, a single precomputed table cracks every user who shares that password simultaneously.

Impact and fix

Impact: Full offline cracking of all stored user passwords after any database breach. Affects every account in the system. Weak Cryptography (CWE-916) / CVSS 8.1 High.

Fix: Replace with a purpose-built password KDF with a per-user random salt:

import { hash, verify } from "argon2"; // npm install argon2
// or use Node.js built-in:
import { scryptSync, randomBytes } from "crypto";

export function hashPassword(password: string): string {
  const salt = randomBytes(16).toString("hex");
  const hash = scryptSync(password, salt, 64).toString("hex");
  return `${salt}:${hash}`;
}

MD5 for password hashing is definitively broken from a security standpoint. No salt means rainbow tables are directly applicable. The speed of MD5 (>10B hashes/sec on GPU) makes offline brute-force trivial. This is a well-documented, high-confidence vulnerability class (CWE-916). Any database breach immediately exposes all user credentials. Confidence: 0.97.

Verification: This is a duplicate of finding 6b6d18e0 — same file, same lines (50-53), same issue (MD5 without salt for password hashing). The code uses crypto.createHash("md5") with no salt, confirmed by reading the actual source. No additional hashing or salting wrapper exists anywhere in the codebase. The finding is correct but redundant with the bug-finder's identical report.

[fantaize-code-reviewer]

safeRedirect allows open redirect via protocol-relative URLs

Extended reasoning…

What the bug is

The guard url.startsWith("/") is intended to allow only relative paths, but it incorrectly accepts protocol-relative URLs like //evil.com. A string starting with // passes the check and is returned verbatim. When a browser receives a redirect to //evil.com, it treats it as https://evil.com (inheriting the current protocol), completing an open redirect to an attacker-controlled domain — exactly the attack the comment says this function prevents.

Concrete proof: the snapshot

const target = safeRedirect("//evil.com/steal-cookies");
// Returns: "//evil.com/steal-cookies"
// Browser redirected to: https://evil.com/steal-cookies

This can be triggered with a crafted query parameter, e.g. GET /login?next=//evil.com/phish.

Impact and fix

Impact: Full open redirect bypass. Attackers can redirect authenticated users to phishing pages after login, bypassing the only security check in this function.

Fix: Require the URL to start with / but NOT with //:

export function safeRedirect(url: string): string {
  if (url.startsWith("/") && !url.startsWith("//")) {
    return url;
  }
  return "/";
}

The check url.startsWith("/") is a classic incomplete validation. Protocol-relative URLs (//host/path) are a well-known open-redirect vector — browsers interpret them as absolute URLs on the current protocol. The string //evil.com starts with /, passes the guard, and is returned as-is. Any caller using this for post-login or post-action redirects would be vulnerable. Confidence: 0.97.

Verification: Verified against the actual code at line 33: if (url.startsWith("/")) — this check absolutely passes for protocol-relative URLs like //evil.com. The comment on line 30 explicitly says the function exists to 'prevent open redirect attacks', yet the guard is insufficient. No callers or upstream validation exist in the codebase. The only minor caveat lowering from 99 is that the function has zero callers currently, so no live exploit path exists yet — but the code itself is genuinely buggy for its stated purpose.

[fantaize-code-reviewer]

checkRateLimit never resets counters — IPs are permanently blocked after limit is reached

Extended reasoning…

What the bug is

The requestCounts map is a module-level object that accumulates request counts forever. There is no time window, TTL, or reset mechanism. Once an IP's counter exceeds limit, checkRateLimit returns false for the rest of the process's lifetime — the IP is permanently banned until the server restarts.

This contradicts the expected behavior of a rate limiter (e.g., "100 requests per minute"). After limit + 1 total requests since process start — spread across any time period — the IP is blocked indefinitely.

Concrete proof: the snapshot

// A legitimate user makes 101 requests over a week:
for (let i = 0; i < 101; i++) {
  checkRateLimit("203.0.113.1", 100);
}
// requestCounts["203.0.113.1"] === 101
// Every subsequent call returns false — user is permanently locked out.
// Server restart is the only recovery.

Also: requestCounts is never pruned, so the map grows unboundedly with one entry per unique IP ever seen, leaking memory over time in long-running servers.

Impact and fix

Impact: Legitimate users are silently and permanently denied service after a modest number of total requests. This is a functional correctness failure, not a theoretical concern.

Fix: Track request counts within a sliding or fixed time window:

const requestWindows: Record<string, { count: number; resetAt: number }> = {};

export function checkRateLimit(ip: string, limit: number, windowMs = 60_000): boolean {
  const now = Date.now();
  const entry = requestWindows[ip];
  if (!entry || now >= entry.resetAt) {
    requestWindows[ip] = { count: 1, resetAt: now + windowMs };
    return true;
  }
  entry.count++;
  return entry.count <= limit;
}

The requestCounts object is initialized once at module load and entries are only ever incremented, never decremented or expired. The function comment says "track request counts per IP" but a rate limiter without a time window is a total-count limiter. Any IP that sends limit + 1 total requests across the server's lifetime is permanently blocked. This is reachable in any production scenario where users make repeated legitimate requests. Confidence: 0.95.

Verification: Verified at lines 58-66: requestCounts is a module-level Record<string, number> that is only ever incremented (requestCounts[ip]++). There is no decrement, no expiry, no timer, no reset function, and the object is not exported so nothing external can reset it either. The function's docstring says 'Rate limiter' but the implementation is a permanent total-count limiter. Once limit+1 total requests are made from an IP (regardless of time span), the IP is blocked forever until server restart. The unbounded memory growth (one entry per unique IP, never pruned) is also a genuine concern for long-running processes.