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 05:45:52 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 16.4.1 (logging_and_monitoring)
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 95eae26 ASVS L3 audit: 16.4.1 (logging_and_monitoring)
95eae26 is described below
commit 95eae26cb5956fee7ecad6db6d719721d373d475
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 07:45:47 2026 +0200
ASVS L3 audit: 16.4.1 (logging_and_monitoring)
---
.../v3/d0aa7e9/logging_and_monitoring/16.4.1.md | 436 +++++++++++++++++++++
1 file changed, 436 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/logging_and_monitoring/16.4.1.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/logging_and_monitoring/16.4.1.md
new file mode 100644
index 0000000..ea2a53a
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/logging_and_monitoring/16.4.1.md
@@ -0,0 +1,436 @@
+# ASVS 16.4.1 Security Audit Report
+
+## Executive Summary
+
+**Repository:** apache/steve
+**Audit Date:** Apr 22, 2026
+**Auditor:** Tooling Agents
+**ASVS Requirement:** 16.4.1 — Verify that all logging components appropriately encode data to prevent log injection.
+**Level:** 2
+**Section:** Log Protection
+**Section Description:** Logs are valuable forensic artifacts and must be protected. If logs can be easily modified or deleted, they lose their integrity and become unreliable for incident investigations or legal proceedings. Logs may expose internal application behavior or sensitive metadata, making them an attractive target for attackers.
+
+**Files Analyzed:** 4 relevant out of 4 total, 0 skipped
+
+### Finding Summary
+
+| Severity | Count | IDs |
+|----------|-------|-----|
+| Critical | 0 | — |
+| High | 0 | — |
+| Medium | 3 | ASVS-1641-MED-001, ASVS-1641-MED-002, ASVS-1641-MED-003 |
+| Low | 0 | — |
+| **Total** | **3** | |
+
+**Key Risk:** The audit identified three findings where user-controllable data flows into logging components without appropriate encoding, allowing log injection attacks. In an election system where logs serve as the primary audit mechanism, this directly undermines forensic reliability.
+
+---
+
+## Security Controls Inventory
+
+### Logging Controls Analysis
+
+| Control | Defined | Applied Consistently | Coverage |
+|---------|---------|---------------------|----------|
+| Structured log format `[{asctime}\|{levelname}\|{name}] {message}` | `main.py:57` | Yes (root logger) | All modules |
+| Bracketed identifiers `[U:uid]`, `[E:eid]`, `[I:iid]` | `pages.py` (various) | Yes, all security log lines | Good convention, no encoding |
+| User input encoding before logging | **NOT DEFINED** | **NOT APPLIED** | **No coverage** |
+| Vote content exclusion from logs | `pages.py:392-394` | Yes | All vote logging |
+| Sensitive data exclusion (salts, keys) | `election.py:171,187` | Yes | API returns |
+
+### Entry Points Logging User-Controlled Data
+
+| Endpoint | User Data Logged | Encoding Applied | Status |
+|----------|-----------------|------------------|--------|
+| `POST /do-create-election` | `form.title` | ❌ None | **VULNERABLE** |
+| `GET /manage/<eid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `GET /vote-on/<eid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `POST /do-vote/<eid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `GET /do-open/<eid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `GET /do-close/<eid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `POST /do-add-issue/<eid>` | `eid` (URL param) + raw form via `print()` | ❌ None | **VULNERABLE** |
+| `POST /do-edit-issue/<eid>/<iid>` | `eid` (URL param) + raw form via `print()` | ❌ None | **VULNERABLE** |
+| `POST /do-delete-issue/<eid>/<iid>` | `eid` (URL param) | ❌ None | **VULNERABLE** |
+| `POST /do-set-open_at/<eid>` | `eid` (URL) + `date_str` | ❌ / ✅ (validated) | Partially safe |
+| `POST /do-set-close_at/<eid>` | `eid` (URL) + `date_str` | ❌ / ✅ (validated) | Partially safe |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py`
+
+| Line(s) | Pattern | User Data | Encoded | Finding |
+|---------|---------|-----------|---------|---------|
+| 104–106 | `_LOGGER.info(f'...{date_str}')` | `date_str` | Validated via `fromisoformat` | ✅ Safe |
+| 392–394 | `_LOGGER.info(f'...{iid}...')` | `iid` from form | Validated against `issue_dict` | ✅ Safe |
+| 396–398 | `_LOGGER.error(f'...{iid}...{e}')` | `iid` + exception | `iid` validated; `{e}` internal | ✅ Safe |
+| **429–431** | `_LOGGER.info(f'...{form.title}')` | `form.title` raw | **❌ No encoding** | **ASVS-1641-MED-001** |
+| 454 | `_LOGGER.info(f'...{election.eid}')` | `eid` from DB | Post-validation | ✅ Safe |
+| 469 | `_LOGGER.info(f'...{election.eid}')` | `eid` from DB | Post-validation | ✅ Safe |
+| **489, 513** | `print('FORM:', form)` | All form data | **❌ No encoding** | **ASVS-1641-MED-003** |
+| 494–496 | `_LOGGER.info(f'...{iid}...')` | `iid` generated | `crypto.create_id()` | ✅ Safe |
+
+### `v3/steve/election.py`
+
+| Line(s) | Pattern | User Data | Encoded | Finding |
+|---------|---------|-----------|---------|---------|
+| **40** | `_LOGGER.debug(f'..."{eid}"')` | `eid` from URL | **❌ Before validation** | **ASVS-1641-MED-002** |
+| 201 | `_LOGGER.info(f'...{iid}...')` | `iid` generated | `crypto.create_id()` | ✅ Safe |
+| 220 | `_LOGGER.info(f'...{iid}...')` | `iid` from param | Post-existence check | ✅ Safe |
+| 433 | `_LOGGER.info(f'...{eid}')` | `eid` generated | `crypto.create_id()` | ✅ Safe |
+
+### `v3/server/bin/tally.py`
+
+| Line(s) | Pattern | User Data | Encoded | Finding |
+|---------|---------|-----------|---------|---------|
+| 113–114 | `_LOGGER.info(f'...{issue_id}')` | CLI argument | ❌ No encoding | Low (server access required) |
+| 115 | `_LOGGER.error(f'...{issue_id}...')` | CLI argument | ❌ No encoding | Low (server access required) |
+| 117 | `_LOGGER.info(f'...{len(issues)}...')` | Integer | N/A | ✅ Safe |
+
+### `v3/server/main.py`
+
+| Line(s) | Pattern | User Data | Encoded | Finding |
+|---------|---------|-----------|---------|---------|
+| 57 | `logging.basicConfig(level=logging.DEBUG, ...)` | N/A | N/A | Enables DEBUG-level injection vectors |
+
+---
+
+## Findings
+
+### ASVS-1641-MED-001 — Log Injection via Unencoded Election Title
+
+**Severity:** MEDIUM
+**Gap Type:** Type A — Entry point with NO log encoding control
+**File:** `v3/server/pages.py`
+**Function:** `do_create_endpoint()`
+**Lines:** ~429–431
+**Related Functions Checked:** `Election.create()`, `_LOGGER.info()`
+
+#### Description
+
+The `do_create_endpoint()` function logs the user-provided election title directly without encoding newlines or control characters, enabling log injection attacks that can forge audit trail entries.
+
+#### Vulnerable Code
+
+```python
+form = edict(await quart.request.form)
+
+# Create the Election.
+election = steve.election.Election.create(DB_FNAME, form.title, result.uid)
+
+_LOGGER.info(
+ f'User[U:{result.uid}] created election[E:{election.eid}];'
+ f' title: "{form.title}"'
+)
+```
+
+#### Data Flow
+
+```
+POST /do-create-election (form body)
+ → form.title (raw user input, no encoding)
+ → _LOGGER.info() f-string interpolation
+ → log output (newlines create forged entries)
+```
+
+#### Proof of Concept
+
+```http
+POST /do-create-election HTTP/1.1
+Content-Type: application/x-www-form-urlencoded
+Cookie: [valid PMC member session]
+
+title=Budget%0A[01/15%2014:30|INFO|pages]%20User[U:admin2]%20opened%20election[E:e-target123]
+```
+
+#### Resulting Log Output
+
+```
+[01/15 14:30|INFO|pages] User[U:attacker] created election[E:e-abc123]; title: "Budget
+[01/15 14:30|INFO|pages] User[U:admin2] opened election[E:e-target123]"
+```
+
+The second line is indistinguishable from a legitimate log entry showing `admin2` opened a different election.
+
+#### Impact
+
+A malicious PMC member can forge audit log entries in an election system. This can be used to fabricate evidence of election state changes, blame other administrators for actions they didn't perform, or obscure unauthorized operations. In the election auditing context, this directly undermines log forensic integrity.
+
+#### Remediation
+
+```python
+import re
+
+def sanitize_for_log(value: str) -> str:
+ """Remove or encode characters that could enable log injection."""
+ if value is None:
+ return 'None'
+ # Replace newlines, carriage returns, and other control characters
+ return re.sub(r'[\r\n\x00-\x1f\x7f-\x9f]', '\\\\n', str(value))
+
+# In do_create_endpoint:
+_LOGGER.info(
+ 'User[U:%s] created election[E:%s]; title: "%s"',
+ sanitize_for_log(result.uid),
+ sanitize_for_log(election.eid),
+ sanitize_for_log(form.title),
+)
+```
+
+---
+
+### ASVS-1641-MED-002 — Log Injection via URL Path Parameters in Election Constructor
+
+**Severity:** MEDIUM
+**Gap Type:** Type D — Log occurs BEFORE validation (existence check)
+**File:** `v3/steve/election.py`
+**Function:** `Election.__init__()`
+**Line:** ~40
+**Related Functions Checked:** `open_database()`, `q_check_election()`, `not_found()`, all `@load_election` decorated endpoints
+
+#### Description
+
+The `Election` constructor logs the `eid` parameter before validating it against the database, allowing log injection through 11 different endpoints that use the `@load_election` decorator.
+
+#### Vulnerable Code
+
+```python
+def __init__(self, db_fname, eid, op='Opening'):
+ _LOGGER.debug(f'{op} election ID "{eid}"') # Logged BEFORE validation
+
+ self.db = self.open_database(db_fname)
+ self.eid = eid
+
+ if not_found(self.q_check_election, eid): # Validation AFTER log
+ raise ElectionNotFound(eid)
+```
+
+#### Data Flow
+
+```
+GET /manage/<eid> (URL path parameter)
+ → load_election decorator → Election(DB_FNAME, eid)
+ → _LOGGER.debug(f'... "{eid}"') ← injection point (BEFORE db check)
+ → ElectionNotFound raised (AFTER log already written)
+```
+
+#### Affected Endpoints
+
+11 routes through `load_election` / `load_election_issue`:
+- `GET /vote-on/<eid>`
+- `GET /manage/<eid>`
+- `GET /manage-stv/<eid>/<iid>`
+- `POST /do-set-open_at/<eid>`
+- `POST /do-set-close_at/<eid>`
+- `POST /do-vote/<eid>`
+- `GET /do-open/<eid>`
+- `GET /do-close/<eid>`
+- `POST /do-add-issue/<eid>`
+- `POST /do-edit-issue/<eid>/<iid>`
+- `POST /do-delete-issue/<eid>/<iid>`
+
+#### Proof of Concept
+
+```http
+GET /manage/fake%0A[01/15%2014:30|INFO|pages]%20User[U:admin]%20closed%20election[E:e-real123] HTTP/1.1
+Cookie: [valid committer session]
+```
+
+#### Resulting Log Output
+
+```
+[01/15 14:30|DEBUG|steve.election] Opening election ID "fake
+[01/15 14:30|INFO|pages] User[U:admin] closed election[E:e-real123]"
+```
+
+#### Why This Is Exploitable Despite DEBUG Level
+
+Both `run_standalone()` and `run_asgi()` in `main.py` set the root logger to `logging.DEBUG`:
+
+```python
+# main.py line 57
+logging.basicConfig(level=logging.DEBUG, ...)
+```
+
+The `steve.election` logger inherits this level and is not overridden.
+
+#### Impact
+
+Any authenticated committer (lower privilege than PMC member) can inject arbitrary log entries across 11 endpoints. The injection occurs before the election ID is validated against the database, so completely arbitrary content is logged. Attackers can forge entries that appear to show election openings, closings, or vote submissions by other users.
+
+#### Remediation
+
+**Option 1: Sanitize before logging**
+
+```python
+def __init__(self, db_fname, eid, op='Opening'):
+ # Sanitize before logging
+ safe_eid = re.sub(r'[\r\n\x00-\x1f\x7f-\x9f]', '', str(eid))[:64]
+ _LOGGER.debug(f'{op} election ID "{safe_eid}"')
+
+ self.db = self.open_database(db_fname)
+ self.eid = eid
+
+ if not_found(self.q_check_election, eid):
+ raise ElectionNotFound(eid)
+```
+
+**Option 2: Move log statement after validation (preferred)**
+
+```python
+def __init__(self, db_fname, eid, op='Opening'):
+ self.db = self.open_database(db_fname)
+ self.eid = eid
+
+ if not_found(self.q_check_election, eid):
+ raise ElectionNotFound(eid)
+
+ # Log only after validation confirms this is a real election ID
+ _LOGGER.debug(f'{op} election ID "{eid}"')
+```
+
+---
+
+### ASVS-1641-MED-003 — Raw User Form Data Written to stdout via print()
+
+**Severity:** MEDIUM
+**Gap Type:** Type A — No encoding control on output channel
+**File:** `v3/server/pages.py`
+**Functions:** `do_add_issue_endpoint()` (line ~489), `do_edit_issue_endpoint()` (line ~513)
+**Related Functions Checked:** `quart.request.form`, `edict()`, stdout capture in Hypercorn
+
+#### Description
+
+Both issue management endpoints dump raw form data to stdout using `print()`, bypassing the logging framework and writing unencoded user input that intermingles with structured logs in ASGI deployments.
+
+#### Vulnerable Code
+
+Both locations:
+
+```python
+form = edict(await quart.request.form)
+print('FORM:', form)
+```
+
+#### Data Flow
+
+```
+POST /do-add-issue/<eid> (form body with arbitrary fields/values)
+ → quart.request.form (URL-decoded form data)
+ → edict(form) (preserves all fields including attacker-controlled)
+ → print() → stdout → server log capture (Hypercorn/process manager)
+```
+
+#### Proof of Concept
+
+```http
+POST /do-add-issue/e-abc123 HTTP/1.1
+Content-Type: application/x-www-form-urlencoded
+Cookie: [valid committer session]
+
+title=Normal+Issue&description=test%0A[01/15%2014:30|INFO|pages]%20User[U:admin]%20opened%20election[E:e-target]
+```
+
+#### Resulting stdout Output
+
+```
+FORM: {'title': 'Normal Issue', 'description': 'test
+[01/15 14:30|INFO|pages] User[U:admin] opened election[E:e-target]'}
+```
+
+In Hypercorn ASGI deployment (configured in `main.py`), stdout is captured alongside structured log output, making injected lines appear alongside legitimate entries.
+
+#### Impact
+
+All form field names and values — including newlines and control characters — are dumped to stdout without encoding. In ASGI deployments, this output intermingles with the structured logging output. Additionally, `print()` bypasses the logging framework entirely, providing no timestamp, level, or source metadata for forensic distinction.
+
+#### Remediation
+
+Remove the debug `print()` statements entirely, or replace with properly encoded logging:
+
+```python
+# Remove these lines:
+# print('FORM:', form)
+
+# If debug output is needed, use the logging framework with encoding:
+_LOGGER.debug('Form received: keys=%s', list(form.keys()))
+```
+
+---
+
+## Positive Security Patterns
+
+1. **Vote content never logged:** `do_vote_endpoint` logs `User[U:{uid}] voted on issue[I:{iid}]` without the vote value (`votestring`), preserving ballot secrecy per domain requirements.
+
+2. **Structured identifier format:** All security log entries use consistent bracket notation `[U:uid]`, `[E:eid]`, `[I:iid]`, enabling reliable parsing by SIEM tools.
+
+3. **Date input validated before logging:** In `_set_election_date`, `datetime.fromisoformat()` validation rejects inputs containing control characters before the log statement is reached, preventing injection through the date parameter.
+
+4. **Generated IDs used post-validation:** Most log statements for successful operations use `election.eid` or `iid` values that have either been generated by `crypto.create_id()` or validated against the database, ensuring clean values in successful-operation audit entries.
+
+5. **Consistent logging format with metadata:** The `[{asctime}|{levelname}|{name}] {message}` format provides timestamp, severity, and source module for each entry, supporting forensic analysis.
+
+6. **Security events comprehensively logged:** Election creation, opening, closing, vote submission, issue management, and date changes all generate INFO-level log entries with actor identification.
+
+---
+
+## Architecture Observations
+
+1. **No centralized log sanitization utility:** The codebase lacks a shared function for sanitizing user input before logging. Each log statement independently decides whether to encode data (and currently none do). A centralized `sanitize_for_log()` utility would provide consistent protection.
+
+2. **Mixed output channels:** The codebase uses both `logging` (structured) and `print()` (unstructured) for output. In ASGI deployment, both go to the same output stream, but `print()` bypasses the logging framework's formatting and level controls, making forged entries harder to distinguish.
+
+3. **DEBUG level in production configurations:** Both `run_standalone()` and `run_asgi()` configure the root logger at `DEBUG` level. This expands the attack surface by enabling log injection through debug-level log statements that would otherwise be silent.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions
+
+1. **Create a centralized log sanitization function** and apply it to all user-controllable values before logging:
+ ```python
+ # steve/logging_utils.py
+ import re
+
+ def safe_log(value, max_length=256):
+ """Sanitize a value for safe inclusion in log messages."""
+ if value is None:
+ return 'None'
+ s = str(value)[:max_length]
+ # Replace control characters (newlines, carriage returns, etc.)
+ return re.sub(r'[\x00-\x1f\x7f-\x9f]', '⌀', s)
+ ```
+
+2. **Remove `print()` debug statements** from production code (`pages.py` lines ~489, ~513). Replace with `_LOGGER.debug()` using the sanitization function if debug output is still needed.
+
+3. **Apply sanitization to ASVS-1641-MED-001:** Encode `form.title` in `do_create_endpoint()` before logging (line ~431).
+
+### Short-Term Actions
+
+4. **Move the log statement in `Election.__init__`** (ASVS-1641-MED-002) to after the existence check, so only validated election IDs are logged. Alternatively, sanitize the `eid` before logging.
+
+5. **Reduce production log level** from `DEBUG` to `INFO` in `main.py` to limit the surface area for log injection through debug-level statements.
+
+### Long-Term Actions
+
+6. **Audit all `_LOGGER` calls** across the full codebase (including modules not provided: `api.py`, `steve/crypto.py`, `steve/persondb.py`) using the same methodology to identify additional unencoded user inputs.
+
+7. **Implement automated testing** for log injection vulnerabilities as part of the CI/CD pipeline.
+
+8. **Consider structured logging library** (e.g., `structlog`) that separates log data from format strings, making injection attacks structurally impossible.
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list (4 files)</summary>
+
+- `v3/server/bin/tally.py` (relevance: 10)
+- `v3/server/main.py` (relevance: 10)
+- `v3/server/pages.py` (relevance: 10)
+- `v3/steve/election.py` (relevance: 10)
+
+</details>
\ 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