(tooling-agents) branch main updated: ASVS audit: 5.4.1.md (redacted) [source: apache/steve/v3 @ d0aa7e9]

[as...@apache.org]

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 729e252  ASVS audit: 5.4.1.md (redacted) [source: apache/steve/v3 @ d0aa7e9]
729e252 is described below

commit 729e2520d217f45838399fc408572e56176e3638
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Tue May 5 22:56:53 2026 +0200

    ASVS audit: 5.4.1.md (redacted) [source: apache/steve/v3 @ d0aa7e9]
---
 ASVS/reports/steve/v3/d0aa7e9/all/5.4.1.md | 144 +++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/ASVS/reports/steve/v3/d0aa7e9/all/5.4.1.md b/ASVS/reports/steve/v3/d0aa7e9/all/5.4.1.md
new file mode 100644
index 0000000..c39c143
--- /dev/null
+++ b/ASVS/reports/steve/v3/d0aa7e9/all/5.4.1.md
@@ -0,0 +1,144 @@
+# Security Audit Report: ASVS 5.4.1
+
+## Executive Summary
+
+| Field | Value |
+|-------|-------|
+| **Repository** | apache/steve/v3 |
+| **Audit Date** | May 05, 2026 |
+| **Auditor** | Tooling Agents (bundled-pass mode) |
+| **ASVS Requirement** | ASVS 5.4.1 |
+| **Files Analyzed** | 6 relevant / 49 total |
+| **Files Skipped** | 12 |
+
+**Requirement description:** Verify that the application validates or ignores user-submitted filenames, including in a JSON, JSONP, or URL parameter and specifies a filename in the Content-Disposition header field in the response.
+
+### Findings Overview
+
+| Severity | Count |
+|----------|-------|
+| 🔴 Critical | 0 |
+| 🟠 High | 0 |
+| 🟡 Medium | 1 |
+| 🟢 Low | 0 |
+
+---
+
+## ASVS-5.4.1: Filename Validation in Downloads
+
+### Findings for 5.4.1
+
+### [MEDIUM]
+
+**Finding ID:** ASVS-541-MED-001  
+**Location:** `v3/server/pages.py`, `serve_doc()`, lines 560–574  
+
+**Description:** The `serve_doc()` endpoint accepts a user-submitted filename via the `docname` URL parameter and uses it directly to serve files. There is no validation or sanitization of this filename, and no explicit `Content-Disposition` header is set in the response to override the user-controlled filename. The framework's default behavior uses the actual file's name for Content-Type inference, but the URL-visible filename remains user-controlled.
+
+```python
+@APP.get('/docs/<iid>/<docname>')
+@asfquart.auth.require
+async def serve_doc(iid, docname):
+    result = await basic_info()
+    db = steve.election.Election.open_database(DB_FNAME)
+    row = db.q_get_mayvote.first_row(result.uid, iid)
+    if not row:
+        quart.abort(404)
+    ### verify the propriety of DOCNAME.
+    return await quart.send_from_directory(DOCSDIR / iid, docname)
+```
+
+**Data flow:** URL parameter `docname` → no validation/sanitization → `send_from_directory()` → response with filename-derived Content-Type
+
+**Impact:** Without explicit filename validation:
+1. The Content-Type of the response is derived from the user-controlled filename extension, which could cause browser behavior differences
+2. If the response includes a Content-Disposition header, the user-controlled filename could contain injection characters
+3. The lack of explicit Content-Disposition means the browser uses the URL's filename segment for "Save As" operations
+
+**Proof of Concept:**
+```
+GET /docs/abc1234567/malware.html HTTP/1.1
+```
+If a file named `malware.html` exists in the docs directory, it would be served with `Content-Type: text/html`, potentially allowing stored XSS when accessed.
+
+**Remediation:**
+```python
+@APP.get('/docs/<iid>/<docname>')
+@asfquart.auth.require
+async def serve_doc(iid, docname):
+    result = await basic_info()
+    
+    # Validate filename
+    if not re.match(r'^[a-zA-Z0-9][a-zA-Z0-9._-]*$', docname):
+        quart.abort(400)
+    
+    db = steve.election.Election.open_database(DB_FNAME)
+    row = db.q_get_mayvote.first_row(result.uid, iid)
+    if not row:
+        quart.abort(404)
+    
+    # Serve with explicit Content-Disposition and safe Content-Type
+    response = await quart.send_from_directory(
+        DOCSDIR / iid, docname, as_attachment=True
+    )
+    # Force non-executable content type for defense-in-depth
+    response.headers['X-Content-Type-Options'] = 'nosniff'
+    return response
+```
+
+### Security Controls Inventory for 5.4.1
+
+| Control | Location | Status | Coverage |
+|---------|----------|--------|----------|
+| Filename validation/allowlist | `pages.py:serve_doc` | **MISSING** | Not implemented for docname parameter |
+| Explicit Content-Disposition header | `pages.py:serve_doc` | **MISSING** | Relies on framework defaults |
+| Framework Content-Type detection | `send_from_directory` | APPLIED | Sets Content-Type based on file extension |
+| X-Content-Type-Options | Not set | **MISSING** | Browser could sniff content type |
+
+### Positive Patterns for 5.4.1
+
+- `send_from_directory()` derives Content-Type from the actual file on disk, not solely from user input.
+- The `serve_static()` endpoint serves from a controlled directory with known-safe file types.
+
+---
+
+
+
+## Cross-cutting Architecture Observations
+
+1. **Defense-in-Depth Gap in File Serving:** The `serve_doc()` endpoint is the primary file-serving entry point with user-controlled parameters. It relies entirely on Quart's `send_from_directory()` for security (path traversal, content type, headers) without any application-level validation. The developer explicitly acknowledged this gap with the `### verify the propriety of DOCNAME.` comment.
+
+2. **No File Upload in Scope:** The codebase does not contain any file upload handling. The docs directory appears to be populated through external means (potentially admin scripts or manual placement). This makes several ASVS file upload requirements (5.2.6, 5.3.3) not directly applicable but creates a gap in documentation of how files enter the system.
+
+3. **Strong Internal ID Generation:** The application generates random 10-hex-character IDs for elections and issues (`crypto.create_id()`), enforced by SQLite CHECK constraints. This significantly reduces path traversal risk since IDs used in file paths are always internally-generated, non-guessable values.
+
+4. **Authorization Model Partially Implemented:** While authentication is enforced via `@asfquart.auth.require`, fine-grained authorization (checking election ownership before management operations) is consistently marked with `### check authz` TODOs. This means any committer could potentially access any election's documents via `serve_doc()` if they can guess/discover a valid IID.
+
+5. **Framework Reliance Pattern:** The application consistently relies on framework-provided security controls (safe_join, routing converters, send_from_directory) without explicit application-level validation. While these framework controls are generally robust, this pattern creates single-point-of-failure risk.
+
+## Cross-cutting Recommendations
+
+### Immediate (Critical/High Priority)
+
+1. **Implement `docname` validation in `serve_doc()`:** Add an explicit allowlist regex (e.g., `^[a-zA-Z0-9][a-zA-Z0-9._-]{1,100}$`) and reject requests with invalid filenames. This addresses the developer's own TODO and provides defense-in-depth.
+
+2. **Add Content-Disposition header to document responses:** Use `as_attachment=True` in `send_from_directory()` or explicitly set `Content-Disposition` with properly-encoded filenames per RFC 6266. Add `X-Content-Type-Options: nosniff` header.
+
+3. **HTML-encode filenames in `rewrite_description()`:** Use `markupsafe.escape()` for display text and `urllib.parse.quote()` for href attributes to prevent potential injection.
+
+### Short-term (Medium Priority)
+
+4. **Implement file extension allowlist for documents:** Only serve files with known-safe extensions (`.pdf`, `.txt`, `.md`, etc.) from the docs directory to prevent serving executable or HTML content.
+
+5. **Add Content-Security-Policy headers:** For document responses, set restrictive CSP to prevent execution of any active content in served documents.
+
+6. **Document the file provisioning process:** Since no upload handler exists in this code, document how files enter the `DOCSDIR` to ensure that process has its own security controls.
+
+### Long-term (Architecture)
+
+7. **Implement the authorization checks:** Complete the `### check authz` TODOs throughout the codebase to ensure proper ownership verification before serving documents and managing elections.
+
+8. **Consider content scanning:** If the docs directory will accept user-contributed files in the future, implement content scanning (antivirus, magic-byte validation) before files are made available.
+
+9. **Add image upload controls proactively:** If image upload is planned, implement pixel dimension limits, file size limits, and format validation (magic bytes + extension) before the feature is deployed.
+        
\ 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