fix(auth): guard the login + change-password verify paths against oversized passwords
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.90.6"
|
version = "0.90.7"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.90.6"
|
version = "0.90.7"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
default-run = "mangalord"
|
default-run = "mangalord"
|
||||||
|
|
||||||
|
|||||||
@@ -135,6 +135,10 @@ async fn login(
|
|||||||
"username and password are required".into(),
|
"username and password are required".into(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
// Bound the password before argon2 runs — guards BOTH the real-verify and
|
||||||
|
// the dummy-hash timing-equaliser branch below, so a giant password can't
|
||||||
|
// make every login attempt a CPU-DoS.
|
||||||
|
reject_oversized_password(&input.password)?;
|
||||||
|
|
||||||
let user = repo::user::find_by_username(&state.db, username).await?;
|
let user = repo::user::find_by_username(&state.db, username).await?;
|
||||||
let Some(user) = user else {
|
let Some(user) = user else {
|
||||||
@@ -205,6 +209,9 @@ async fn change_password(
|
|||||||
Json(input): Json<ChangePassword>,
|
Json(input): Json<ChangePassword>,
|
||||||
) -> AppResult<impl IntoResponse> {
|
) -> AppResult<impl IntoResponse> {
|
||||||
check_auth_rate_limit(&state, "change_password")?;
|
check_auth_rate_limit(&state, "change_password")?;
|
||||||
|
// Cap current_password before verify_password runs argon2 (same DoS
|
||||||
|
// vector as login). new_password is bounded by validate_password below.
|
||||||
|
reject_oversized_password(&input.current_password)?;
|
||||||
if !verify_password(&input.current_password, &user.password_hash) {
|
if !verify_password(&input.current_password, &user.password_hash) {
|
||||||
return Err(AppError::Unauthenticated);
|
return Err(AppError::Unauthenticated);
|
||||||
}
|
}
|
||||||
@@ -425,11 +432,67 @@ pub(crate) fn validate_username(u: &str) -> AppResult<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Upper bound on password length (bytes). argon2 has no inherent length
|
||||||
|
/// limit, so without a cap an attacker could submit a multi-megabyte password
|
||||||
|
/// and make every hash a CPU-heavy DoS. 1024 bytes is far longer than any real
|
||||||
|
/// passphrase. Measured in bytes (like the min check) since that's what argon2
|
||||||
|
/// actually processes.
|
||||||
|
pub(crate) const MAX_PASSWORD_BYTES: usize = 1024;
|
||||||
|
|
||||||
|
/// Reject an over-cap password *before* any argon2 work runs. The
|
||||||
|
/// verification paths (login, change-password) don't go through
|
||||||
|
/// [`validate_password`] — they hash/verify the raw input — so without this
|
||||||
|
/// an attacker could submit a multi-megabyte password and turn every
|
||||||
|
/// login/verify into a CPU-DoS. Length-only and independent of whether the
|
||||||
|
/// account exists, so it leaks nothing (no username enumeration).
|
||||||
|
pub(crate) fn reject_oversized_password(p: &str) -> AppResult<()> {
|
||||||
|
if p.len() > MAX_PASSWORD_BYTES {
|
||||||
|
return Err(AppError::InvalidInput(format!(
|
||||||
|
"password must be at most {MAX_PASSWORD_BYTES} bytes"
|
||||||
|
)));
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn validate_password(p: &str) -> AppResult<()> {
|
pub(crate) fn validate_password(p: &str) -> AppResult<()> {
|
||||||
if p.len() < 8 {
|
if p.len() < 8 {
|
||||||
return Err(AppError::InvalidInput(
|
return Err(AppError::InvalidInput(
|
||||||
"password must be at least 8 characters".into(),
|
"password must be at least 8 characters".into(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
if p.len() > MAX_PASSWORD_BYTES {
|
||||||
|
return Err(AppError::InvalidInput(
|
||||||
|
format!("password must be at most {MAX_PASSWORD_BYTES} bytes"),
|
||||||
|
));
|
||||||
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn validate_password_rejects_too_short() {
|
||||||
|
assert!(validate_password("short").is_err());
|
||||||
|
assert!(validate_password("1234567").is_err()); // 7 chars
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn validate_password_accepts_in_range() {
|
||||||
|
assert!(validate_password("hunter2hunter2").is_ok());
|
||||||
|
// Exactly at the cap is allowed.
|
||||||
|
assert!(validate_password(&"a".repeat(MAX_PASSWORD_BYTES)).is_ok());
|
||||||
|
// Minimum length boundary.
|
||||||
|
assert!(validate_password("12345678").is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn validate_password_rejects_over_cap() {
|
||||||
|
// One byte past the cap is refused so a giant password can't turn each
|
||||||
|
// login/register into an argon2 CPU-DoS.
|
||||||
|
let too_long = "a".repeat(MAX_PASSWORD_BYTES + 1);
|
||||||
|
let err = validate_password(&too_long).unwrap_err();
|
||||||
|
assert!(matches!(err, AppError::InvalidInput(_)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -170,6 +170,26 @@ async fn login_rejects_wrong_password(pool: PgPool) {
|
|||||||
assert_eq!(body["error"]["code"], "unauthenticated");
|
assert_eq!(body["error"]["code"], "unauthenticated");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
|
async fn login_rejects_oversized_password_before_argon2(pool: PgPool) {
|
||||||
|
// A multi-KB password on the login path must be rejected as malformed
|
||||||
|
// input (400) rather than fed to argon2 — otherwise every attempt is a
|
||||||
|
// CPU-DoS. The account need not even exist; the guard is input-shape only.
|
||||||
|
let h = common::harness(pool);
|
||||||
|
let giant = "a".repeat(5000);
|
||||||
|
let resp = h
|
||||||
|
.app
|
||||||
|
.oneshot(common::post_json(
|
||||||
|
"/api/v1/auth/login",
|
||||||
|
json!({ "username": "alice", "password": giant }),
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
|
||||||
|
let body = common::body_json(resp).await;
|
||||||
|
assert_eq!(body["error"]["code"], "invalid_input");
|
||||||
|
}
|
||||||
|
|
||||||
#[sqlx::test(migrations = "./migrations")]
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
async fn login_rejects_unknown_user(pool: PgPool) {
|
async fn login_rejects_unknown_user(pool: PgPool) {
|
||||||
let h = common::harness(pool);
|
let h = common::harness(pool);
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.90.6",
|
"version": "0.90.7",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
Reference in New Issue
Block a user