feature: add contact-form PII plaintext storage detection rule#220
feature: add contact-form PII plaintext storage detection rule#220ticket-fixer[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review Summary
Total Issues Found: 2
Critical Issues: 0
Suggestions: 2
Two minor, non-critical suggestions were found:
- A list is embedded directly in an f-string (line 247), producing ugly
reproutput — use', '.join(matching)for clean comma-separated output. - A redundant
Content-Typeheader (line 90) is set explicitly despiterequestsautomatically setting it when using thejson=parameter.
No critical or functional issues were identified. The code is functionally sound and only needs minor readability and code-hygiene polish. ✅
| vulnerability = self.create_vulnerability(target) | ||
| vulnerability.technical_detail = ( | ||
| f"{endpoint} exposes an unauthenticated GraphQL contact-form " | ||
| f"mutation {matching} that collects PII (email/phone) and persists " |
There was a problem hiding this comment.
matching is a list[str], and embedding it directly in an f-string will produce the Python repr of the list (e.g., ['SendContactMessageMutation', 'ContactUs']), including brackets and quotes. This results in ugly, less readable vulnerability reports. Use ', '.join(matching) instead to produce clean, comma-separated output.
| f"mutation {matching} that collects PII (email/phone) and persists " | |
| f"mutation {', '.join(matching)} that collects PII (email/phone) and persists " |
| response = session.post( | ||
| url, | ||
| json={"query": INTROSPECTION_QUERY}, | ||
| headers={"Content-Type": "application/json"}, |
There was a problem hiding this comment.
The Content-Type: application/json header is set automatically when using the json= parameter in requests. Setting it explicitly here is redundant and can be removed to reduce clutter.
Summary
Adds a generic detection rule for the "Contact-Form PII Stored in Plaintext Without Encryption in Ticket.description" finding (ticket os-35096). The rule detects the externally observable surface of this vulnerability class: an unauthenticated public GraphQL endpoint that exposes a contact-form mutation collecting PII (
email/phone). The plaintext storage is the documented backend reality; a remote scanner cannot verify at-rest encryption, so the rule reports the PII-collection surface as a low-severity, defense-in-depth / data-minimization gap (privacy issue).Changes
agent/exploits/contact_form_pii_plaintext_storage.py— newContactFormPiiPlaintextStorageExploit(registered viaexploits_registry). It discovers a GraphQL endpoint across common candidate paths, runs an introspection query, and flags contact-form mutations (name contains "contact") whose arguments declare bothemailandphone. Metadata carries the ticket's title, description, Low risk rating, references (OWASP cryptographic storage cheat sheet, PostgreSQL pgcrypto), and the field/DB-level encryption recommendation;privacy_issue=True,security_issue=False,has_public_exploit=False.tests/exploits/contact_form_pii_plaintext_storage_test.py— 8 unit tests (usingrequests_mock) covering accept/check for: vulnerable contact mutation, no GraphQL endpoint, PII-collecting mutation reported, contact mutation missing PII args, no contact mutation, introspection disabled, request exception, and multiple matching mutations.Verification
ruff format --check✅ruff check✅mypy✅ (258 files, no issues)pytest tests/exploits/contact_form_pii_plaintext_storage_test.py✅ (8 passed)The 5 pre-existing test failures in
tests/exploits_registry_test.py/tests/asteroid_agent_test.pyare unrelated — they stem frompysnmp/pwntools(pwn) modules failing to build in this sandbox; in CI those dependencies install normally andexploits.import_all()succeeds. Confirmed the failures reproduce identically without these changes.Closes os-35096.