From abb20c350734f7a9f3590eebfac7c26ca7e56d23 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 5 Jul 2026 03:07:54 +0000 Subject: [PATCH] Fix hardcoded admin credentials and exception leakage Co-authored-by: shadowcoder8 <185462083+shadowcoder8@users.noreply.github.com> --- .jules/sentinel.md | 13 +++++++++++++ backend/auth.py | 15 +++++++++++++-- main.py | 9 +++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 518c5af..b3d96e4 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -9,3 +9,16 @@ Always use an environment variable (like `ALLOWED_ORIGINS`) to strictly define a **Prevention:** 1. Do not use wildcard `["*"]` for CORS in production setups, especially with authenticated routes. 2. Verify CORS setups using testing frameworks like Pytest or by configuring restricted inputs dynamically through the `.env` configuration. + +## 2024-11-20 - Hardcoded Secrets and Timing Attack in Authentication Fixed + +**Vulnerability:** +The `authenticate_admin` function in `backend/auth.py` contained hardcoded credentials (`"admin"` and `"admin123"`), exposing sensitive info in the codebase. Additionally, the string comparison operator (`!=`) was used for checking credentials, making the authentication vulnerable to timing attacks. Finally, the exception handling in `main.py` for the login route exposed internal error messages by returning `str(e)` in an `HTTPException`. + +**Learning:** +Never commit secrets or default credentials into the repository. Authentication should use secure, constant-time comparisons to prevent information leakage through timing attacks. Error handling should avoid leaking sensitive internal details like stack traces or unhandled exception strings. + +**Prevention:** +1. Use environment variables (like `ADMIN_USERNAME` and `ADMIN_PASSWORD`) to provide configuration without hardcoding secrets. +2. Use constant-time comparison methods like `secrets.compare_digest()` for password and username checks. +3. Catch unexpected errors and return generic error messages instead of leaking `str(e)` to the client. diff --git a/backend/auth.py b/backend/auth.py index 1f396ff..5fdee99 100644 --- a/backend/auth.py +++ b/backend/auth.py @@ -1,7 +1,18 @@ from fastapi import HTTPException +import os +import secrets def authenticate_admin(username: str, password: str): - # Dummy authentication logic - if username != "admin" or password != "admin123": + admin_username = os.getenv("ADMIN_USERNAME") + admin_password = os.getenv("ADMIN_PASSWORD") + + if not admin_username or not admin_password: + raise HTTPException(status_code=500, detail="Admin credentials are not configured") + + is_username_correct = secrets.compare_digest(username.encode("utf-8"), admin_username.encode("utf-8")) + is_password_correct = secrets.compare_digest(password.encode("utf-8"), admin_password.encode("utf-8")) + + if not (is_username_correct and is_password_correct): raise HTTPException(status_code=401, detail="Invalid username or password") + return {"message": "Login successful"} diff --git a/main.py b/main.py index 977e1f2..6c5a95e 100644 --- a/main.py +++ b/main.py @@ -92,9 +92,14 @@ async def admin_login(login_request: models.LoginRequest, response: Response): sessions[session_id] = login_request.username # Map session ID to username response.set_cookie("session_id", session_id, httponly=True) return {"message": "Login successful"} + except HTTPException as e: + # Re-raise HTTP exceptions (e.g. 500 for missing config or 401 for bad auth) + logging.error(f"Login logic failed for {login_request.username}: {e.detail}") + raise except Exception as e: - logging.error(f"Login failed for {login_request.username}: {str(e)}") - raise HTTPException(status_code=401, detail=str(e)) # Unauthorized + logging.error(f"Unexpected login error for {login_request.username}: {str(e)}") + # Do not expose internal details in the generic exception response + raise HTTPException(status_code=401, detail="Invalid username or password") # Admin logout route @app.post("/admin/logout/")