diff --git a/create-critter.php b/create-critter.php index 862d326..86b821d 100644 --- a/create-critter.php +++ b/create-critter.php @@ -327,15 +327,23 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && $userCredits > 0 && !$dailyLimitRea if ($imageData !== false && strlen($imageData) > 0 && strlen($imageData) <= 15 * 1024 * 1024) { $imageInfo = @getimagesizefromstring($imageData); - $allowedImageTypes = ['image/webp', 'image/png', 'image/jpeg']; + // Extension is derived from the detected mime type below, so it always + // matches the actual bytes on disk instead of assuming every allowed + // type is webp. + $imageExtensionsByMime = [ + 'image/webp' => 'webp', + 'image/png' => 'png', + 'image/jpeg' => 'jpg', + ]; + $detectedMime = $imageInfo['mime'] ?? ''; - if ($imageInfo !== false && in_array($imageInfo['mime'] ?? '', $allowedImageTypes, true)) { + if ($imageInfo !== false && isset($imageExtensionsByMime[$detectedMime])) { $imagesDir = __DIR__ . '/critter-images'; if (!is_dir($imagesDir)) { mkdir($imagesDir, 0755, true); } - $candidateFilename = bin2hex(random_bytes(16)) . '.webp'; + $candidateFilename = bin2hex(random_bytes(16)) . '.' . $imageExtensionsByMime[$detectedMime]; $bytesWritten = file_put_contents($imagesDir . '/' . $candidateFilename, $imageData); if ($bytesWritten !== false) { $imageFilename = $candidateFilename; diff --git a/forgot-password.php b/forgot-password.php index 993d4fe..6f3c432 100644 --- a/forgot-password.php +++ b/forgot-password.php @@ -51,13 +51,27 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown'); $emailIdentifier = 'email:' . strtolower($email); - $rateLimited = passwordResetRequestCount($pdo, $ipIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_IP - || passwordResetRequestCount($pdo, $emailIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_EMAIL; + // Check both limits and record the reservation atomically, so concurrent requests + // can't all read the same stale counts before any of them commits. Kept as a short + // transaction that excludes the mail send below, since holding a write transaction + // open across a slow network call is what caused the "database is locked" issue + // fixed earlier in create-critter.php. + $pdo->exec('BEGIN IMMEDIATE'); + try { + $rateLimited = passwordResetRequestCount($pdo, $ipIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_IP + || passwordResetRequestCount($pdo, $emailIdentifier, PASSWORD_RESET_REQUEST_WINDOW_SECONDS) >= MAX_PASSWORD_RESET_REQUESTS_PER_EMAIL; + + if (!$rateLimited) { + recordPasswordResetRequest($pdo, $ipIdentifier); + recordPasswordResetRequest($pdo, $emailIdentifier); + } + $pdo->exec('COMMIT'); + } catch (Throwable $e) { + $pdo->exec('ROLLBACK'); + throw $e; + } if (!$rateLimited) { - recordPasswordResetRequest($pdo, $ipIdentifier); - recordPasswordResetRequest($pdo, $emailIdentifier); - $stmt = $pdo->prepare('SELECT id, username FROM users WHERE email = :email'); $stmt->execute(['email' => $email]); $user = $stmt->fetch(PDO::FETCH_ASSOC); diff --git a/login.php b/login.php index 53554a0..466ab37 100644 --- a/login.php +++ b/login.php @@ -54,22 +54,45 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown'); $emailIdentifier = 'email:' . strtolower($email); + $attemptIds = []; - if ( - loginAttemptCount($pdo, $ipIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_IP - || loginAttemptCount($pdo, $emailIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_EMAIL - ) { - $errors['login'] = 'Too many login attempts. Please wait a while before trying again.'; - } else { + // Check both limits and reserve attempt rows for both identifiers atomically, so + // concurrent requests can't all read the same stale counts before any of them + // commits. Reserving happens before password_verify() runs (rather than only on a + // confirmed failure) so the check-then-record pair is a single fast transaction; + // the reservation is released below if the login actually succeeds, so only failed + // attempts count against the limit — matching the previous behavior. + $pdo->exec('BEGIN IMMEDIATE'); + try { + if ( + loginAttemptCount($pdo, $ipIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_IP + || loginAttemptCount($pdo, $emailIdentifier, LOGIN_ATTEMPT_WINDOW_SECONDS) >= MAX_LOGIN_ATTEMPTS_PER_EMAIL + ) { + $errors['login'] = 'Too many login attempts. Please wait a while before trying again.'; + } else { + recordLoginAttempt($pdo, $ipIdentifier); + $attemptIds[] = (int) $pdo->lastInsertId(); + recordLoginAttempt($pdo, $emailIdentifier); + $attemptIds[] = (int) $pdo->lastInsertId(); + } + $pdo->exec('COMMIT'); + } catch (Throwable $e) { + $pdo->exec('ROLLBACK'); + throw $e; + } + + if (empty($errors)) { $stmt = $pdo->prepare('SELECT * FROM users WHERE email = :email'); $stmt->execute(['email' => $email]); $user = $stmt->fetch(PDO::FETCH_ASSOC); if (!$user || !password_verify($password, $user['password'])) { - recordLoginAttempt($pdo, $ipIdentifier); - recordLoginAttempt($pdo, $emailIdentifier); $errors['login'] = 'Invalid email or password.'; } else { + foreach ($attemptIds as $attemptId) { + $pdo->prepare('DELETE FROM login_attempts WHERE id = :id')->execute(['id' => $attemptId]); + } + start_app_session(); session_regenerate_id(true); $_SESSION['user_id'] = $user['id']; diff --git a/register.php b/register.php index 477c941..df7eb0b 100644 --- a/register.php +++ b/register.php @@ -45,12 +45,31 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { $pdo->exec('CREATE INDEX IF NOT EXISTS idx_registration_attempts_identifier ON registration_attempts (identifier)'); $ipIdentifier = 'ip:' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown'); + $registrationAttemptId = null; if (!csrf_verify($_POST['csrf_token'] ?? null)) { $errors['csrf'] = 'Your session has expired. Please refresh the page and try again.'; - } elseif (registrationAttemptCount($pdo, $ipIdentifier, REGISTRATION_WINDOW_SECONDS) >= MAX_REGISTRATIONS_PER_IP) { - $errors['rate_limit'] = 'Too many accounts have been created from this network. Please try again later.'; } else { + // Check the per-IP limit and reserve a slot atomically, so concurrent requests + // can't all read the same stale count before any of them commits. The reservation + // is released below if this request doesn't end up creating an account, so only + // actual registrations count against the limit — matching the previous behavior. + $pdo->exec('BEGIN IMMEDIATE'); + try { + if (registrationAttemptCount($pdo, $ipIdentifier, REGISTRATION_WINDOW_SECONDS) >= MAX_REGISTRATIONS_PER_IP) { + $errors['rate_limit'] = 'Too many accounts have been created from this network. Please try again later.'; + } else { + recordRegistrationAttempt($pdo, $ipIdentifier); + $registrationAttemptId = (int) $pdo->lastInsertId(); + } + $pdo->exec('COMMIT'); + } catch (Throwable $e) { + $pdo->exec('ROLLBACK'); + throw $e; + } + } + + if (empty($errors)) { if ($username === '') { $errors['username'] = 'Username is required.'; } elseif (strlen($username) < 3 || strlen($username) > 12) { @@ -97,12 +116,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { 'email' => $email, 'password' => $hashedPassword, ]); - // Capture the new user's id before any other insert runs on this connection — - // SQLite's lastInsertId() returns the most recent rowid inserted on the - // connection regardless of table, so recordRegistrationAttempt()'s own insert - // would otherwise clobber it and log the session in as the wrong user. $newUserId = (int) $pdo->lastInsertId(); - recordRegistrationAttempt($pdo, $ipIdentifier); } catch (PDOException $e) { if (str_contains($e->getMessage(), 'UNIQUE constraint failed: users.username')) { $errors['username'] = 'That username is not available.'; @@ -130,6 +144,13 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } } } + + // Reaching here means this request didn't end in a created account (validation + // failed, a username/email collision was caught, or the rate limit itself + // rejected it) — release any reserved slot so only actual registrations count. + if ($registrationAttemptId !== null) { + $pdo->prepare('DELETE FROM registration_attempts WHERE id = :id')->execute(['id' => $registrationAttemptId]); + } } $pageTitle = 'Register';