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 08:41:08 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 2.1.3 (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 4b58deb ASVS L3 audit: 2.1.3 (input_validation_and_injection)
4b58deb is described below
commit 4b58deba1c0b8b3374b43c3145ecdc1c104e0a18
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 10:41:03 2026 +0200
ASVS L3 audit: 2.1.3 (input_validation_and_injection)
---
.../input_validation_and_injection/2.1.3.md | 829 +++++++++++++++++++++
1 file changed, 829 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/2.1.3.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/2.1.3.md
new file mode 100644
index 0000000..2e6cd68
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/input_validation_and_injection/2.1.3.md
@@ -0,0 +1,829 @@
+# ASVS 2.1.3 Security Audit Report
+## Apache STeVe v3 Election System
+
+---
+
+## Executive Summary
+
+**Repository:** apache/steve
+**Audit Date:** Apr 22, 2026
+**Auditor:** Tooling Agents
+**ASVS Requirement:** 2.1.3 — Verify that expectations for business logic limits and validations are documented, including both per-user and globally across the application.
+**Level:** 2
+**Section:** Validation and Business Logic Documentation
+**Chapter:** Validation and Business Logic
+
+**Files Analyzed:** 23 relevant out of 23 total, 0 skipped
+
+### Finding Summary
+
+| Severity | Count | Status |
+|----------|-------|--------|
+| Critical | 1 | ❌ Missing authorization on all election management endpoints |
+| High | 2 | ❌ Missing vote validation, unsafe HTTP methods |
+| Medium | 3 | ⚠️ No resource limits, assertion-based state machine, path validation gaps |
+| Low | 0 | — |
+| **Total** | **6** | **Requires immediate remediation** |
+
+**Key Findings:**
+- **Type B Gaps Identified:** 27+ `### check authz` comments document intended authorization controls that were never implemented
+- **Critical Authorization Bypass:** Any authenticated committer can open, close, modify, or delete any election in the system
+- **Vote Integrity Risk:** No validation of vote content against issue type (YNA vs STV)
+- **State Machine Fragility:** Election lifecycle enforced via Python assertions that disappear in optimized deployments
+
+---
+
+## Security Controls Inventory
+
+| Control | Defined Where | Applied Where | Coverage |
+|---------|---------------|---------------|----------|
+| Authentication (committer) | `pages.py` decorators | All voting/admin routes | ✅ Complete |
+| Authentication (pmc_member) | `pages.py:466` | `/do-create-election` only | ✅ Complete |
+| Election ownership authorization | `pages.py` (`### check authz` x14) | **NOWHERE** | ❌ **Type B Gap** |
+| Vote format validation | `election.py:261` comment | **NOWHERE** | ❌ **Type B Gap** |
+| CSRF token | Templates (all forms) | `basic_info()` placeholder | ❌ **Type B Gap** |
+| Election state machine | `election.py` (assertions) | All state-changing operations | ⚠️ Fragile (assertions) |
+| Voter eligibility check | `election.py:265` (`q_get_mayvote`) | `add_vote()` (implicit) | ⚠️ Partial |
+| Parameterized queries | `queries.yaml` | All DB operations | ✅ Complete |
+| EID/IID validation | `election.py` constructors | `load_election`/`load_election_issue` | ✅ Complete |
+| Resource creation limits | **NOT DEFINED** | **NOT APPLIED** | ❌ **Type A Gap** |
+| Vote revision limits | **NOT DEFINED** | **NOT APPLIED** | ❌ **Type A Gap** |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py` — Route Handlers
+
+| Function | Auth Check | Authz Check | Input Validation | State Check | Rating |
+|----------|------------|-------------|------------------|-------------|--------|
+| `home_page` | None (public) | N/A | N/A | N/A | ✅ |
+| `voter_page` | ✅ committer | Implicit (query) | N/A | N/A | ✅ |
+| `vote_on_page` | ✅ committer | ✅ mayvote | N/A | N/A | ✅ |
+| `admin_page` | ✅ committer | ✅ owned query | N/A | N/A | ✅ |
+| `manage_page` | ✅ committer | ❌ **MISSING** | N/A | N/A | ❌ |
+| `do_open_endpoint` | ✅ committer | ❌ **MISSING** | N/A | ⚠️ assert | ❌ |
+| `do_close_endpoint` | ✅ committer | ❌ **MISSING** | N/A | ⚠️ assert | ❌ |
+| `do_vote_endpoint` | ✅ committer | ⚠️ implicit | ❌ **MISSING** | ✅ required_state | ⚠️ |
+| `do_create_endpoint` | ✅ pmc_member | N/A | ❌ no limits | N/A | ⚠️ |
+| `do_add_issue_endpoint` | ✅ committer | ❌ **MISSING** | ❌ no limits | ⚠️ assert | ❌ |
+| `do_edit_issue_endpoint` | ✅ committer | ❌ **MISSING** | ❌ no limits | ⚠️ assert | ❌ |
+| `do_delete_issue_endpoint` | ✅ committer | ❌ **MISSING** | N/A | ⚠️ assert | ❌ |
+| `_set_election_date` | ✅ committer | ❌ **MISSING** | ✅ date format | N/A | ⚠️ |
+| `serve_doc` | ✅ session | ✅ mayvote | ❌ **MISSING** | N/A | ⚠️ |
+
+### `v3/steve/election.py` — Business Logic
+
+| Method | State Guard | Input Validation | Limit Enforcement | Rating |
+|--------|-------------|------------------|-------------------|--------|
+| `create()` | N/A | ❌ no title/authz limits | ❌ no per-user limit | ⚠️ |
+| `open()` | ⚠️ assert | N/A | N/A | ⚠️ |
+| `close()` | ⚠️ assert | N/A | N/A | ⚠️ |
+| `add_issue()` | ⚠️ assert | ⚠️ assert vtype | ❌ no per-election limit | ⚠️ |
+| `add_vote()` | ✅ required_state | ❌ **MISSING** votestring | ❌ no revision limit | ❌ |
+| `add_voter()` | ⚠️ assert | N/A | N/A | ⚠️ |
+
+### `v3/queries.yaml` — SQL Queries
+
+| Query | Parameterized | Notes |
+|-------|---------------|-------|
+| All `c_*` commands | ✅ `?` placeholders | Proper parameter binding |
+| All `q_*` queries | ✅ `?` placeholders | Proper parameter binding |
+
+---
+
+## Findings
+
+### ASVS-213-CRITICAL-001 — Missing Authorization Checks on All Election Management Endpoints
+
+**Severity:** CRITICAL
+**Gap Type:** Type B (Control EXISTS but NOT CALLED)
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Functions:**
+ - `load_election()` (line ~193)
+ - `load_election_issue()` (line ~218)
+ - `_set_election_date()` (line ~81)
+ - `do_open_endpoint()` (line ~489)
+ - `do_close_endpoint()` (line ~508)
+ - `do_add_issue_endpoint()` (line ~526)
+ - `do_edit_issue_endpoint()` (line ~550)
+ - `do_delete_issue_endpoint()` (line ~572)
+ - `do_vote_endpoint()` (line ~425)
+
+**Related Functions Checked:**
+- `basic_info()` — provides authenticated user ID but no authorization enforcement
+- `Election.get_metadata()` — returns election data including `owner_pid` and `authz` fields, but these are never validated against current user
+
+**Description:**
+
+The codebase contains 14+ instances of `### check authz` comments indicating developer awareness of the need for authorization checks, but these checks were never implemented. Any authenticated ASF committer can manage, open, close, or modify any election regardless of ownership.
+
+**Vulnerable Code:**
+
+In `load_election` decorator:
+```python
+async def loader(eid):
+ try:
+ e = steve.election.Election(DB_FNAME, eid)
+ except steve.election.ElectionNotFound:
+ ...
+ _LOGGER.debug(f'Loaded: {e}')
+ ### check authz # <-- INTENDED but NOT IMPLEMENTED
+ return await func(e)
+```
+
+In `do_open_endpoint`:
+```python
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+ ### check authz # <-- INTENDED but NOT IMPLEMENTED
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+ election.open(pdb) # <-- Irreversible operation proceeds without authz
+```
+
+In `do_vote_endpoint`:
+```python
+@APP.post('/do-vote/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_vote_endpoint(election):
+ result = await basic_info()
+ ### check authz # <-- INTENDED but NOT IMPLEMENTED
+ form = edict(await quart.request.form)
+ ...
+ election.add_vote(result.uid, iid, votestring)
+```
+
+**Data Flow:**
+```
+Authenticated committer
+→ /manage/<eid>, /do-open/<eid>, /do-close/<eid>, /do-add-issue/<eid>, etc.
+→ Missing ownership/role check
+→ Election state modified
+```
+
+**Proof of Concept:**
+
+Opening any election:
+```http
+GET /do-open/TARGET_ELECTION_EID HTTP/1.1
+Host: steve.example.org
+Cookie: session=<any_valid_committer_session>
+```
+
+Deleting issues from any election:
+```http
+POST /do-delete-issue/TARGET_EID/TARGET_IID HTTP/1.1
+Host: steve.example.org
+Cookie: session=<any_valid_committer_session>
+Content-Type: application/x-www-form-urlencoded
+
+csrf_token=placeholder
+```
+
+**Impact:**
+- Any authenticated ASF committer can open, close, or modify any election in the system
+- Opening an election is irreversible (generates cryptographic keys)
+- Closing an election permanently ends voting
+- Issues can be added, edited, or deleted from elections the user doesn't own
+- Election dates can be modified by any committer
+- This undermines the entire election integrity model
+
+**Remediation:**
+
+```python
+def load_election(func):
+ @functools.wraps(func)
+ async def loader(eid):
+ try:
+ e = steve.election.Election(DB_FNAME, eid)
+ except steve.election.ElectionNotFound:
+ ...
+
+ result = await basic_info()
+ md = e.get_metadata()
+
+ # Verify the authenticated user is the election owner
+ # or has appropriate authorization (authz group membership)
+ if md.owner_pid != result.uid:
+ # TODO: also check authz group membership
+ quart.abort(403, 'Not authorized to manage this election')
+
+ return await func(e)
+ return loader
+```
+
+---
+
+### ASVS-213-HIGH-002 — Missing Vote Content Validation Against Issue Type
+
+**Severity:** HIGH
+**Gap Type:** Type B (Control EXISTS but NOT CALLED)
+
+**Location:**
+- **File:** `v3/steve/election.py`
+- **Function:** `add_vote()` (line ~256)
+
+**Related Functions Checked:**
+- `vtypes` module — contains validation logic for YNA and STV vote types, but never invoked
+- `q_get_issue` — retrieves issue metadata including type, but not used for validation
+- `do_vote_endpoint()` in `pages.py` — passes raw form data to `add_vote()` without validation
+
+**Description:**
+
+Vote content is never validated against the issue's vote type (YNA or STV). A comment at line 261 explicitly documents the need for validation, but it was never implemented. Arbitrary strings can be stored as votes.
+
+**Vulnerable Code:**
+
+```python
+def add_vote(self, pid: str, iid: str, votestring: str):
+ "Add VOTESTRING as the (latest) vote by PID for IID."
+
+ # The Election should be open.
+ md = self._all_metadata(self.S_OPEN)
+
+ ### validate VOTESTRING for ISSUE.TYPE voting # <-- INTENDED but NOT IMPLEMENTED
+
+ mayvote = self.q_get_mayvote.first_row(pid, iid)
+ vote_token = crypto.gen_vote_token(md.opened_key, pid, iid, mayvote.salt)
+
+ # Pass MAYVOTE.SALT for PBKDF.
+ ciphertext = crypto.create_vote(vote_token, mayvote.salt, votestring)
+
+ self.c_add_vote.perform(vote_token, ciphertext)
+```
+
+**Data Flow:**
+```
+User form submission
+→ do_vote_endpoint()
+→ election.add_vote(uid, iid, votestring)
+→ Missing validation against issue.type
+→ Arbitrary string encrypted and stored
+```
+
+**Proof of Concept:**
+
+For a YNA (yes/no/abstain) issue:
+```http
+POST /do-vote/ELECTION_EID HTTP/1.1
+Host: steve.example.org
+Cookie: session=<valid_session>
+Content-Type: application/x-www-form-urlencoded
+
+vote-ISSUE_IID=MALICIOUS_GARBAGE_DATA_OR_VERY_LONG_STRING
+```
+
+For an STV (ranked choice) issue:
+```http
+POST /do-vote/ELECTION_EID HTTP/1.1
+Content-Type: application/x-www-form-urlencoded
+
+vote-ISSUE_IID=invalid_candidate,nonexistent_label,<script>alert(1)</script>
+```
+
+**Impact:**
+- Arbitrary strings can be stored as votes for both YNA and STV issues
+- During tallying, `vtypes` module may encounter unexpected input, causing errors or incorrect results
+- For STV issues, invalid candidate labels could corrupt ranking calculations
+- For YNA issues, values other than y/n/a bypass the intended three-choice constraint
+- Large votestrings could bloat the database (no length limit)
+
+**Remediation:**
+
+```python
+def add_vote(self, pid: str, iid: str, votestring: str):
+ md = self._all_metadata(self.S_OPEN)
+
+ # Validate votestring against issue type
+ issue = self.q_get_issue.first_row(iid)
+ if not issue:
+ raise IssueNotFound(iid)
+
+ vtype_mod = vtypes.vtype_module(issue.type)
+ if not vtype_mod.validate(votestring, self.json2kv(issue.kv)):
+ raise ValueError(f'Invalid vote format for {issue.type}: {votestring!r}')
+
+ mayvote = self.q_get_mayvote.first_row(pid, iid)
+ if not mayvote:
+ raise ValueError(f'Person {pid} is not authorized to vote on issue {iid}')
+ ...
+```
+
+---
+
+### ASVS-213-HIGH-003 — Irreversible State-Changing Operations via GET Method
+
+**Severity:** HIGH
+**Gap Type:** Type A (Entry point with no control)
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Functions:**
+ - `do_open_endpoint()` (line ~485)
+ - `do_close_endpoint()` (line ~505)
+
+**Related Functions Checked:**
+- Template JavaScript in `manage.ezt` and `manage-stv.ezt` — triggers state changes via `window.location.href`
+- `election.open()` — irreversibly generates cryptographic keys
+- `election.close()` — permanently ends voting period
+
+**Description:**
+
+Critical state-changing operations (opening and closing elections) use GET requests, making them vulnerable to CSRF via image tags, link prefetching, or any mechanism that triggers GET requests.
+
+**Vulnerable Code:**
+
+```python
+@APP.get('/do-open/<eid>') # <-- GET for irreversible state change
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ ...
+ election.open(pdb) # Irreversible: generates crypto keys, sets salts
+
+@APP.get('/do-close/<eid>') # <-- GET for irreversible state change
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_close_endpoint(election):
+ ...
+ election.close() # Irreversible: permanently ends voting
+```
+
+Template confirmation triggers via simple navigation:
+```javascript
+// manage.ezt and manage-stv.ezt
+confirmOpen.addEventListener('click', () => {
+ openModal.hide();
+ window.location.href = '/do-open/[eid]'; // GET request
+});
+confirmClose.addEventListener('click', () => {
+ closeModal.hide();
+ window.location.href = '/do-close/[eid]'; // GET request
+});
+```
+
+**Data Flow:**
+```
+Browser navigation / link prefetch / img src
+→ GET /do-open/<eid>
+→ election.open()
+→ Election irreversibly opened
+```
+
+**Proof of Concept:**
+
+Embedding in any page viewed by an authenticated committer:
+```html
+<img src="https://steve.example.org/do-close/TARGET_EID" style="display:none">
+```
+
+Or via email with an HTML image:
+```html
+<img src="https://steve.example.org/do-open/TARGET_EID" width="1" height="1">
+```
+
+Browser prefetching or link preview features could also trigger these actions.
+
+**Impact:**
+- Elections can be opened or closed by any mechanism that triggers a GET request
+- Opening is irreversible: generates cryptographic keys, sets per-voter salts
+- Closing permanently ends voting with no undo
+- Combined with ASVS-213-CRITICAL-001 (no ownership check), this amplifies the attack surface
+- CSRF attacks become trivial (no token needed for GET requests)
+
+**Remediation:**
+
+```python
+@APP.post('/do-open/<eid>') # Change to POST
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ # Validate CSRF token
+ form = await quart.request.form
+ if not validate_csrf_token(form.get('csrf_token')):
+ quart.abort(403, 'Invalid CSRF token')
+ ...
+```
+
+Update templates to use forms instead of `window.location.href`:
+```javascript
+confirmOpen.addEventListener('click', () => {
+ const form = document.createElement('form');
+ form.method = 'POST';
+ form.action = '/do-open/[eid]';
+ const csrf = document.createElement('input');
+ csrf.type = 'hidden';
+ csrf.name = 'csrf_token';
+ csrf.value = csrfToken;
+ form.appendChild(csrf);
+ document.body.appendChild(form);
+ form.submit();
+});
+```
+
+---
+
+### ASVS-213-MEDIUM-004 — No Business Logic Limits on Resource Creation or Vote Revisions
+
+**Severity:** MEDIUM
+**Gap Type:** Type A (Entry point with no control)
+
+**Location:**
+- **File:** `v3/server/pages.py` and `v3/steve/election.py`
+- **Functions:**
+ - `do_create_endpoint()` (line ~466)
+ - `do_add_issue_endpoint()` (line ~522)
+ - `add_vote()` in election.py (line ~256)
+
+**Related Functions Checked:**
+- `Election.create()` — no per-user limit on election creation
+- `election.add_issue()` — no per-election limit on issue count
+- `c_add_vote` query — always INSERTs, never UPDATEs, creating unbounded rows
+
+**Description:**
+
+No business logic limits are defined or enforced for resource creation (elections, issues) or vote revisions. The vote storage model uses INSERT for every revision, allowing unbounded database growth.
+
+**Vulnerable Code — Election Creation (no per-user limit):**
+
+```python
+@APP.post('/do-create-election')
+@asfquart.auth.require({R.pmc_member})
+async def do_create_endpoint():
+ result = await basic_info()
+ form = edict(await quart.request.form)
+ election = steve.election.Election.create(DB_FNAME, form.title, result.uid)
+ # No limit on how many elections a user can create
+```
+
+**Vulnerable Code — Issue Addition (no per-election limit):**
+
+```python
+@APP.post('/do-add-issue/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_add_issue_endpoint(election):
+ ...
+ iid = election.add_issue(form.title, form.description, vtype, kv)
+ # No limit on number of issues per election
+```
+
+**Vulnerable Code — Vote Revisions (no per-user limit):**
+
+```python
+# election.py - add_vote creates a new row each time
+def add_vote(self, pid, iid, votestring):
+ ...
+ self.c_add_vote.perform(vote_token, ciphertext)
+ # INSERT creates new row; no limit on revisions
+```
+
+```sql
+# queries.yaml
+c_add_vote: |
+ INSERT INTO vote (vote_token, ciphertext)
+ VALUES (?, ?)
+ # Always inserts, never updates — unlimited rows per voter per issue
+```
+
+**Data Flow:**
+```
+Authenticated user
+→ Repeated POST requests
+→ No per-user or global limits
+→ Unbounded resource consumption
+```
+
+**Proof of Concept:**
+
+Creating thousands of elections:
+```bash
+for i in $(seq 1 10000); do
+ curl -X POST https://steve.example.org/do-create-election \
+ -H "Cookie: session=VALID_SESSION" \
+ -d "title=Flood+Election+$i&csrf_token=placeholder"
+done
+```
+
+Submitting thousands of vote revisions:
+```bash
+for i in $(seq 1 100000); do
+ curl -X POST https://steve.example.org/do-vote/ELECTION_EID \
+ -H "Cookie: session=VALID_SESSION" \
+ -d "vote-ISSUE_IID=y&csrf_token=placeholder"
+done
+```
+
+**Impact:**
+- Database can be flooded with unlimited elections, issues, or vote records
+- Vote table grows unboundedly since each revision adds a new row (INSERT not UPDATE)
+- No input length limits on title or description fields could cause storage issues
+- Tallying performance degrades with excessive vote revisions per voter
+- Violates ASVS principle: "Business logic includes limits and controls to detect and prevent automated attacks"
+
+**Remediation:**
+
+```python
+# Add limits as constants (document in design specs)
+MAX_ELECTIONS_PER_USER = 50
+MAX_ISSUES_PER_ELECTION = 100
+MAX_VOTE_REVISIONS_PER_ISSUE = 10
+MAX_TITLE_LENGTH = 200
+MAX_DESCRIPTION_LENGTH = 5000
+
+@APP.post('/do-create-election')
+@asfquart.auth.require({R.pmc_member})
+async def do_create_endpoint():
+ result = await basic_info()
+ form = edict(await quart.request.form)
+
+ # Enforce title length
+ if not form.title or len(form.title) > MAX_TITLE_LENGTH:
+ quart.abort(400, f'Title must be 1-{MAX_TITLE_LENGTH} characters')
+
+ # Check per-user election limit
+ owned = steve.election.Election.owned_elections(DB_FNAME, result.uid)
+ if len(owned) >= MAX_ELECTIONS_PER_USER:
+ await flash_danger(f'Cannot create more than {MAX_ELECTIONS_PER_USER} elections.')
+ return quart.redirect('/admin', code=303)
+ ...
+```
+
+---
+
+### ASVS-213-MEDIUM-005 — Election State Machine Enforced via Python Assertions
+
+**Severity:** MEDIUM
+**Gap Type:** Type C (Control CALLED but RESULT can be IGNORED)
+
+**Location:**
+- **File:** `v3/steve/election.py`
+- **Functions:**
+ - `delete()` (line ~48)
+ - `open()` (line ~65)
+ - `close()` (line ~114)
+ - `add_salts()` (line ~123)
+ - `add_issue()` (line ~184)
+ - `edit_issue()` (line ~205)
+ - `delete_issue()` (line ~219)
+ - `add_voter()` (line ~248)
+
+**Related Functions Checked:**
+- `is_editable()` — correctly checks state but only called via assertions
+- `is_open()` — correctly checks state but only called via assertions
+- `get_state()` — returns current state from database
+
+**Description:**
+
+The election state machine (editable → open → closed) is enforced exclusively through Python `assert` statements. When Python is run with optimization (`-O` flag or `PYTHONOPTIMIZE=1`), all assertions are removed, completely disabling state validation.
+
+**Vulnerable Code:**
+
+```python
+def delete(self):
+ assert self.is_editable() # Removed with python -O
+
+def open(self, pdb):
+ assert self.is_editable() # Removed with python -O
+
+def close(self):
+ assert self.is_open() # Removed with python -O
+
+def add_salts(self):
+ assert self.is_editable() # Removed with python -O
+
+def add_issue(self, title, description, vtype, kv):
+ assert self.is_editable() # Removed with python -O
+ assert vtype in vtypes.TYPES # Removed with python -O
+
+def add_voter(self, pid, iid=None):
+ assert self.is_editable() # Removed with python -O
+```
+
+**Data Flow:**
+```
+State-changing request
+→ election.py method
+→ assert self.is_editable()
+→ Compiled with -O: assertion removed
+→ Operation proceeds in wrong state
+```
+
+**Impact:**
+- If the application is deployed with `python -O` or `PYTHONOPTIMIZE=1`, all state machine assertions are silently disabled
+- Issues could be added to open elections, elections could be opened twice, closed elections could be re-closed
+- The entire election lifecycle integrity depends on assertions that are not guaranteed to execute
+- This violates the ASVS principle that "the business logic flow is sequential, processed in order, and cannot be bypassed"
+- Creates deployment-dependent security behavior (works in dev, fails in production)
+
+**Remediation:**
+
+Replace assertions with explicit checks that raise proper exceptions:
+
+```python
+def _require_state(self, required):
+ """Enforce election state as a hard check, not an assertion."""
+ current = self.get_state()
+ if current != required:
+ raise ElectionBadState(self.eid, current, required)
+
+def open(self, pdb):
+ self._require_state(self.S_EDITABLE) # Never optimized away
+ self.add_salts()
+ ...
+
+def close(self):
+ self._require_state(self.S_OPEN)
+ self.c_close.perform(self.eid)
+
+def add_issue(self, title, description, vtype, kv):
+ self._require_state(self.S_EDITABLE)
+ if vtype not in vtypes.TYPES:
+ raise ValueError(f'Invalid vote type: {vtype}')
+ ...
+```
+
+---
+
+### ASVS-213-MEDIUM-006 — Missing Document Name Validation in File Serving Endpoint
+
+**Severity:** MEDIUM
+**Gap Type:** Type B (Control EXISTS but NOT CALLED)
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Function:** `serve_doc()` (line ~613)
+
+**Related Functions Checked:**
+- `quart.send_from_directory()` — provides built-in path traversal protection but doesn't validate filename format
+- `q_get_mayvote` — validates voter authorization but not document name
+
+**Description:**
+
+The file serving endpoint has a developer comment explicitly noting the need to verify document name propriety, but no validation is implemented. While `send_from_directory` prevents path traversal, there's no validation of filename format or filtering of hidden/system files.
+
+**Vulnerable Code:**
+
+```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. # <-- INTENDED but NOT IMPLEMENTED
+
+ # Return per-issue document.
+ return await quart.send_from_directory(DOCSDIR / iid, docname)
+```
+
+**Data Flow:**
+```
+URL parameter docname
+→ serve_doc(iid, docname)
+→ Missing validation
+→ quart.send_from_directory(DOCSDIR / iid, docname)
+```
+
+**Impact:**
+- While `send_from_directory` provides built-in path traversal protection for `docname`, the developer-acknowledged missing validation could interact with edge cases in file naming
+- The `iid` is used as a directory component; although the `q_get_mayvote` check would reject non-existent IIDs, no format validation ensures IIDs don't contain path-significant characters
+- Unexpected filenames (e.g., hidden files, system files, backup files) within the docs directory could be served
+- No protection against serving files with double extensions or unusual formats
+
+**Remediation:**
+
+```python
+import re
+
+# Only allow safe document names
+SAFE_DOCNAME = re.compile(r'^[a-zA-Z0-9][a-zA-Z0-9._-]{0,254}$')
+SAFE_IID = re.compile(r'^[a-zA-Z0-9_-]+$')
+
+@APP.get('/docs/<iid>/<docname>')
+@asfquart.auth.require
+async def serve_doc(iid, docname):
+ result = await basic_info()
+
+ # Validate IID and docname format
+ if not SAFE_IID.match(iid) or not SAFE_DOCNAME.match(docname):
+ quart.abort(404)
+
+ # Reject hidden files and double extensions
+ if docname.startswith('.') or '..' in docname:
+ quart.abort(404)
+
+ 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. **Parameterized Queries Throughout**: All SQL in `queries.yaml` uses `?` parameter binding via `asfpy.db`, preventing SQL injection. No string concatenation or f-strings in SQL.
+
+2. **Cryptographic Vote Anonymization**: The vote storage model with per-voter salts, derived vote tokens, and encrypted ciphertext provides strong voter anonymity. Vote decryption requires knowledge of `opened_key` and per-voter salt.
+
+3. **Anti-Tamper Detection**: `is_tampered()` recomputes `opened_key` from all election data (metadata + issues + voters) to detect post-opening modifications.
+
+4. **Sensitive Data Exclusion**: `get_metadata()` and `get_issue()` explicitly exclude `salt` and `opened_key` from returned data. `q_open_to_me` and `q_upcoming_to_me` never select salt columns.
+
+5. **Mayvote-Based Voter Eligibility**: The `q_get_mayvote` check in `add_vote()` provides an implicit (if fragile) authorization layer for vote submission.
+
+6. **Template Output Encoding**: EZT templates use `[format "js,html"]` for values embedded in JavaScript contexts, and EZT's default behavior provides HTML escaping for regular output.
+
+7. **STV Candidate Randomization**: `random.shuffle(issue.candidates)` in `vote_on_page` prevents ballot-order bias.
+
+8. **SRI Hashes**: External JavaScript includes use Subresource Integrity attributes.
+
+---
+
+## Architecture Observations
+
+1. **Authorization Architecture Gap**: The system has a clear authentication layer (`asfquart.auth.require`) but no authorization middleware. The `### check authz` comments appear 14+ times, indicating a planned but unbuilt authorization subsystem. The `authz` field exists in the election schema but is never checked.
+
+2. **State Machine Design**: The election lifecycle (editable → open → closed) is well-designed conceptually but implemented with assertions rather than exception-based checks, making it fragile and deployment-dependent.
+
+3. **Vote Revision Model**: The INSERT-only vote model (every revision creates a new row, latest is queried) is functionally correct but lacks bounds, creating a potential storage amplification vector.
+
+4. **LDAP Integration**: `asf-load-ldap.py` uses hardcoded LDAP filter `uid=*` with credentials from file — appropriate for a batch import script. No user-controllable input reaches the LDAP query.
+
+5. **Type B Gap Pattern**: The codebase exhibits a consistent pattern of documented security controls (via comments) that were never implemented, creating false confidence in the security posture during code review.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (1-2 weeks)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| **1** | ASVS-213-CRITICAL-001: Implement election ownership authorization checks | Medium | Prevents any committer from managing any election |
+| **2** | ASVS-213-HIGH-003: Change /do-open and /do-close to POST method | Low | Prevents link/image-triggered state changes |
+
+### Short-term Actions (1-2 months)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| **3** | ASVS-213-HIGH-002: Implement vote format validation in `add_vote()` | Medium | Ensures vote integrity and correct tallying |
+| **4** | ASVS-213-MEDIUM-005: Replace assertions with exception-based state checks | Low | Makes state machine deployment-proof |
+
+### Long-term Actions (3-6 months)
+
+| Priority | Finding | Effort | Impact |
+|----------|---------|--------|--------|
+| **5** | ASVS-213-MEDIUM-004: Add resource creation limits | Medium | Prevents abuse and database bloat |
+| **6** | ASVS-213-MEDIUM-006: Add document name validation | Low | Defense-in-depth for file serving |
+| **7** | Document all business logic limits in formal specification | High | Satisfies ASVS 2.1.3 documentation requirement |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand full file list (23 files)</summary>
+
+- `v3/queries.yaml` (relevance: 10)
+- `v3/server/api.py` (relevance: 10)
+- `v3/server/bin/asf-load-ldap.py` (relevance: 10)
+- `v3/server/bin/create-election.py` (relevance: 10)
+- `v3/server/pages.py` (relevance: 10)
+- `v3/server/templates/about.ezt` (relevance: 10)
+- `v3/server/templates/admin.ezt` (relevance: 10)
+- `v3/server/templates/e_bad_eid.ezt` (relevance: 10)
+- `v3/server/templates/e_bad_iid.ezt` (relevance: 10)
+- `v3/server/templates/e_bad_pid.ezt` (relevance: 10)
+- `v3/server/templates/flashes.ezt` (relevance: 10)
+- `v3/server/templates/footer.ezt` (relevance: 10)
+- `v3/server/templates/header.ezt` (relevance: 10)
+- `v3/server/templates/home.ezt` (relevance: 10)
+- `v3/server/templates/manage-stv.ezt` (relevance: 10)
+- `v3/server/templates/manage.ezt` (relevance: 10)
+- `v3/server/templates/privacy.ezt` (relevance: 10)
+- `v3/server/templates/profile.ezt` (relevance: 10)
+- `v3/server/templates/settings.ezt` (relevance: 10)
+- `v3/server/templates/vote-on.ezt` (relevance: 10)
+- `v3/server/templates/voter.ezt` (relevance: 10)
+- `v3/steve/election.py` (relevance: 10)
+- `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