(tooling-agents) branch main updated: ASVS audit: 15.4.1.md (redacted)

[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 0701c75  ASVS audit: 15.4.1.md (redacted)
0701c75 is described below

commit 0701c7501c0573d2d878ee2c03807e75538854fd
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Tue May 5 14:04:42 2026 +0200

    ASVS audit: 15.4.1.md (redacted)
---
 ASVS/reports/steve/v3/d0aa7e9/all/15.4.1.md | 178 ++++++++++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/ASVS/reports/steve/v3/d0aa7e9/all/15.4.1.md b/ASVS/reports/steve/v3/d0aa7e9/all/15.4.1.md
new file mode 100644
index 0000000..126f6f8
--- /dev/null
+++ b/ASVS/reports/steve/v3/d0aa7e9/all/15.4.1.md
@@ -0,0 +1,178 @@
+# Security Audit Report: ASVS 15.4.1
+
+## Executive Summary
+
+| Field | Value |
+|-------|-------|
+| **Repository** | apache/steve/v3 |
+| **Audit Date** | May 05, 2026 |
+| **Auditor** | Tooling Agents (bundled-pass mode) |
+| **ASVS Requirement** | ASVS 15.4.1 |
+| **Files Analyzed** | 3 relevant / 49 total |
+| **Files Skipped** | 12 |
+
+**Requirement description:** Verify that shared objects in multi-threaded code (such as caches, files, or in-memory objects accessed by multiple threads) are accessed safely by using thread-safe types and synchronization mechanisms like locks or semaphores to avoid race conditions and data corruption.
+
+### Findings Overview
+
+| Severity | Count |
+|----------|-------|
+| 🔴 Critical | 0 |
+| 🟠 High | 0 |
+| 🟡 Medium | 1 |
+| 🟢 Low | 1 |
+
+---
+
+## ASVS-15.4.1: Thread-Safe Shared Objects
+
+### Findings for 15.4.1
+
+### [MEDIUM]
+**Finding ID:** ASVS-1541-MED-001
+
+**File:** `v3/steve/election.py`, method `delete`, lines 50-67; method `add_salts`, lines 116-133
+
+**Vulnerable code:**
+```python
+def delete(self):
+    assert self.is_editable()
+    self.db.conn.execute('BEGIN TRANSACTION')
+    self.c_delete_mayvote.perform(self.eid)
+    self.c_delete_issues.perform(self.eid)
+    self.c_delete_election.perform(self.eid)
+    self.db.conn.execute('COMMIT')
+    self.db.conn.close()
+    self.db = None
+```
+
+**Data flow:** Multiple concurrent async requests → same SQLite database file → potential `SQLITE_BUSY` errors
+
+**Proof of concept:** Two concurrent requests: one calls `do_open_endpoint` (which triggers `add_salts()` with transaction), another simultaneously calls `do_close_endpoint` on the same election. Both obtain separate `Election` instances pointing to the same database file. SQLite's file-level locking means one will get `SQLITE_BUSY` if the lock timeout isn't configured.
+
+**Impact:** Under concurrent access, `SQLITE_BUSY` exceptions could propagate unhandled, causing 500 errors and potentially leaving transactions in an inconsistent state (no rollback handling).
+
+**Remediation:**
+```python
+import contextlib
+
+def delete(self):
+    assert self.is_editable()
+    try:
+        self.db.conn.execute('BEGIN TRANSACTION')
+        self.c_delete_mayvote.perform(self.eid)
+        self.c_delete_issues.perform(self.eid)
+        self.c_delete_election.perform(self.eid)
+        self.db.conn.execute('COMMIT')
+    except sqlite3.OperationalError:
+        self.db.conn.execute('ROLLBACK')
+        raise
+    finally:
+        self.db.conn.close()
+        self.db = None
+```
+
+Additionally, configure `PRAGMA busy_timeout` on connection creation:
+```python
+@staticmethod
+def open_database(db_fname):
+    db = asfpy.db.DB(db_fname, yaml_fname=QUERIES, yaml_section='election')
+    db.conn.execute('PRAGMA busy_timeout = 5000')  # 5 second wait
+    return db
+```
+
+---
+
+### [LOW]
+**Finding ID:** ASVS-1541-LOW-001
+
+**File:** `v3/server/pages.py`, module-level constants, lines 37-41
+
+**Vulnerable code:**
+```python
+DB_FNAME = THIS_DIR / APP.cfg.db
+```
+
+**Data flow:** All request handlers → `Election(DB_FNAME, eid)` → new connection per request → shared SQLite file
+
+**Impact:** The application creates a new database connection for each request (via `Election.__init__` → `open_database`). While this avoids sharing connection objects between concurrent async tasks, it means no connection pooling or coordination exists. Under high concurrency, this could lead to resource exhaustion or lock contention at the filesystem level.
+
+**Remediation:** Consider implementing a connection pool or an asyncio Lock to serialize write operations:
+```python
+import asyncio
+_DB_WRITE_LOCK = asyncio.Lock()
+
+# In state-changing endpoints:
+async with _DB_WRITE_LOCK:
+    election.open(pdb)
+```
+
+---
+
+### Security Controls Inventory for 15.4.1
+
+| Control | Location | Coverage Status |
+|---------|----------|-----------------|
+| Per-request DB connections | `Election.__init__` | ✅ Avoids shared connection objects |
+| SQLite transactions | `delete()`, `add_salts()` | ⚠️ No rollback on error |
+| Thread-safe types (Lock/Semaphore) | N/A | ❌ NOT present |
+| Connection pooling | N/A | ❌ NOT present |
+| `PRAGMA busy_timeout` | N/A | ❌ NOT configured |
+| asyncio.Lock for writes | N/A | ❌ NOT present |
+
+### Positive Patterns for 15.4.1
+
+1. **No shared mutable state**: Each request creates its own `Election` instance with its own database connection, preventing direct object sharing between concurrent handlers.
+2. **Transaction boundaries**: Critical multi-step operations (`delete`, `add_salts`) use explicit `BEGIN TRANSACTION`/`COMMIT` to ensure atomicity at the database level.
+3. **Instance isolation**: After `delete()`, the instance is disabled (`self.db = None`), preventing stale state reuse.
+
+---
+
+
+
+## Cross-cutting Architecture Observations
+
+1. **Python + Async + SQLite Architecture**: The combination of Quart (async framework) with SQLite (file-based database) creates an inherent tension. SQLite's file-level locking doesn't align well with concurrent async request handling. While the single-threaded nature of asyncio mitigates threading issues, the lack of connection pooling and proper busy-timeout handling means production deployments could experience contention.
+
+2. **Assertion-based validation**: The codebase extensively uses `assert` statements for state validation (`assert self.is_editable()`). In production Python (`-O` flag), assertions are stripped entirely, removing all state guards. This is a systemic architectural risk that spans multiple ASVS requirements.
+
+3. **EasyDict usage throughout**: The pervasive use of `EasyDict` means type information is lost at runtime. While convenient, it makes it impossible to enforce schemas on data structures, creating a gap for type safety (15.3.5) and mass assignment concerns.
+
+4. **Separation of concerns**: The `Election` class handles both data access and business logic. The web layer (`pages.py`) directly instantiates `Election` objects rather than going through a service layer, which makes it harder to apply cross-cutting concerns like atomic transactions spanning business operations.
+
+5. **Authorization gap pattern**: The `### check authz` placeholders throughout `pages.py` indicate a systematic Type B gap — the authentication framework exists and is applied, but fine-grained authorization (ownership verification) is consistently missing.
+
+---
+
+## Cross-cutting Recommendations
+
+### Immediate (Critical/High Priority)
+
+1. **Replace assertions with proper validation**: Convert all `assert self.is_editable()` patterns to explicit checks that raise proper exceptions and cannot be disabled with `-O`:
+   ```python
+   if not self.is_editable():
+       raise ElectionBadState(self.eid, self.get_state(), self.S_EDITABLE)
+   ```
+
+2. **Add transaction error handling**: Wrap all `BEGIN TRANSACTION`/`COMMIT` blocks in try/except with explicit `ROLLBACK` on failure to prevent database lock leaks.
+
+3. **Make election state transitions atomic**: Combine state checks and state changes into single transactions using `BEGIN IMMEDIATE` to prevent TOCTOU races on open/close operations.
+
+### Short-term (Medium Priority)
+
+4. **Configure SQLite busy timeout**: Add `PRAGMA busy_timeout = 5000` on every new connection to handle concurrent access gracefully.
+
+5. **Add type validation for JSON inputs**: Verify that `get_json()` returns a dict and that expected fields have correct types before processing.
+
+6. **Implement duplicate parameter detection**: Check for multi-valued form parameters in vote submission and other form-processing endpoints.
+
+7. **Unify transaction patterns**: Adopt a consistent approach (e.g., context manager) for all database write operations.
+
+### Long-term (Architectural)
+
+8. **Consider database migration**: For production use with concurrent access, consider migrating from SQLite to PostgreSQL, which has proper row-level locking and better concurrency support.
+
+9. **Add application-level write serialization**: Implement an asyncio Lock for state-changing operations on the same election to prevent concurrent modification.
+
+10. **Implement a service layer**: Separate business logic from data access to centralize transaction management and make it easier to apply atomic operation patterns consistently.
+        
\ 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