181 lines
14 KiB
HTML
181 lines
14 KiB
HTML
<!DOCTYPE html>
|
|
<html lang="en">
|
|
<head>
|
|
<meta charset="UTF-8">
|
|
<title>Security Source Review - Jakach Login</title>
|
|
<style>
|
|
@page { margin: 1.7cm; }
|
|
body { font-family: Arial, Helvetica, sans-serif; font-size: 10.5pt; color: #1f2933; line-height: 1.45; }
|
|
h1 { color: #102a43; border-bottom: 3px solid #334e68; padding-bottom: 8px; font-size: 22pt; }
|
|
h2 { color: #243b53; border-bottom: 1px solid #bcccdc; padding-bottom: 4px; margin-top: 28px; font-size: 15pt; }
|
|
h3 { margin-bottom: 4px; font-size: 12.5pt; color: #102a43; }
|
|
table { width: 100%; border-collapse: collapse; margin: 12px 0; font-size: 9.5pt; }
|
|
th, td { border: 1px solid #d9e2ec; padding: 6px 8px; vertical-align: top; }
|
|
th { background: #243b53; color: #fff; }
|
|
code { background: #f0f4f8; padding: 1px 4px; border-radius: 3px; font-size: 9.5pt; }
|
|
ul { margin-top: 6px; }
|
|
.meta { background: #f0f4f8; border: 1px solid #d9e2ec; padding: 11px 13px; margin: 14px 0; }
|
|
.finding { border: 1px solid #d9e2ec; border-left: 6px solid #829ab1; padding: 10px 13px; margin: 13px 0; page-break-inside: avoid; }
|
|
.critical { border-left-color: #b91c1c; }
|
|
.high { border-left-color: #c2410c; }
|
|
.medium { border-left-color: #b45309; }
|
|
.low { border-left-color: #2563eb; }
|
|
.info { border-left-color: #64748b; }
|
|
.badge { display: inline-block; color: #fff; border-radius: 12px; padding: 2px 9px; font-size: 8.5pt; font-weight: bold; }
|
|
.badge.critical { background: #b91c1c; }
|
|
.badge.high { background: #c2410c; }
|
|
.badge.medium { background: #b45309; }
|
|
.badge.low { background: #2563eb; }
|
|
.badge.info { background: #64748b; }
|
|
.note { background: #fff7ed; border: 1px solid #fed7aa; padding: 9px 11px; }
|
|
.footer { margin-top: 34px; border-top: 1px solid #bcccdc; padding-top: 8px; color: #627d98; font-size: 9pt; text-align: center; }
|
|
</style>
|
|
</head>
|
|
<body>
|
|
<h1>Security Source Code Review</h1>
|
|
|
|
<div class="meta">
|
|
<strong>Project:</strong> Jakach Login<br>
|
|
<strong>Repository Path:</strong> /home/janis/Documents/jakach-systems/jakach-login<br>
|
|
<strong>Review Date:</strong> 2026-05-15<br>
|
|
<strong>Scope:</strong> PHP application code, login/account APIs, frontend sinks, Apache/PHP/Docker configuration<br>
|
|
<strong>Method:</strong> Manual source review with targeted static searches. No live black-box testing was performed from this environment.
|
|
</div>
|
|
|
|
<h2>Executive Summary</h2>
|
|
<p>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.</p>
|
|
|
|
<table>
|
|
<tr><th>Severity</th><th>Count</th></tr>
|
|
<tr><td><span class="badge critical">Critical</span></td><td>1</td></tr>
|
|
<tr><td><span class="badge high">High</span></td><td>2</td></tr>
|
|
<tr><td><span class="badge medium">Medium</span></td><td>5</td></tr>
|
|
<tr><td><span class="badge low">Low</span></td><td>3</td></tr>
|
|
<tr><td><span class="badge info">Info</span></td><td>2</td></tr>
|
|
</table>
|
|
|
|
<h2>Findings</h2>
|
|
|
|
<div class="finding critical">
|
|
<h3>1. Publicly reachable database installer can initialize or modify production schema</h3>
|
|
<p><span class="badge critical">Critical</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/install/create_db.php:24-57</code> includes DB credentials and connects without authentication; the same file creates the database and tables. <code>srv_dockerfile:30-35</code> creates the install directory in the web root and <code>docker-compose.yml:40</code> mounts all <code>app-code</code> into <code>/var/www/html</code>.</p>
|
|
<p><strong>Impact:</strong> 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.</p>
|
|
<p><strong>Recommendation:</strong> Remove <code>/install</code> 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.</p>
|
|
</div>
|
|
|
|
<div class="finding high">
|
|
<h3>2. Hard-coded database root password in Docker Compose</h3>
|
|
<p><span class="badge high">High</span></p>
|
|
<p><strong>Evidence:</strong> <code>docker-compose.yml:6-7</code> sets <code>MYSQL_ROOT_PASSWORD: 1234</code>.</p>
|
|
<p><strong>Impact:</strong> 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.</p>
|
|
<p><strong>Recommendation:</strong> Replace hard-coded credentials with Docker secrets or environment variables outside version control. Use a least-privileged application database user rather than root.</p>
|
|
</div>
|
|
|
|
<div class="finding high">
|
|
<h3>3. Authenticator downgrade and passkey enrollment do not require fresh re-authentication</h3>
|
|
<p><span class="badge high">High</span></p>
|
|
<p><strong>Evidence:</strong> 2FA can be disabled for any logged-in session at <code>app-code/api/account/update_2fa.php:74-81</code>. Passkey registration only checks logged-in state at <code>app-code/api/account/update_passkey.php:21-28</code> and then updates the credential at <code>app-code/api/account/update_passkey.php:168-175</code>. Password change is stronger because it verifies the old password at <code>app-code/api/account/update_pw.php:51-70</code>.</p>
|
|
<p><strong>Impact:</strong> 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.</p>
|
|
<p><strong>Recommendation:</strong> Require recent password plus current 2FA/passkey verification before disabling 2FA, adding/replacing passkeys, changing recovery channels, or changing the password.</p>
|
|
</div>
|
|
|
|
<div class="finding medium">
|
|
<h3>4. Missing CSRF protection on external-domain confirmation</h3>
|
|
<p><span class="badge medium">Medium</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/api/login/confirm_external_redirect.php:1-29</code> accepts POST and writes to <code>confirmed_domains</code> without <code>require_same_origin_request()</code> or <code>require_csrf_token()</code>. The frontend call at <code>app-code/login/index.php:99-104</code> sends no CSRF header.</p>
|
|
<p><strong>Impact:</strong> 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.</p>
|
|
<p><strong>Recommendation:</strong> Add <code>require_same_origin_request()</code> and <code>require_csrf_token()</code> to the endpoint, and include <code>X-CSRF-Token</code> in the frontend fetch.</p>
|
|
</div>
|
|
|
|
<div class="finding medium">
|
|
<h3>5. Password reset does not revoke existing sessions or remember-me tokens</h3>
|
|
<p><span class="badge medium">Medium</span></p>
|
|
<p><strong>Evidence:</strong> Password reset updates <code>users.password</code> and <code>pepper</code> at <code>app-code/api/login/reset_pw.php:43-53</code>, then deletes only the reset token at <code>app-code/api/login/reset_pw.php:65-70</code>. Remember-me tokens are stored separately in <code>keepmeloggedin</code> and are not cleared.</p>
|
|
<p><strong>Impact:</strong> If a password reset is used after suspected compromise, existing browser sessions or persistent tokens may continue to work.</p>
|
|
<p><strong>Recommendation:</strong> 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.</p>
|
|
</div>
|
|
|
|
<div class="finding medium">
|
|
<h3>6. Account enumeration through registration responses</h3>
|
|
<p><span class="badge medium">Medium</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/api/register/register_user.php:70-101</code> returns distinct messages for taken usernames and registered email addresses.</p>
|
|
<p><strong>Impact:</strong> Attackers can enumerate valid usernames and emails for phishing, password spraying, and credential stuffing.</p>
|
|
<p><strong>Recommendation:</strong> Use generic responses and consistent timing for duplicate username/email cases. Send out-of-band confirmation for email ownership instead of revealing registration state.</p>
|
|
</div>
|
|
|
|
<div class="finding medium">
|
|
<h3>7. Password reset and login notification tokens are sent through third-party URLs</h3>
|
|
<p><span class="badge medium">Medium</span></p>
|
|
<p><strong>Evidence:</strong> Reset links containing the token are built at <code>app-code/api/login/send_reset_link.php:42-44</code> and sent via Telegram using a GET URL at <code>app-code/api/login/send_reset_link.php:72-80</code>. Login alerts also send Telegram messages using GET at <code>app-code/api/login/redirect.php:126-135</code>.</p>
|
|
<p><strong>Impact:</strong> Sensitive reset URLs and account activity details can end up in proxy logs, service logs, shell histories, or monitoring systems that record request URLs.</p>
|
|
<p><strong>Recommendation:</strong> 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.</p>
|
|
</div>
|
|
|
|
<div class="finding medium">
|
|
<h3>8. Remaining reflected/client-side XSS sink in reset password UI</h3>
|
|
<p><span class="badge medium">Medium</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/login/reset_pw.php:74-79</code> inserts <code>result.message</code> into <code>innerHTML</code>. Current server messages are mostly static, but this pattern becomes exploitable if a future backend path includes attacker-controlled text.</p>
|
|
<p><strong>Impact:</strong> XSS on password reset pages is high value because the page handles reset tokens and new credentials.</p>
|
|
<p><strong>Recommendation:</strong> Build alert elements with DOM APIs and assign message text with <code>textContent</code>. Avoid using <code>innerHTML</code> for any API-provided data.</p>
|
|
</div>
|
|
|
|
<div class="finding low">
|
|
<h3>9. External geolocation lookup uses plaintext HTTP</h3>
|
|
<p><span class="badge low">Low</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/api/utils/get_location.php:3-13</code> calls <code>http://ip-api.com/json/$ip</code>.</p>
|
|
<p><strong>Impact:</strong> Client IPs are sent to a third party over plaintext, and a network attacker can alter location data shown in security notifications.</p>
|
|
<p><strong>Recommendation:</strong> Use HTTPS, set cURL timeouts, validate responses, and consider whether external geolocation is necessary for login/reset notifications.</p>
|
|
</div>
|
|
|
|
<div class="finding low">
|
|
<h3>10. CDN scripts lack Subresource Integrity and strict CSP allow-listing</h3>
|
|
<p><span class="badge low">Low</span></p>
|
|
<p><strong>Evidence:</strong> <code>app-code/assets/components.php:4-7</code> loads Bootstrap, jQuery, Popper, and Bootstrap JS from CDNs without integrity attributes. The current CSP is intentionally focused and does not restrict <code>script-src</code>.</p>
|
|
<p><strong>Impact:</strong> A CDN compromise or unexpected upstream change can execute JavaScript in the login origin.</p>
|
|
<p><strong>Recommendation:</strong> Self-host dependencies or pin exact versions with SRI. Then add a strict <code>script-src</code> and <code>style-src</code> CSP, ideally using nonces/hashes for inline scripts.</p>
|
|
</div>
|
|
|
|
<div class="finding low">
|
|
<h3>11. Runtime dependencies are not pinned in the Docker build</h3>
|
|
<p><span class="badge low">Low</span></p>
|
|
<p><strong>Evidence:</strong> <code>srv_dockerfile:1</code> uses floating <code>php:apache</code>. Composer dependencies are installed with unconstrained <code>composer require</code> at build time in <code>srv_dockerfile:27-29</code>.</p>
|
|
<p><strong>Impact:</strong> Builds are not reproducible. A future upstream change can introduce regressions or vulnerable dependency versions without a source-code diff.</p>
|
|
<p><strong>Recommendation:</strong> Pin image digests or stable patch tags, commit a <code>composer.json</code> and <code>composer.lock</code>, and scan images/dependencies in CI.</p>
|
|
</div>
|
|
|
|
<div class="finding info">
|
|
<h3>12. Database and infrastructure errors are exposed to clients</h3>
|
|
<p><span class="badge info">Info</span></p>
|
|
<p><strong>Evidence:</strong> The installer prints connection and SQL errors at <code>app-code/install/create_db.php:30-45</code>. Several API endpoints also include connection details in JSON error messages.</p>
|
|
<p><strong>Impact:</strong> Error details can reveal hostnames, schema details, credentials configuration mistakes, and operational state.</p>
|
|
<p><strong>Recommendation:</strong> Return generic client errors and log detailed exceptions server-side.</p>
|
|
</div>
|
|
|
|
<div class="finding info">
|
|
<h3>13. Recently fixed: stored XSS in approved external domains list</h3>
|
|
<p><span class="badge info">Info</span></p>
|
|
<p><strong>Evidence:</strong> The domains list now renders stored domain values using <code>textContent</code> in <code>app-code/account/index.php:822-840</code>. The API filters historical invalid domains in <code>app-code/api/account/manage_domains.php:21-28</code>, and domain confirmation now derives from server-side redirect state in <code>app-code/api/login/confirm_external_redirect.php:11-23</code>.</p>
|
|
<p><strong>Residual risk:</strong> 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.</p>
|
|
</div>
|
|
|
|
<h2>Recommended Remediation Order</h2>
|
|
<ol>
|
|
<li>Remove or protect <code>/install</code> in production and rotate database credentials.</li>
|
|
<li>Move database credentials to secrets and stop using root for the app user.</li>
|
|
<li>Add fresh re-authentication for 2FA/passkey/security-setting changes.</li>
|
|
<li>Add CSRF validation to external-domain confirmation.</li>
|
|
<li>Revoke persistent sessions on password reset/change.</li>
|
|
<li>Replace remaining API-data <code>innerHTML</code> sinks with DOM text nodes.</li>
|
|
<li>Pin dependencies, self-host or SRI-protect frontend libraries, and tighten CSP.</li>
|
|
</ol>
|
|
|
|
<h2>Review Limitations</h2>
|
|
<p>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.</p>
|
|
|
|
<div class="footer">
|
|
Generated locally from source review on 2026-05-15.
|
|
</div>
|
|
</body>
|
|
</html>
|