You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by as...@apache.org on 2026/04/22 07:46:37 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 1.3.6 (input_validation_and_injection)
This is an automated email from the ASF dual-hosted git repository.
asf-tooling pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-agents.git
The following commit(s) were added to refs/heads/main by this push:
new 03f902b ASVS L3 audit: 1.3.6 (input_validation_and_injection)
03f902b is described below
commit 03f902bf76697d6f55017cb5e2202ebaa69fe47e
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 09:46:32 2026 +0200
ASVS L3 audit: 1.3.6 (input_validation_and_injection)
---
.../input_validation_and_injection/1.3.6.md | 267 +++++++++++++++++++++
1 file changed, 267 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/1.3.6.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/1.3.6.md
new file mode 100644
index 0000000..f177e37
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/1.3.6.md
@@ -0,0 +1,267 @@
+# ASVS 1.3.6 Security Audit Report
+
+## Executive Summary
+
+- **Repository:** apache/steve
+- **Audit Date:** Apr 22, 2026
+- **Auditor:** Tooling Agents
+- **ASVS Requirement:** 1.3.6 — Verify that the application protects against Server-side Request Forgery (SSRF) attacks, by validating untrusted data against an allowlist of protocols, domains, paths and ports and sanitizing potentially dangerous characters before using the data to call another service.
+ - **Level:** 2
+ - **Section:** Sanitization
+ - **Section Description:** The ideal protection against using untrusted content in an unsafe context is to use context-specific encoding or escaping, which maintains the same semantic meaning of the unsafe content but renders it safe for use in that particular context.
+- **Files analyzed:** 23 relevant out of 23 total, 0 skipped
+
+### Finding Counts
+
+| Severity | Count |
+|----------|-------|
+| Critical | 0 |
+| High | 0 |
+| Medium | 1 |
+| Low | 0 |
+| **Total** | **1** |
+
+**Assessment Summary:** This application has a **minimal SSRF attack surface** — it does not make outbound HTTP/network requests based on user input. LDAP connections use hardcoded URLs and the database is local SQLite. The primary concern is in the `serve_doc` endpoint, where untrusted path data is used to access local filesystem resources with a recognized but unimplemented validation gap.
+
+---
+
+## Security Controls Inventory
+
+| Control | Location | Purpose | Coverage Status |
+|---------|----------|---------|-----------------|
+| Parameterized SQL queries | `queries.yaml` | Prevent SQL injection in all database operations | ✅ Full |
+| `send_from_directory` (path traversal protection) | Quart framework | Protect `/docs/<iid>/<docname>`, `/static/<path:filename>` endpoints | ✅ Applied |
+| Docname allowlist validation | `serve_doc` (TODO comment) | Validate document filenames against safe patterns | ❌ Gap - NOT IMPLEMENTED |
+| LDAP URL hardcoding | `asf-load-ldap.py` line 28 | Prevent SSRF via LDAP connection (line 38) | ✅ Full |
+| Database authorization for file access | `serve_doc` line 533 | Verify user authorization via `q_get_mayvote` before serving files | ✅ Applied |
+| IID generation (server-side) | `crypto.create_id()` | Prevent injection via server-generated identifiers in `add_issue`, `create` | ✅ Full |
+| Quart route converters | Framework level | Prevent slashes in route parameters like `/docs/<iid>/<docname>` | ✅ Applied |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py` — Web Endpoints (SSRF Surface)
+
+| Endpoint | User Input Used In | Makes Outbound Request? | SSRF Risk |
+|----------|-------------------|------------------------|-----------|
+| `serve_doc` | `iid`, `docname` → filesystem path | No (local file serving) | Low (path-based) |
+| `serve_static` | `filename` → filesystem path | No (local file serving) | None (`send_from_directory`) |
+| `do_vote_endpoint` | form data → database | No | None |
+| `do_create_endpoint` | form data → database | No | None |
+| `do_set_open_at_endpoint` | JSON date → database | No | None |
+| `do_set_close_at_endpoint` | JSON date → database | No | None |
+| `do_add_issue_endpoint` | form data → database | No | None |
+| `do_edit_issue_endpoint` | form data → database | No | None |
+| `do_delete_issue_endpoint` | URL parameter → database | No | None |
+
+### `v3/server/bin/asf-load-ldap.py` — LDAP Script
+
+| Operation | Input Source | SSRF Risk |
+|-----------|------------|-----------|
+| LDAP connection URL | Hardcoded (`ldaps://ldap-us.apache.org/`) | None |
+| LDAP base DN | Hardcoded (`ou=people,dc=apache,dc=org`) | None |
+| LDAP search filter | Hardcoded (`uid=*`) | None |
+| Bind credentials | Local file (`bind.txt`) | None |
+
+### `v3/queries.yaml` — SQL Queries
+
+| Pattern | Status |
+|---------|--------|
+| All queries use `?` parameter binding | ✅ No SQL injection vectors |
+| No dynamic query construction | ✅ |
+| No user data in table/column names | ✅ |
+
+---
+
+## Findings
+
+### Medium Severity
+
+#### ASVS-136-MED-001: Missing Path Sanitization/Validation for Document Serving Endpoint
+
+**Severity:** MEDIUM
+**Gap Type:** Type B — Control EXISTS (acknowledged via TODO comment) but NOT CALLED
+**File:** `v3/server/pages.py`
+**Function:** `serve_doc` (lines 527–543)
+**Line Numbers:** 527-543
+
+**Related Functions Checked:**
+- `quart.send_from_directory` (framework-level path traversal protection)
+- `q_get_mayvote.first_row` (authorization check, line 533)
+
+**Description:**
+
+The `docname` parameter is user-controllable via the URL and is passed directly to `quart.send_from_directory` without any application-level validation. The developer explicitly recognized this gap with the comment `### verify the propriety of DOCNAME.` but did not implement the control.
+
+This violates the ASVS 1.3.6 principle of validating untrusted data against an allowlist of **paths** and sanitizing dangerous characters before using the data to access a resource. Reliance on a single framework-level protection without defense-in-depth is a risk if a bypass is discovered in `safe_join`.
+
+**Vulnerable Code:**
+
+```python
+@APP.get('/docs/<iid>/<docname>')
+@asfquart.auth.require # fine-grained per-doc authz within the handler
+async def serve_doc(iid, docname):
+ result = await basic_info() # get PID ### grr: called uid
+
+ db = steve.election.Election.open_database(DB_FNAME)
+ row = db.q_get_mayvote.first_row(result.uid, iid)
+ if not row:
+ # Nothing fancy. Just pretend the file does not exist
+ quart.abort(404)
+ # NOTREACHED
+
+ ### verify the propriety of DOCNAME. # <-- EXPLICIT TODO: Not implemented
+
+ # Return per-issue document.
+ return await quart.send_from_directory(DOCSDIR / iid, docname)
+```
+
+**Data Flow:**
+
+```
+URL parameter `docname` (user-controlled)
+ → serve_doc(iid, docname)
+ → NO explicit validation/sanitization of docname
+ → quart.send_from_directory(DOCSDIR / iid, docname) [filesystem access]
+```
+
+**Current Mitigations:**
+- Quart's `<docname>` converter prevents slashes (single path segment only)
+- `send_from_directory` uses werkzeug's `safe_join` internally to prevent `..` traversal
+- The `iid` parameter is validated against the `mayvote` database table
+- Authorization check via `q_get_mayvote` before file serving
+
+**Proof of Concept:**
+
+```http
+GET /docs/valid-iid-here/..%00secret.txt HTTP/1.1
+Host: target
+Cookie: session=valid-session
+```
+
+While `safe_join` should block this, null-byte or encoding bypasses in specific framework versions could allow access to unintended files within the DOCSDIR tree.
+
+**Impact:**
+- Potential local file disclosure within the `DOCSDIR` directory subtree if framework-level path traversal protection is bypassed
+- The developer explicitly recognized additional validation was needed but didn't implement it, creating false confidence in the security posture
+- Violation of defense-in-depth principle by relying solely on framework-level controls
+
+**Remediation:**
+
+```python
+import re
+
+# Allowlist for document filenames: alphanumeric, hyphens, underscores, dots
+SAFE_DOCNAME = re.compile(r'^[a-zA-Z0-9][a-zA-Z0-9._-]{0,254}$')
+
+@APP.get('/docs/<iid>/<docname>')
+@asfquart.auth.require
+async def serve_doc(iid, docname):
+ result = await basic_info()
+
+ # Validate docname against allowlist
+ if not SAFE_DOCNAME.match(docname):
+ quart.abort(400, 'Invalid document name')
+
+ # Reject path traversal components explicitly
+ if '..' in docname or docname.startswith('.'):
+ quart.abort(400, 'Invalid document name')
+
+ db = steve.election.Election.open_database(DB_FNAME)
+ row = db.q_get_mayvote.first_row(result.uid, iid)
+ if not row:
+ quart.abort(404)
+
+ return await quart.send_from_directory(DOCSDIR / iid, docname)
+```
+
+---
+
+## Positive Security Patterns
+
+1. **No outbound HTTP client usage**: The application imports no HTTP client libraries (`requests`, `urllib`, `httpx`, `aiohttp`). No web endpoints construct or fetch external URLs based on user input. This eliminates the primary SSRF attack vector entirely.
+
+2. **Hardcoded LDAP connection parameters**: `asf-load-ldap.py` (lines 28-30) uses constants for LDAP URL, DN, and filter — no user input influences the LDAP connection target.
+
+3. **Server-generated identifiers**: Election IDs (`eid`) and Issue IDs (`iid`) are generated by `crypto.create_id()` (lines 197, 415 of `election.py`), not derived from user input. This prevents injection of malicious path components via IDs.
+
+4. **Authorization check before file serving**: `serve_doc` validates that the authenticated user has a `mayvote` entry for the requested `iid` before serving any file (line 533), preventing unauthorized access even if path traversal were possible.
+
+5. **Parameterized SQL throughout**: All queries in `queries.yaml` use `?` placeholders, and `asfpy.db` binds parameters safely. No string interpolation in SQL construction.
+
+6. **Framework-level path safety**: Both `serve_doc` and `serve_static` use `quart.send_from_directory`, which employs werkzeug's `safe_join` to prevent directory traversal in filenames.
+
+7. **Quart route converters limit input**: Standard `<param>` converters (used for `<eid>`, `<iid>`, `<docname>`) reject slashes, limiting path injection to single path segments.
+
+---
+
+## Architecture Observations
+
+1. **Minimal SSRF attack surface by design**: This application is primarily a form-processing and database-backed system. It does not proxy requests, fetch URLs, or call external services based on user input. The SSRF risk profile is inherently low.
+
+2. **LDAP script is CLI-only**: `asf-load-ldap.py` is a batch script run by administrators, not exposed as a web endpoint. Even though it makes network calls (LDAP), these are not reachable from the web interface.
+
+3. **File serving is the closest SSRF analog**: The `serve_doc` and `serve_static` endpoints are the only places where user input influences resource retrieval. Both serve local files, not remote resources.
+
+4. **No URL construction from user data**: Redirects use either hardcoded paths or database-validated identifiers (e.g., `quart.redirect(f'/manage/{election.eid}')` where `eid` is verified to exist).
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (Priority 1)
+
+| Recommendation | Effort | Impact |
+|---------------|--------|--------|
+| Implement explicit `docname` allowlist validation in `serve_doc` to close the acknowledged TODO gap | Low | High - Addresses the only identified finding and implements defense-in-depth |
+
+### Short-term Actions (Priority 2)
+
+| Recommendation | Effort | Impact |
+|---------------|--------|--------|
+| Add a `SAFE_IID` regex check on `iid` in `serve_doc` for defense-in-depth (even though DB check exists) | Low | Medium - Additional layer of validation |
+| Document the application's minimal SSRF surface in architecture docs so future developers maintain this property | Low | Medium - Prevents regression |
+
+### Long-term Actions (Priority 3)
+
+| Recommendation | Effort | Impact |
+|---------------|--------|--------|
+| If future features add URL fetching (webhooks, imports, etc.), implement a URL allowlist with protocol/domain/port restrictions per ASVS 1.3.6 requirements | Medium | High - Proactive protection for future features |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand full file list (23 files)</summary>
+
+1. `v3/queries.yaml` (relevance: 10)
+2. `v3/server/api.py` (relevance: 10)
+3. `v3/server/bin/asf-load-ldap.py` (relevance: 10)
+4. `v3/server/bin/create-election.py` (relevance: 10)
+5. `v3/server/pages.py` (relevance: 10)
+6. `v3/server/templates/about.ezt` (relevance: 10)
+7. `v3/server/templates/admin.ezt` (relevance: 10)
+8. `v3/server/templates/e_bad_eid.ezt` (relevance: 10)
+9. `v3/server/templates/e_bad_iid.ezt` (relevance: 10)
+10. `v3/server/templates/e_bad_pid.ezt` (relevance: 10)
+11. `v3/server/templates/flashes.ezt` (relevance: 10)
+12. `v3/server/templates/footer.ezt` (relevance: 10)
+13. `v3/server/templates/header.ezt` (relevance: 10)
+14. `v3/server/templates/home.ezt` (relevance: 10)
+15. `v3/server/templates/manage-stv.ezt` (relevance: 10)
+16. `v3/server/templates/manage.ezt` (relevance: 10)
+17. `v3/server/templates/privacy.ezt` (relevance: 10)
+18. `v3/server/templates/profile.ezt` (relevance: 10)
+19. `v3/server/templates/settings.ezt` (relevance: 10)
+20. `v3/server/templates/vote-on.ezt` (relevance: 10)
+21. `v3/server/templates/voter.ezt` (relevance: 10)
+22. `v3/steve/election.py` (relevance: 10)
+23. `v3/steve/persondb.py` (relevance: 10)
+
+</details>
+
+---
+
+**End of Report**
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org