From fc3181ee3bab398e8dad62b7a5a7a31a3393b93a Mon Sep 17 00:00:00 2001 From: janis steiner Date: Wed, 20 May 2026 19:35:11 +0200 Subject: [PATCH] fixing minor security issues --- app-code/api/account/update_passkey.php | 9 +- app-code/api/login/check_passkey.php | 10 +- app-code/api/manage/fetch_users.php | 29 ++-- pentest_report.html | 180 ++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 13 deletions(-) create mode 100644 pentest_report.html diff --git a/app-code/api/account/update_passkey.php b/app-code/api/account/update_passkey.php index c477e97..cd8b245 100644 --- a/app-code/api/account/update_passkey.php +++ b/app-code/api/account/update_passkey.php @@ -10,6 +10,11 @@ secure_session_start(); require_same_origin_request(); require_csrf_token(); +function print_json_response($data): void +{ + print(htmlentities(json_encode($data, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT), ENT_NOQUOTES, 'UTF-8')); +} + // Assuming you've already established a database connection here include "../../config/config.php"; $conn = new mysqli($DB_SERVERNAME, $DB_USERNAME, $DB_PASSWORD,$DB_DATABASE); @@ -104,7 +109,7 @@ try { $createArgs = $WebAuthn->getCreateArgs(\hex2bin($userId), $userName, $userDisplayName, 60*4, $requireResidentKey, $userVerification, $crossPlatformAttachment); header('Content-Type: application/json'); - print(json_encode($createArgs)); + print_json_response($createArgs); // save challange to session. you have to deliver it to processGet later. $_SESSION['challenge'] = $WebAuthn->getChallenge(); @@ -138,7 +143,7 @@ try { $getArgs = $WebAuthn->getGetArgs($ids, 60*4, $typeUsb, $typeNfc, $typeBle, $typeHyb, $typeInt, $userVerification); header('Content-Type: application/json'); - print(json_encode($getArgs)); + print_json_response($getArgs); // save challange to session. you have to deliver it to processGet later. $_SESSION['challenge'] = $WebAuthn->getChallenge(); diff --git a/app-code/api/login/check_passkey.php b/app-code/api/login/check_passkey.php index c26a415..4fd2d24 100644 --- a/app-code/api/login/check_passkey.php +++ b/app-code/api/login/check_passkey.php @@ -7,6 +7,12 @@ include "../utils/security.php"; secure_session_start(); require_same_origin_request(); require_csrf_token(); + +function print_json_response($data): void +{ + print(htmlentities(json_encode($data, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT), ENT_NOQUOTES, 'UTF-8')); +} + include "../../config/config.php"; $conn = new mysqli($DB_SERVERNAME, $DB_USERNAME, $DB_PASSWORD,$DB_DATABASE); if ($conn->connect_error) { @@ -90,7 +96,7 @@ try { // Get create arguments $createArgs = $WebAuthn->getCreateArgs(\hex2bin($userId), $userName, $userDisplayName, 60*4, $requireResidentKey, $userVerification); header('Content-Type: application/json'); - print(json_encode($createArgs)); + print_json_response($createArgs); // Save challenge to session or somewhere else if needed } else if ($fn === 'getGetArgs') { @@ -120,7 +126,7 @@ try { $getArgs = $WebAuthn->getGetArgs($ids, 60*4, $typeUsb, $typeNfc, $typeBle, $typeHyb, $typeInt, $userVerification); header('Content-Type: application/json'); - print(json_encode($getArgs)); + print_json_response($getArgs); // save challange to session. you have to deliver it to processGet later. $_SESSION['challenge'] = $WebAuthn->getChallenge(); diff --git a/app-code/api/manage/fetch_users.php b/app-code/api/manage/fetch_users.php index 0a19db5..d5686fe 100644 --- a/app-code/api/manage/fetch_users.php +++ b/app-code/api/manage/fetch_users.php @@ -16,21 +16,32 @@ if ($conn->connect_error) { } $search = trim($_GET['search'] ?? ''); -$sort = $_GET['sort'] ?? 'id'; +$sort = $_GET['sort'] ?? ''; $order = strtoupper($_GET['order'] ?? 'ASC') === 'DESC' ? 'DESC' : 'ASC'; -$allowedSorts = ['id', 'username']; -if (!in_array($sort, $allowedSorts)) { - $sort = 'id'; -} - if ($search !== '') { - $query = "SELECT id, username FROM users WHERE username LIKE ? ORDER BY $sort $order"; + if ($sort === 'username') { + $query = $order === 'DESC' + ? "SELECT id, username FROM users WHERE username LIKE ? ORDER BY username DESC" + : "SELECT id, username FROM users WHERE username LIKE ? ORDER BY username ASC"; + } else { + $query = $order === 'DESC' + ? "SELECT id, username FROM users WHERE username LIKE ? ORDER BY id DESC" + : "SELECT id, username FROM users WHERE username LIKE ? ORDER BY id ASC"; + } $stmt = $conn->prepare($query); $like = '%' . $search . '%'; $stmt->bind_param('s', $like); } else { - $query = "SELECT id, username FROM users ORDER BY $sort $order"; + if ($sort === 'username') { + $query = $order === 'DESC' + ? "SELECT id, username FROM users ORDER BY username DESC" + : "SELECT id, username FROM users ORDER BY username ASC"; + } else { + $query = $order === 'DESC' + ? "SELECT id, username FROM users ORDER BY id DESC" + : "SELECT id, username FROM users ORDER BY id ASC"; + } $stmt = $conn->prepare($query); } @@ -50,4 +61,4 @@ $stmt->close(); $conn->close(); echo json_encode(['success' => true, 'data' => $users]); -?> \ No newline at end of file +?> diff --git a/pentest_report.html b/pentest_report.html new file mode 100644 index 0000000..58af0f1 --- /dev/null +++ b/pentest_report.html @@ -0,0 +1,180 @@ + + + + +Security Source Review - Jakach Login + + + +

Security Source Code Review

+ +
+ Project: Jakach Login
+ Repository Path: /home/janis/Documents/jakach-systems/jakach-login
+ Review Date: 2026-05-15
+ Scope: PHP application code, login/account APIs, frontend sinks, Apache/PHP/Docker configuration
+ Method: Manual source review with targeted static searches. No live black-box testing was performed from this environment. +
+ +

Executive Summary

+

The application has several good security controls already in place, including widespread prepared statements, CSRF checks on most state-changing endpoints, recent secure cookie hardening, security headers, and safer rendering for the external domains list. The remaining issues are concentrated around deployment exposure, account security downgrade paths, recovery/session lifecycle, inconsistent CSRF coverage, and supply-chain/runtime configuration.

+ + + + + + + + +
SeverityCount
Critical1
High2
Medium5
Low3
Info2
+ +

Findings

+ +
+

1. Publicly reachable database installer can initialize or modify production schema

+

Critical

+

Evidence: app-code/install/create_db.php:24-57 includes DB credentials and connects without authentication; the same file creates the database and tables. srv_dockerfile:30-35 creates the install directory in the web root and docker-compose.yml:40 mounts all app-code into /var/www/html.

+

Impact: If reachable in production, an unauthenticated user can trigger installer logic and observe database errors. Installer endpoints are often a path to destructive changes, schema drift, and information disclosure.

+

Recommendation: Remove /install from deployed images after setup, block it in Apache, or require a one-time out-of-band installation secret. Move migrations to a CLI-only deployment step.

+
+ +
+

2. Hard-coded database root password in Docker Compose

+

High

+

Evidence: docker-compose.yml:6-7 sets MYSQL_ROOT_PASSWORD: 1234.

+

Impact: Anyone with repository access, leaked compose files, or access to the Docker network can authenticate as the database root user if the deployment uses this value.

+

Recommendation: Replace hard-coded credentials with Docker secrets or environment variables outside version control. Use a least-privileged application database user rather than root.

+
+ +
+

3. Authenticator downgrade and passkey enrollment do not require fresh re-authentication

+

High

+

Evidence: 2FA can be disabled for any logged-in session at app-code/api/account/update_2fa.php:74-81. Passkey registration only checks logged-in state at app-code/api/account/update_passkey.php:21-28 and then updates the credential at app-code/api/account/update_passkey.php:168-175. Password change is stronger because it verifies the old password at app-code/api/account/update_pw.php:51-70.

+

Impact: A stolen active session can remove 2FA or add an attacker-controlled passkey without knowing the password or current 2FA code, increasing account takeover persistence.

+

Recommendation: Require recent password plus current 2FA/passkey verification before disabling 2FA, adding/replacing passkeys, changing recovery channels, or changing the password.

+
+ +
+

4. Missing CSRF protection on external-domain confirmation

+

Medium

+

Evidence: app-code/api/login/confirm_external_redirect.php:1-29 accepts POST and writes to confirmed_domains without require_same_origin_request() or require_csrf_token(). The frontend call at app-code/login/index.php:99-104 sends no CSRF header.

+

Impact: This endpoint is inconsistent with the rest of the state-changing API. The recent change to derive the domain server-side reduces impact, but the missing CSRF check still leaves unnecessary risk if browser or SameSite assumptions change.

+

Recommendation: Add require_same_origin_request() and require_csrf_token() to the endpoint, and include X-CSRF-Token in the frontend fetch.

+
+ +
+

5. Password reset does not revoke existing sessions or remember-me tokens

+

Medium

+

Evidence: Password reset updates users.password and pepper at app-code/api/login/reset_pw.php:43-53, then deletes only the reset token at app-code/api/login/reset_pw.php:65-70. Remember-me tokens are stored separately in keepmeloggedin and are not cleared.

+

Impact: If a password reset is used after suspected compromise, existing browser sessions or persistent tokens may continue to work.

+

Recommendation: On successful password reset and password change, revoke existing remember-me tokens, rotate session identifiers, and consider invalidating all server-side sessions for that user if session storage supports it.

+
+ +
+

6. Account enumeration through registration responses

+

Medium

+

Evidence: app-code/api/register/register_user.php:70-101 returns distinct messages for taken usernames and registered email addresses.

+

Impact: Attackers can enumerate valid usernames and emails for phishing, password spraying, and credential stuffing.

+

Recommendation: Use generic responses and consistent timing for duplicate username/email cases. Send out-of-band confirmation for email ownership instead of revealing registration state.

+
+ +
+

7. Password reset and login notification tokens are sent through third-party URLs

+

Medium

+

Evidence: Reset links containing the token are built at app-code/api/login/send_reset_link.php:42-44 and sent via Telegram using a GET URL at app-code/api/login/send_reset_link.php:72-80. Login alerts also send Telegram messages using GET at app-code/api/login/redirect.php:126-135.

+

Impact: Sensitive reset URLs and account activity details can end up in proxy logs, service logs, shell histories, or monitoring systems that record request URLs.

+

Recommendation: Use Telegram's POST API with request body fields, avoid placing reset tokens in third-party URL query strings, and keep reset links out of logs where possible.

+
+ +
+

8. Remaining reflected/client-side XSS sink in reset password UI

+

Medium

+

Evidence: app-code/login/reset_pw.php:74-79 inserts result.message into innerHTML. Current server messages are mostly static, but this pattern becomes exploitable if a future backend path includes attacker-controlled text.

+

Impact: XSS on password reset pages is high value because the page handles reset tokens and new credentials.

+

Recommendation: Build alert elements with DOM APIs and assign message text with textContent. Avoid using innerHTML for any API-provided data.

+
+ +
+

9. External geolocation lookup uses plaintext HTTP

+

Low

+

Evidence: app-code/api/utils/get_location.php:3-13 calls http://ip-api.com/json/$ip.

+

Impact: Client IPs are sent to a third party over plaintext, and a network attacker can alter location data shown in security notifications.

+

Recommendation: Use HTTPS, set cURL timeouts, validate responses, and consider whether external geolocation is necessary for login/reset notifications.

+
+ +
+

10. CDN scripts lack Subresource Integrity and strict CSP allow-listing

+

Low

+

Evidence: app-code/assets/components.php:4-7 loads Bootstrap, jQuery, Popper, and Bootstrap JS from CDNs without integrity attributes. The current CSP is intentionally focused and does not restrict script-src.

+

Impact: A CDN compromise or unexpected upstream change can execute JavaScript in the login origin.

+

Recommendation: Self-host dependencies or pin exact versions with SRI. Then add a strict script-src and style-src CSP, ideally using nonces/hashes for inline scripts.

+
+ +
+

11. Runtime dependencies are not pinned in the Docker build

+

Low

+

Evidence: srv_dockerfile:1 uses floating php:apache. Composer dependencies are installed with unconstrained composer require at build time in srv_dockerfile:27-29.

+

Impact: Builds are not reproducible. A future upstream change can introduce regressions or vulnerable dependency versions without a source-code diff.

+

Recommendation: Pin image digests or stable patch tags, commit a composer.json and composer.lock, and scan images/dependencies in CI.

+
+ +
+

12. Database and infrastructure errors are exposed to clients

+

Info

+

Evidence: The installer prints connection and SQL errors at app-code/install/create_db.php:30-45. Several API endpoints also include connection details in JSON error messages.

+

Impact: Error details can reveal hostnames, schema details, credentials configuration mistakes, and operational state.

+

Recommendation: Return generic client errors and log detailed exceptions server-side.

+
+ +
+

13. Recently fixed: stored XSS in approved external domains list

+

Info

+

Evidence: The domains list now renders stored domain values using textContent in app-code/account/index.php:822-840. The API filters historical invalid domains in app-code/api/account/manage_domains.php:21-28, and domain confirmation now derives from server-side redirect state in app-code/api/login/confirm_external_redirect.php:11-23.

+

Residual risk: Keep using DOM text APIs for account-page lists. Historical malicious rows should also be cleaned from the database, not only filtered from API output.

+
+ +

Recommended Remediation Order

+
    +
  1. Remove or protect /install in production and rotate database credentials.
    2. +
  2. Move database credentials to secrets and stop using root for the app user.
    3. +
  3. Add fresh re-authentication for 2FA/passkey/security-setting changes.
    4. +
  4. Add CSRF validation to external-domain confirmation.
    5. +
  5. Revoke persistent sessions on password reset/change.
    6. +
  6. Replace remaining API-data innerHTML sinks with DOM text nodes.
    7. +
  7. Pin dependencies, self-host or SRI-protect frontend libraries, and tighten CSP.
    8. +
+ +

Review Limitations

+

This review was performed from source code and local configuration only. PHP, Apache tooling, and Docker socket access were not available for runtime validation in this environment, so findings should be confirmed in a staging deployment after fixes are applied.

+ +
+ Generated locally from source review on 2026-05-15. +
+ +