chore: migrate legacy PHP customer portal into monorepo#3
chore: migrate legacy PHP customer portal into monorepo#3digitaldrreamer wants to merge 1 commit into
Conversation
Migrates the existing PHP web module into the monorepo under examples/. Module handles authentication, file uploads, and admin utilities for the legacy customer portal.
|
Important Review skippedToo many files! This PR contains 201 files, which is 51 over the limit of 150. To get a review, narrow the scope: Upgrade to Pro or Pro Plus to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (18)
📒 Files selected for processing (201)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several new features and security improvements to the DVWA application, including a new Broken Access Control module, an API module with OpenAPI documentation, and various security hardening measures across existing modules. The review feedback identified critical security vulnerabilities related to Local File Inclusion (LFI) and Directory Traversal in the help and source viewer utilities, as well as high-severity issues in session management, API authentication, and database query construction. I have filtered out comments that were purely explanatory or lacked actionable feedback.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (array_key_exists ("id", $_GET) && | ||
| array_key_exists ("security", $_GET) && | ||
| array_key_exists ("locale", $_GET)) { | ||
| $id = $_GET[ 'id' ]; | ||
| $security = $_GET[ 'security' ]; | ||
| $locale = $_GET[ 'locale' ]; | ||
|
|
||
| ob_start(); | ||
| if ($locale == 'en') { | ||
| eval( '?>' . file_get_contents( DVWA_WEB_PAGE_TO_ROOT . "vulnerabilities/{$id}/help/help.php" ) . '<?php ' ); | ||
| } else { | ||
| eval( '?>' . file_get_contents( DVWA_WEB_PAGE_TO_ROOT . "vulnerabilities/{$id}/help/help.{$locale}.php" ) . '<?php ' ); | ||
| } | ||
| $help = ob_get_contents(); | ||
| ob_end_clean(); | ||
| } else { | ||
| $help = "<p>Not Found</p>"; | ||
| } |
There was a problem hiding this comment.
The help viewer utility accepts raw user input for $id and $locale from $_GET and passes them directly to file_get_contents() and eval(). This introduces a critical Local File Inclusion (LFI) and Remote Code Execution (RCE) vulnerability in the portal's core administration utility. Restrict $id and $locale to a strict whitelist of allowed values.
if (array_key_exists ("id", $_GET) &&
array_key_exists ("security", $_GET) &&
array_key_exists ("locale", $_GET)) {
$id = $_GET[ 'id' ];
$security = $_GET[ 'security' ];
$locale = $_GET[ 'locale' ];
$allowed_ids = array('fi', 'brute', 'csrf', 'exec', 'sqli', 'sqli_blind', 'upload', 'xss_r', 'xss_s', 'weak_id', 'javascript', 'authbypass', 'open_redirect', 'bac', 'api');
$allowed_locales = array('en', 'zh');
if (!in_array($id, $allowed_ids) || !in_array($locale, $allowed_locales)) {
$help = "<p>Not Found</p>";
} else {
ob_start();
if ($locale == 'en') {
eval( '?>' . file_get_contents( DVWA_WEB_PAGE_TO_ROOT . "vulnerabilities/${id}/help/help.php" ) . '<?php ' );
} else {
eval( '?>' . file_get_contents( DVWA_WEB_PAGE_TO_ROOT . "vulnerabilities/${id}/help/help.${locale}.php" ) . '<?php ' );
}
$help = ob_get_contents();
ob_end_clean();
}
} else {
$help = "<p>Not Found</p>";
}| if (array_key_exists ("id", $_GET) && array_key_exists ("security", $_GET)) { | ||
| $id = $_GET[ 'id' ]; | ||
| $security = $_GET[ 'security' ]; | ||
|
|
||
|
|
||
| switch ($id) { | ||
| case "fi" : | ||
| $vuln = 'File Inclusion'; | ||
| break; | ||
| case "brute" : | ||
| $vuln = 'Brute Force'; | ||
| break; | ||
| case "csrf" : | ||
| $vuln = 'CSRF'; | ||
| break; | ||
| case "exec" : | ||
| $vuln = 'Command Injection'; | ||
| break; | ||
| case "sqli" : | ||
| $vuln = 'SQL Injection'; |
There was a problem hiding this comment.
The source viewer utility accepts raw user input for $id and $security from $_GET and passes them directly to file_get_contents(). This introduces a critical Arbitrary File Disclosure / Directory Traversal vulnerability in the portal's core administration utility. Restrict $id and $security to a strict whitelist of allowed values.
if (array_key_exists ("id", $_GET) && array_key_exists ("security", $_GET)) {
$id = $_GET[ 'id' ];
$security = $_GET[ 'security' ];
$allowed_ids = array('fi', 'brute', 'csrf', 'exec', 'sqli', 'sqli_blind', 'upload', 'xss_r', 'xss_s', 'weak_id', 'javascript', 'authbypass', 'open_redirect', 'bac', 'api');
$allowed_security = array('low', 'medium', 'high', 'impossible');
if (!in_array($id, $allowed_ids) || !in_array($security, $allowed_security)) {
$page['body'] = "<p>Not found</p>";
dvwaSourceHtmlEcho( $page );
exit;
}| if (array_key_exists("id", $_GET)) { | ||
| $id = $_GET['id']; | ||
|
|
There was a problem hiding this comment.
The source viewer utility accepts raw user input for $id from $_GET and passes it directly to file_get_contents(). This introduces a critical Arbitrary File Disclosure / Directory Traversal vulnerability in the portal's core administration utility. Restrict $id to a strict whitelist of allowed values.
if (array_key_exists("id", $_GET)) {
$id = $_GET['id'];
$allowed_ids = array('fi', 'brute', 'csrf', 'exec', 'sqli', 'sqli_blind', 'upload', 'xss_r', 'xss_s', 'weak_id', 'javascript', 'authbypass', 'open_redirect', 'bac', 'api');
if (!in_array($id, $allowed_ids)) {
$page['body'] = "<p>Not found</p>";
dvwaSourceHtmlEcho( $page );
exit;
}|
|
||
| $maxlifetime = 86400; | ||
| $secure = false; | ||
| $domain = parse_url($_SERVER['HTTP_HOST'], PHP_URL_HOST); |
There was a problem hiding this comment.
parse_url() expects a full URL with a scheme (e.g., http://) to correctly parse the host. When $_SERVER['HTTP_HOST'] is passed directly (which is typically just domain or domain:port), parse_url() returns null. This prevents the session cookie domain from being set correctly. Use preg_replace to strip the port instead.
$domain = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST']);| path: '/vulnerabilities/api/v2/order/{id}', | ||
| operationId: 'getOrderByID', | ||
| description: 'Get a order by ID.', | ||
| security: [ "basicAuth" ], |
There was a problem hiding this comment.
The security requirement in OpenAPI 3.0 must be an array of objects mapping the security scheme name to its scopes. Passing a raw string "basicAuth" results in an invalid OpenAPI spec (scalar: basicAuth). Additionally, the security scheme defined on line 66 is named "http", not "basicAuth". Update the annotation to correctly reference "http" as an associative array.
security: [ ["http" => []] ],| public static function check_refresh_token($token) { | ||
| $tokenObj = new Token(); | ||
| $decrypted = $tokenObj->decrypt_token ($token); | ||
|
|
||
| if ($decrypted['secret'] == self::REFRESH_TOKEN_SECRET && $decrypted['expires'] > time()) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
In check_refresh_token(), there is no check to verify if $decrypted is false (which happens if decryption fails). Accessing $decrypted['secret'] on a boolean false value will trigger a PHP Warning. Add a check similar to the one in check_access_token().
public static function check_refresh_token($token) {
$tokenObj = new Token();
$decrypted = $tokenObj->decrypt_token ($token);
if ($decrypted === false) {
return false;
}
if ($decrypted['secret'] == self::REFRESH_TOKEN_SECRET && $decrypted['expires'] > time()) {
return true;
}
return false;
}| private function login() { | ||
| # Default fail, just in case. | ||
| $response['status_code_header'] = 'HTTP/1.1 401 Unauthorized'; | ||
| $response['body'] = json_encode (array ("status" => "Authentication failed")); | ||
|
|
||
| if (array_key_exists ("PHP_AUTH_USER", $_SERVER) && | ||
| array_key_exists ("PHP_AUTH_PW", $_SERVER)) { | ||
| $client_id = $_SERVER['PHP_AUTH_USER']; | ||
| $client_secret = $_SERVER['PHP_AUTH_PW']; | ||
|
|
||
| # App auth check | ||
| if ($client_id == "1471.dvwa.digi.ninja" && $client_secret == "ABigLongSecret") { | ||
|
|
There was a problem hiding this comment.
The login() method only checks $_POST for credentials and grant types. However, the OpenAPI specification (openapi.yml) documents this endpoint as accepting application/json. When clients send JSON payloads, $_POST remains empty, causing the authentication to fail with a "Missing grant type" error. Parse the JSON input from php://input if the content type is application/json.
private function login() {
# Default fail, just in case.
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Authentication failed"));
$params = $_POST;
if (array_key_exists("CONTENT_TYPE", $_SERVER) && stripos($_SERVER['CONTENT_TYPE'], "application/json") !== false) {
$json = json_decode(file_get_contents('php://input'), true);
if (is_array($json)) {
$params = array_merge($params, $json);
}
}
if (array_key_exists ("PHP_AUTH_USER", $_SERVER) &&
array_key_exists ("PHP_AUTH_PW", $_SERVER)) {
$client_id = $_SERVER['PHP_AUTH_USER'];
$client_secret = $_SERVER['PHP_AUTH_PW'];
# App auth check
if ($client_id == "1471.dvwa.digi.ninja" && $client_secret == "ABigLongSecret") {
if (array_key_exists ("grant_type", $params)) {
switch ($params['grant_type']) {
case "password":
if (array_key_exists ("username", $params) &&
array_key_exists ("password", $params)) {
$username = $params['username'];
$password = $params['password'];
if ($username == "mrbennett" && $password == "becareful") {
$response['status_code_header'] = 'HTTP/1.1 200 OK';
$response['body'] = Login::create_token();
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Invalid user credentials"));
}
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Missing user credentials"));
}
break;
case "refresh_token":
if (array_key_exists ("refresh_token", $params)) {
$refresh_token = $params['refresh_token'];
# Because this is sent in a POST body, any + characters
# get replaced by a space when the URL decode happens. This
# puts them back to plus characters.
$ref = str_replace (" ", "+", $refresh_token);
if (Login::check_refresh_token($ref)) {
$response['status_code_header'] = 'HTTP/1.1 200 OK';
$response['body'] = Login::create_token();
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Invalid refresh token"));
}
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Missing refresh token"));
}
break;
default:
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Unknown grant type"));
}
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Missing grant type"));
}
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Invalid clientid/clientsecret credentials"));
}
} else {
$response['status_code_header'] = 'HTTP/1.1 401 Unauthorized';
$response['body'] = json_encode (array ("status" => "Missing clientid/clientsecret credentials"));
}| $query = "SELECT COUNT(*) as attempt_count | ||
| FROM bac_log | ||
| WHERE user_id = ? | ||
| AND timestamp > DATE_SUB(NOW(), INTERVAL ? SECOND)"; | ||
|
|
||
| $stmt = mysqli_prepare($GLOBALS["___mysqli_ston"], $query); | ||
| if ($stmt) { | ||
| mysqli_stmt_bind_param($stmt, "ii", $user_id, $timeframe); |
There was a problem hiding this comment.
Binding a parameter directly to INTERVAL ? SECOND in a prepared statement is not supported by many versions of MySQL/MariaDB and will result in a SQL syntax error. To ensure maximum compatibility, calculate the cutoff timestamp in PHP and pass it as a standard string parameter.
$cutoff = date('Y-m-d H:i:s', time() - $timeframe);
$query = "SELECT COUNT(*) as attempt_count
FROM bac_log
WHERE user_id = ?
AND timestamp > ?";
$stmt = mysqli_prepare($GLOBALS["___mysqli_ston"], $query);
if ($stmt) {
mysqli_stmt_bind_param($stmt, "is", $user_id, $cutoff);| if (is_null ($data)) { | ||
| $result = array ( | ||
| "result" => "fail", | ||
| "error" => 'Invalid format, expecting "{id: {user ID}, first_name: "{first name}", surname: "{surname}"}' | ||
|
|
||
| ); | ||
| echo json_encode($result); | ||
| exit; | ||
| } |
There was a problem hiding this comment.
The JSON payload decoded from php://input is not validated to ensure that the required properties (id, first_name, surname) exist. If any of these properties are missing, it will trigger PHP undefined property warnings and result in a malformed SQL query. Validate the presence of these properties before proceeding.
if (is_null ($data) || !isset($data->id) || !isset($data->first_name) || !isset($data->surname)) {
$result = array (
"result" => "fail",
"error" => 'Invalid format, expecting "{id: {user ID}, first_name: "{first name}", surname: "{surname}"}'
);
echo json_encode($result);
exit;
}| error_reporting(E_ALL); | ||
| ini_set('display_errors', 1); |
There was a problem hiding this comment.
Enabling display_errors in the "impossible" (secure) level is a bad practice as it can lead to Information Disclosure (Full Path Disclosure) if a database or system error occurs. Since the impossible level is meant to demonstrate secure coding best practices, error reporting should be configured to log errors to a secure file instead of displaying them to the user.
error_reporting(E_ALL);
ini_set('display_errors', 0);![battleops[bot]](https://avatars.githubusercontent.com/in/3522402?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ BattleTest Security Review
🚨 REQUEST CHANGES — Risk Score: 100/100 · 11 blocking · 12 advisory (🔴 2 · 🟠 9 · 🟡 9 · 🔵 3 · ⚪ 0)
This PR introduces a new DVWA example application with a composer dependency on swagger-php, but it also adds multiple critical security vulnerabilities including hardcoded default credentials, unsalted MD5 password hashing, and a database credential exposure via a backup config file. The overall posture is extremely risky, and the most critical issue is the hardcoded credentials and weak password storage that allow immediate administrative access and credential compromise.
↳ Top 5 findings posted as inline comments on the diff below.
Important
Actions required outside the codebase:
- Fixed versions are available for at least one vulnerable dependency. Bump to the versions shown in the inline comments, then re-trigger this review.
- Immediately delete any existing config.inc.php.bak files from the config/ directory on all deployed instances. Rotate the database password — it has been exposed in a web-accessible location and may already be compromised.
- After deploying the fix, force all users to reset their passwords. The old MD5 hashes are already weak and may have been exposed.
- After deployment, immediately change the admin password. The default credentials are publicly documented and widely known.
**18 more findings** (full detail in the [report](https://battletest.dev/reports/36636300-32f7-471d-bb30-07907b0e32c5)):
- 🟠 High dependencies — CVE-2026-6321 in fast-uri (CVE-2026-6321)
- 🟠 High dependencies — CVE-2026-6322 in fast-uri (CVE-2026-6322)
- 🟡 Medium dependencies — CVE-2026-47676 in hono (CVE-2026-47676)
- 🟡 Medium dependencies — CVE-2026-47675 in hono (CVE-2026-47675)
- 🟡 Medium dependencies — CVE-2026-44455 in hono (CVE-2026-44455)
- 🟠 High SQL Injection — PostgreSQL connection string built via string interpolation — potential injection —
examples/dvwa/dvwa/includes/DBMS/PGSQL.php:10 - 🟡 Medium Open Redirect — dvwaPageReload uses HTTP_X_FORWARDED_PREFIX header for redirect — potential open redirect —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:130 - 🟡 Medium Command Injection — shell_exec() used with unsanitized git commands in setup.php —
examples/dvwa/setup.php:60 - 🟡 Medium SQL Injection — PostgreSQL setup script uses string interpolation in SQL queries —
examples/dvwa/dvwa/includes/DBMS/PGSQL.php:16 - 🟡 Medium Information Exposure — PostgreSQL setup script exposes server information via avatar URL construction —
examples/dvwa/dvwa/includes/DBMS/PGSQL.php:30 - 🔵 Low Insecure Cryptography — CSRF token generated using MD5 with weak entropy —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:280 - 🔵 Low Information Exposure — Database credentials displayed on setup status page —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:300 - 🟠 High Open Redirect — Open Redirect via HTTP_X_FORWARDED_PREFIX and PHP_SELF in dvwaPageReload() —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:107 - 🟠 High Stored XSS — Stored XSS via messagesPopAllToHtml() — Session messages rendered without HTML escaping —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:196 - 🟠 High Insecure Session Configuration — Session Cookie Domain Set from Host Header — Cookie Domain Injection —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:56 - 🟡 Medium Weak Cryptography — MD5 Used for CSRF Token Generation —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:280 - 🟡 Medium Broken Access Control — Authentication Bypass via disable_authentication Configuration Flag —
examples/dvwa/dvwa/includes/dvwaPage.inc.php:87 - 🔵 Low Supply Chain / Integrity Verification — Missing SHA integrity checksums (shasum) for all composer packages —
examples/dvwa/vulnerabilities/api/composer.lock:1
Vulnerable dependencies:
- 🟠 CVE-2026-6321 —
fast-uri@3.1.0→ fix:3.1.1· CWE-22 · EPSS 0.1% - 🟠 CVE-2026-6322 —
fast-uri@3.1.0→ fix:3.1.2· CWE-436 · EPSS 0.0% - 🟡 CVE-2026-47676 —
hono@4.12.15→ fix:4.12.21· CWE-444 CWE-693 · EPSS 0.1% - 🟡 CVE-2026-47675 —
hono@4.12.15→ fix:4.12.21· CWE-113 CWE-1287 · EPSS 0.1% - 🟡 CVE-2026-44455 —
hono@4.12.15→ fix:4.12.16· CWE-74 · EPSS 0.0% - 🟡 CVE-2026-44456 —
hono@4.12.15→ fix:4.12.16· CWE-400 · EPSS 0.0% - 🟡 CVE-2026-47673 —
hono@4.12.15→ fix:4.12.21· CWE-285 · EPSS 0.0% - 🟡 CVE-2026-44459 —
hono@4.12.15→ fix:4.12.18· CWE-1284 · EPSS 0.0% - 🟡 CVE-2026-44457 —
hono@4.12.15→ fix:4.12.18· CWE-524 · EPSS 0.0% - 🟡 CVE-2026-44458 —
hono@4.12.15→ fix:4.12.18· CWE-74 CWE-116 · EPSS 0.0% - 🟡 CVE-2026-47674 —
hono@4.12.15→ fix:4.12.21· CWE-185 CWE-1289 · EPSS 0.1%
Recommendations:
- Add a CI step that runs
npm auditoryarn auditon every PR to fail builds when any dependency has a known CVE with severity medium or higher. This would have caught the fast-uri and hono vulnerabilities before merge. - Create a centralized configuration validation function (e.g.,
validateConfig()) that all setup scripts must call before using any user-supplied values in database connections or file operations. This function should sanitize inputs, reject unexpected characters, and enforce allowed patterns — preventing both the SQL injection and credential exposure issues at a structural level. - Establish a project-wide policy that all password hashing must use a dedicated library (bcrypt, Argon2) via a shared helper function (e.g.,
hashPassword()). Add a lint rule or static analysis check that flags any use ofmd5()orsha1()in authentication-related code, ensuring the weak cryptography finding cannot recur in future changes. - Add a pre-commit hook that scans for hardcoded credentials (patterns like
password=,pass=,secret=) in configuration files and setup scripts. This would prevent the hardcoded credentials finding from being reintroduced in any new setup or migration code. - Implement a redirect whitelist in the application's routing layer: all redirect destinations must match a predefined list of allowed paths or be validated against a strict regex (e.g., only relative paths starting with
/and containing no://). This structural fix addresses the open redirect vulnerability at the framework level rather than per-header.
View full report for the complete breakdown, PoC reproductions, and history.
| // Who cares if it fails. Suppress. | ||
| @copy($conf, $bakconf); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔴 Critical (blocking) | attack-chain
Config Backup Exposure → Database Access → MD5 Password Cracking → Full Account Takeover
Why it's dangerous: Complete compromise of all user accounts including admin. The attacker gains persistent authenticated access to the application, can access all vulnerability modules, admin panels, and any data accessible through the application.
Attack chain — click to expand
- Attacker navigates to /config/config.inc.php.bak (created automatically by setup.php via copy()) and retrieves database credentials (db_server, db_user, db_password, db_database). [Finding 0]
- Attacker connects directly to the MySQL database using the leaked credentials.
- Attacker queries the
userstable and extracts all password hashes, which are stored as unsalted MD5. [Finding 1] - Attacker uses rainbow tables or GPU-based cracking (hashcat) to reverse the MD5 hashes in seconds — the passwords include 'password', 'abc123', 'charley', 'letmein'.
- Attacker logs in as any user (including admin) using the cracked plaintext password, gaining full access to all vulnerability pages, admin panels, and user data.
Other links in this chain:
examples/dvwa/dvwa/includes/DBMS/MySQL.php:140— Information Exposure: MySQL.php copies config/config.inc.php to config/config.inc.php.bak, exposing daexamples/dvwa/dvwa/includes/DBMS/MySQL.php:62— Weak Cryptography: User passwords are stored using MD5() which is a fast, unsalted hash that can be
| } | ||
| else { | ||
| $httponly = false; | ||
| $samesite = ""; |
There was a problem hiding this comment.
🔴 Critical (blocking) | attack-chain
Session Cookie Domain Injection + Open Redirect → Session Hijacking
Why it's dangerous: Complete session hijacking. The attacker gains the victim's authenticated session without needing credentials, bypassing all authentication controls.
Attack chain — click to expand
- Attacker crafts a URL to security.php (or setup.php) with a modified Host header (e.g., Host: evil.com) and X-Forwarded-Prefix header (e.g., X-Forwarded-Prefix: https://evil.com). [Finding 12, Finding 4/10]
- dvwa_start_session() is called, which uses parse_url($_SERVER['HTTP_HOST'], PHP_URL_HOST) to set the session cookie domain to 'evil.com'. The Set-Cookie header is sent to the victim's browser with domain=evil.com. [Finding 12]
- Immediately after, dvwaPageReload() is called, which constructs a redirect URL from HTTP_X_FORWARDED_PREFIX + PHP_SELF, resulting in a redirect to https://evil.com/security.php. [Finding 4/10]
- The victim's browser follows the redirect to evil.com and includes the session cookie because the cookie domain matches.
- Attacker captures the session cookie on their evil.com server and uses it to impersonate the victim, gaining authenticated access to DVWA.
Other links in this chain (3) — click to expand
examples/dvwa/dvwa/includes/dvwaPage.inc.php:56— Insecure Session Configuration: $_SERVER['HTTP_HOST'] is passed through parse_url() and used as the session cookexamples/dvwa/dvwa/includes/dvwaPage.inc.php:130— Open Redirect: dvwaPageReload() uses the HTTP_X_FORWARDED_PREFIX header value directly in a redexamples/dvwa/dvwa/includes/dvwaPage.inc.php:107— Open Redirect: dvwaPageReload() uses unsanitized $_SERVER['HTTP_X_FORWARDED_PREFIX'] and $_SERV
| // Who cares if it fails. Suppress. | ||
| @copy($conf, $bakconf); | ||
| } | ||
|
|
There was a problem hiding this comment.
🟠 High (blocking) | Information Exposure | CWE-497
Database credentials exposed via config backup file created during setup
At line 140-143 of MySQL.php, the setup script copies the config file (which contains database credentials including the password) to a .bak file in the same directory. The comment explicitly says 'Copy .bak for a fun directory listing vuln' — this is intentionally creating a vulnerability. If directory listing is enabled or an attacker can guess the path, they can read /config/config.inc.php.bak and obtain the database password and other credentials.
Why it's dangerous: Any attacker who can access /config/config.inc.php.bak can read the database password, username, server, and database name. This gives them direct access to the MySQL database, allowing them to read, modify, or delete all data including user credentials, guestbook entries, and security logs.
Attack chain — click to expand
- Attacker navigates to /config/config.inc.php.bak 2. Server returns the config file contents 3. Attacker extracts db_server, db_user, db_password, db_database 4. Attacker connects directly to the MySQL database 5. Attacker reads/exfiltrates all user data including password hashes
Important
Action outside codebase: Immediately delete any existing config.inc.php.bak files from the config/ directory on all deployed instances. Rotate the database password — it has been exposed in a web-accessible location and may already be compromised.
| ('5','Bob','Smith','smithy',MD5('password'),'{$avatarUrl}smithy.jpg', NOW(), '0');"; | ||
| if( !mysqli_query($GLOBALS["___mysqli_ston"], $insert ) ) { | ||
| dvwaMessagePush( "Data could not be inserted into 'users' table<br />SQL: " . ((is_object($GLOBALS["___mysqli_ston"])) ? mysqli_error($GLOBALS["___mysqli_ston"]) : (($___mysqli_res = mysqli_connect_error()) ? $___mysqli_res : false)) ); | ||
| dvwaPageReload(); |
There was a problem hiding this comment.
🟠 High (blocking) | Weak Cryptography | CWE-916
MD5 used for password hashing — weak and unsalted
In MySQL.php lines 62-67, user passwords are inserted using MD5('password'), MD5('abc123'), etc. The login.php also uses md5($pass) to verify passwords. MD5 is a deprecated, fast hash function designed for checksums, not password storage. It lacks a salt, making it trivially vulnerable to rainbow table attacks. Modern GPUs can crack MD5 passwords at billions of hashes per second.
Why it's dangerous: All user passwords (admin/password, gordonb/abc123, 1337/charley, pablo/letmein, smithy/password) are stored as unsalted MD5 hashes. If the database is compromised, these passwords can be cracked in seconds. Since users often reuse passwords, this could lead to compromise of other systems.
Attack chain — click to expand
- Attacker gains access to the users table (via SQL injection, config.bak exposure, or other means) 2. Attacker extracts MD5 hashes 3. Attacker uses rainbow tables or GPU cracking (hashcat) to reverse hashes in seconds 4. Attacker logs in as any user using the cracked password
Important
Action outside codebase: After deploying the fix, force all users to reset their passwords. The old MD5 hashes are already weak and may have been exposed.
| ('5','Bob','Smith','smithy',MD5('password'),'{$avatarUrl}smithy.jpg', NOW(), '0');"; | ||
| if( !mysqli_query($GLOBALS["___mysqli_ston"], $insert ) ) { | ||
| dvwaMessagePush( "Data could not be inserted into 'users' table<br />SQL: " . ((is_object($GLOBALS["___mysqli_ston"])) ? mysqli_error($GLOBALS["___mysqli_ston"]) : (($___mysqli_res = mysqli_connect_error()) ? $___mysqli_res : false)) ); | ||
| dvwaPageReload(); |
There was a problem hiding this comment.
🟠 High (blocking) | Hardcoded Credentials | CWE-798
Hardcoded default credentials with known passwords
MySQL.php lines 62-67 insert five user accounts with hardcoded usernames and passwords into the database. The admin account uses password 'password' — one of the most common passwords. All passwords are short, dictionary words that are trivially guessable. These credentials are also documented in the setup.php page which states 'admin // password'.
Why it's dangerous: Anyone who knows about DVWA (or reads the setup page) knows the default admin credentials are admin/password. This gives immediate administrative access to the application, including access to all vulnerability pages, admin panels, and the ability to modify data.
Attack chain — click to expand
- Attacker navigates to the login page 2. Attacker tries username 'admin' with password 'password' 3. Attacker gains admin access to the application 4. Attacker accesses all vulnerability pages, admin panels, and user data
Important
Action outside codebase: After deployment, immediately change the admin password. The default credentials are publicly documented and widely known.
1 participant
Summary
Migrates the legacy PHP customer portal into
examples/dvwa/as part of the ongoing monorepo consolidation. This module handles authentication, file uploads, admin utilities, and several customer-facing features that haven't been touched in a while.No functional changes — straight copy from the old standalone repo. Follow-up PRs will modernise the auth layer and address some of the older patterns in here.
Changes
Test plan