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 03:18:01 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 2.3.5 (authorization_and_access_control)
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 685f0d2 ASVS L3 audit: 2.3.5 (authorization_and_access_control)
685f0d2 is described below
commit 685f0d2d359cb1bd6038621ccb3a3e81f55a1d4e
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 05:17:57 2026 +0200
ASVS L3 audit: 2.3.5 (authorization_and_access_control)
---
.../authorization_and_access_control/2.3.5.md | 434 +++++++++++++++++++++
1 file changed, 434 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/authorization_and_access_control/2.3.5.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/authorization_and_access_control/2.3.5.md
new file mode 100644
index 0000000..464c56e
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/authorization_and_access_control/2.3.5.md
@@ -0,0 +1,434 @@
+# ASVS 2.3.5 Security Audit Report
+
+## Executive Summary
+
+**Repository:** apache/steve
+
+**Audit Date:** Apr 22, 2026
+
+**Auditor:** Tooling Agents
+
+**ASVS Requirement:** 2.3.5 — Verify that high-value business logic flows require multi-user approval to prevent unauthorized or accidental actions. This could include but is not limited to large monetary transfers, contract approvals, access to classified information, or safety overrides in manufacturing.
+
+- **Level:** 3
+- **Section:** Business Logic Security
+- **Section Description:** This section considers key requirements to ensure that the application enforces business logic processes in the correct way
+
+**Files Analyzed:** 4 relevant out of 4 total, 0 skipped
+
+**Finding Counts:**
+
+| Severity | Count |
+|----------|-------|
+| Critical | 1 |
+| High | 2 |
+| Medium | 0 |
+| Low | 0 |
+| **Total** | **3** |
+
+**Summary:** The election system contains critical and high-severity gaps in authorization enforcement and completely lacks multi-user approval workflows for irreversible, high-value election operations. The database schema defines ownership and authorization fields (`owner_pid`, `authz`) but these are never checked in any web handler — a Type B control gap. Furthermore, no multi-user approval mechanism exists for any election lifecycle operation (opening, closing, deletion), all of whic [...]
+
+---
+
+## Security Controls Inventory
+
+| Control | Defined | Location | Applied At | Coverage |
+|---------|---------|----------|------------|----------|
+| Election ownership (`owner_pid`) | ✅ Schema | `schema.sql:68` | ❌ NEVER checked | **0% - Type B Gap** |
+| Election authz group (`authz`) | ✅ Schema | `schema.sql:73` | ❌ NEVER checked | **0% - Type B Gap** |
+| Election state machine | ✅ Code | `election.py:395-408` | ✅ All state-changing methods | 100% |
+| Voter eligibility (`mayvote`) | ✅ Schema | `schema.sql:147-170` | ✅ `add_vote()`, `vote_on_page()` | ~90% |
+| Anti-tamper (`opened_key`) | ✅ Code | `election.py:359-376` | ⚠️ Available but not called before close/tally in web | Partial |
+| Multi-user approval | ❌ Not defined | N/A | N/A | **0%** |
+| Authentication (`R.committer`) | ✅ Framework | `pages.py` decorators | ✅ All sensitive endpoints | 100% |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py` — Web Handler Layer
+
+| Function | Authorization Check | Owner Verification | Multi-User Approval | Verdict |
+|----------|--------------------|--------------------|---------------------|---------|
+| `do_open_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | **CRITICAL** |
+| `do_close_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | **CRITICAL** |
+| `do_create_endpoint` | `R.pmc_member` ✅ | N/A (creating) | ❌ Missing | MEDIUM |
+| `do_add_issue_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | HIGH |
+| `do_edit_issue_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | HIGH |
+| `do_delete_issue_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | HIGH |
+| `do_vote_endpoint` | `R.committer` ✅ | N/A (voter) | N/A | OK |
+| `do_set_open_at_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | HIGH |
+| `do_set_close_at_endpoint` | `R.committer` ✅ | ❌ Missing | ❌ Missing | HIGH |
+| `manage_page` | `R.committer` ✅ | ❌ Missing | N/A (read) | MEDIUM |
+| `vote_on_page` | `R.committer` ✅ | N/A | N/A | OK (has mayvote check) |
+
+### `v3/steve/election.py` — Business Logic Layer
+
+| Method | State Check | Input Validation | Ownership Check | Multi-User Gate |
+|--------|-------------|------------------|-----------------|-----------------|
+| `open()` | ✅ `assert is_editable()` | ✅ Generates crypto | ❌ None | ❌ None |
+| `close()` | ✅ `assert is_open()` | ✅ State verified | ❌ None | ❌ None |
+| `delete()` | ✅ `assert is_editable()` | ✅ Transactional | ❌ None | ❌ None |
+| `add_issue()` | ✅ `assert is_editable()` | ✅ vtype validated | ❌ None | ❌ None |
+| `add_vote()` | ✅ `_all_metadata(S_OPEN)` | ✅ mayvote checked | N/A (voter) | N/A |
+| `tally_issue()` | ✅ `_all_metadata(S_CLOSED)` | ✅ Crypto verified | ❌ None | ❌ None |
+
+---
+
+## Findings
+
+### Critical Severity
+
+#### ASVS-235-CRT-001: Missing Election Ownership Authorization — Any Committer Can Open/Close/Modify Any Election (Type B Gap)
+
+**Severity:** CRITICAL
+
+**File:** `v3/server/pages.py` (multiple functions) and `v3/steve/election.py`
+
+**Gap Type:** Type B — Control EXISTS (schema fields `owner_pid` and `authz`) but is NOT CALLED at any entry point.
+
+**Control Definition:**
+```sql
+-- v3/schema.sql lines 68-75
+owner_pid TEXT NOT NULL,
+/* What authz group is allowed to edit this election? If NULL,
+ then only the OWNER_PID can edit. */
+authz TEXT,
+```
+
+**Missing Check Locations (9 entry points):**
+
+| Endpoint | File:Line | Comment Found | Actual Check |
+|----------|-----------|---------------|--------------|
+| `do_open_endpoint` | pages.py:479-496 | `### check authz` (line 483) | ❌ None |
+| `do_close_endpoint` | pages.py:499-515 | `### check authz` (line 503) | ❌ None |
+| `do_add_issue_endpoint` | pages.py:518-542 | `### check authz` (line 522) | ❌ None |
+| `do_edit_issue_endpoint` | pages.py:545-568 | `### check authz` (line 549) | ❌ None |
+| `do_delete_issue_endpoint` | pages.py:571-590 | `### check authz` (line 575) | ❌ None |
+| `do_set_open_at_endpoint` | pages.py:398-401 | via `_set_election_date` (line 87) | ❌ None |
+| `do_set_close_at_endpoint` | pages.py:404-407 | via `_set_election_date` (line 87) | ❌ None |
+| `load_election` (decorator) | pages.py:170-193 | `### check authz` (line 185) | ❌ None |
+| `load_election_issue` (decorator) | pages.py:196-227 | `### check authz` (line 215) | ❌ None |
+
+**Vulnerable Code (Opening an election — any committer):**
+```python
+# v3/server/pages.py lines 479-496
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.committer}) ### need general solution
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+
+ ### check authz # <--- NEVER IMPLEMENTED
+
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+
+ # Open the Election — IRREVERSIBLE, generates crypto keys
+ election.open(pdb)
+ ...
+```
+
+**Data Flow:**
+```
+Authenticated committer (any) → GET /do-open/<eid> → load_election(eid)
+→ [NO owner_pid check] → election.open(pdb) → IRREVERSIBLE state change
+```
+
+**Proof of Concept:**
+Any authenticated ASF committer, even one with no relation to the election, can:
+```
+GET /do-open/a1b2c3d4e5 → Opens any editable election
+GET /do-close/a1b2c3d4e5 → Closes any open election
+POST /do-add-issue/a1b2c3d4e5 → Adds issues to any election
+POST /do-delete-issue/a1b2c3d4e5/f6g7h8i9j0 → Deletes issues from any election
+```
+
+**Impact:** Any authenticated committer can irreversibly open or close elections they do not own, add or remove voting issues from others' elections, and effectively sabotage any election in the system. This provides false confidence because the schema documents an ownership model that is never enforced.
+
+**Remediation:**
+```python
+# Add to load_election decorator or create a separate check
+async def verify_election_owner(election, uid):
+ """Verify the current user is the election owner or in the authz group."""
+ md = election.get_metadata()
+ if md.owner_pid == uid:
+ return True
+ if md.authz:
+ # Check LDAP group membership for authz group
+ session = await asfquart.session.read()
+ if md.authz in session.get('groups', []):
+ return True
+ quart.abort(403, 'You are not authorized to manage this election')
+
+# Apply in each management endpoint:
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+ await verify_election_owner(election, result.uid) # ENFORCE
+ ...
+```
+
+---
+
+### High Severity
+
+#### ASVS-235-HGH-001: No Multi-User Approval for Irreversible Election State Transitions
+
+**Severity:** HIGH
+
+**Files:** `v3/server/pages.py` lines 479-515, `v3/steve/election.py` lines 70-120
+
+**Description:** Opening and closing elections are the highest-value operations in this system. Opening an election is explicitly irreversible (generates cryptographic salt and opened_key, sets per-voter salts), and closing permanently terminates voting. Neither operation requires approval from a second authorized user.
+
+**Irreversibility evidence in election.py:**
+```python
+# v3/steve/election.py lines 70-83
+def open(self, pdb):
+ # Double-check the Election is in the editing state.
+ assert self.is_editable()
+
+ # Add salts first — generates per-voter cryptographic material
+ self.add_salts()
+
+ edata = self.gather_election_data(pdb)
+ salt = crypto.gen_salt()
+ opened_key = crypto.gen_opened_key(edata, salt) # Anti-tamper hash
+
+ self.c_open.perform(salt, opened_key, self.eid)
+ # NO UNDO PATH EXISTS — state machine prevents reversal
+```
+
+```python
+# v3/steve/election.py lines 114-120
+def close(self):
+ "Close an election."
+ assert self.is_open()
+ # Simple tweak — but irreversible via state machine
+ self.c_close.perform(self.eid)
+```
+
+**Single-user invocation (pages.py):**
+```python
+# v3/server/pages.py lines 479-496
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+ ### check authz
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+ election.open(pdb) # Single user, immediate execution, no approval
+ ...
+```
+
+**High-Value Operations Without Multi-User Approval:**
+
+| Operation | Endpoint | Reversible | Multi-User Approval |
+|-----------|----------|------------|---------------------|
+| Open election | `GET /do-open/<eid>` | ❌ No | ❌ None |
+| Close election | `GET /do-close/<eid>` | ❌ No | ❌ None |
+| Delete election | `election.delete()` (CLI) | ❌ No | ❌ None |
+| Tally votes | `election.tally_issue()` (CLI) | N/A (read) | ❌ None |
+
+**Impact:** A single user (or an attacker who compromises a single committer account) can unilaterally open an election prematurely (before all issues/voters are configured), close an election early (disenfranchising voters), or trigger tallying. In an election system where integrity and fairness are paramount, the lack of multi-user approval for irreversible operations creates significant risk of both accidental and intentional abuse.
+
+**Proof of Concept:**
+```
+# Attacker with any committer credentials prematurely opens an election
+# that is still being configured, locking it from further edits:
+GET /do-open/a1b2c3d4e5
+
+# Or closes a running election before intended close date:
+GET /do-close/a1b2c3d4e5
+```
+
+**Remediation:**
+```python
+# 1. Add approval tracking to the schema
+# schema.sql addition:
+"""
+CREATE TABLE approval_request (
+ arid INTEGER PRIMARY KEY AUTOINCREMENT,
+ eid TEXT NOT NULL,
+ action TEXT NOT NULL CHECK (action IN ('open', 'close')),
+ requested_by TEXT NOT NULL,
+ requested_at INTEGER NOT NULL,
+ approved_by TEXT,
+ approved_at INTEGER,
+ status TEXT NOT NULL DEFAULT 'pending'
+ CHECK (status IN ('pending', 'approved', 'rejected')),
+ FOREIGN KEY (eid) REFERENCES election(eid),
+ FOREIGN KEY (requested_by) REFERENCES person(pid),
+ FOREIGN KEY (approved_by) REFERENCES person(pid),
+ CHECK (requested_by != approved_by) -- Different users required
+) STRICT;
+"""
+
+# 2. Two-phase endpoint pattern:
+@APP.post('/do-request-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def request_open_endpoint(election):
+ result = await basic_info()
+ await verify_election_owner(election, result.uid)
+ # Record pending approval request
+ create_approval_request(election.eid, 'open', result.uid)
+ await flash_info('Open request submitted. A second authorized user must approve.')
+ return quart.redirect(f'/manage/{election.eid}', code=303)
+
+@APP.post('/do-approve-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def approve_open_endpoint(election):
+ result = await basic_info()
+ await verify_election_owner(election, result.uid)
+ request = get_pending_approval(election.eid, 'open')
+ if request.requested_by == result.uid:
+ quart.abort(403, 'Cannot approve your own request')
+ # Now execute the open
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+ election.open(pdb)
+ mark_approved(request, result.uid)
+ ...
+```
+
+---
+
+#### ASVS-235-HGH-002: Irreversible State-Changing Operations Use GET Method Enabling Accidental Triggering
+
+**Severity:** HIGH
+
+**File:** `v3/server/pages.py` lines 479-515
+
+**Description:** The most consequential operations in the system — opening and closing elections — are implemented as GET endpoints. This violates HTTP semantics (GET should be safe/idempotent) and makes these irreversible operations trivially triggerable by link clicks, browser prefetching, crawlers, or embedded resources, directly contravening ASVS 2.3.5's goal of preventing "unauthorized or accidental actions."
+
+**Vulnerable Code:**
+```python
+# v3/server/pages.py lines 479-480
+@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
+ ...
+
+# v3/server/pages.py lines 499-500
+@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
+ ...
+```
+
+**Attack Vector:**
+```html
+<!-- Embedded in any page visible to an authenticated committer -->
+<img src="https://steve.example.org/do-close/a1b2c3d4e5" width="0" height="0">
+
+<!-- Or link that appears legitimate -->
+<a href="/do-open/a1b2c3d4e5">View election details</a>
+```
+
+**Impact:** Combined with the missing authorization checks (ASVS-235-CRT-001), this means an attacker can embed a URL that will irreversibly open or close any election when visited by any authenticated committer. Even without malicious intent, browser prefetching or link previewing tools could accidentally trigger these operations.
+
+**Remediation:**
+```python
+# Change to POST with confirmation
+@APP.post('/do-open/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+ await verify_election_owner(election, result.uid)
+ # Verify CSRF token
+ form = await quart.request.form
+ if form.get('csrf_token') != result.csrf_token:
+ quart.abort(403)
+ election.open(pdb)
+ ...
+
+@APP.post('/do-close/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_close_endpoint(election):
+ result = await basic_info()
+ await verify_election_owner(election, result.uid)
+ form = await quart.request.form
+ if form.get('csrf_token') != result.csrf_token:
+ quart.abort(403)
+ election.close()
+ ...
+```
+
+---
+
+## Positive Security Patterns
+
+1. **Robust State Machine:** The `_compute_state()` method and assertion-based state checks in `election.py` effectively prevent invalid state transitions (e.g., voting on an editable election, closing an already-closed election).
+
+2. **Anti-Tamper Detection:** The `is_tampered()` method and `opened_key` mechanism provide integrity verification — changes to election data, issues, or voter lists after opening would be detected.
+
+3. **Cryptographic Vote Privacy:** Per-voter salts, encrypted vote storage, and vote shuffling before tallying demonstrate strong consideration for voter anonymity.
+
+4. **Non-Sequential IDs:** 10-character hex IDs with collision retry prevent election/issue enumeration.
+
+5. **Parameterized Queries:** Use of `asfpy.db` with YAML-defined parameterized queries prevents SQL injection.
+
+6. **Voter Eligibility Enforcement:** The `mayvote` table with `(pid, iid)` primary key provides database-level enforcement of voter eligibility, and this is checked in `add_vote()` before accepting votes.
+
+7. **Transactional Operations:** Multi-step destructive operations (`delete()`, `add_salts()`) use explicit `BEGIN TRANSACTION`/`COMMIT`.
+
+---
+
+## Architecture Observations
+
+1. **Two-Layer Authorization Gap:** The system has authentication at the web layer (`asfquart.auth.require`) but zero authorization at either the web layer (pages.py) or business logic layer (election.py). The `owner_pid` and `authz` fields create a documented authorization model that is never implemented.
+
+2. **Insufficient Separation of Privileges:** Election creation requires `R.pmc_member` (elevated privilege), but all subsequent management operations (including opening/closing) only require `R.committer`. This privilege downgrade means the stricter creation check provides no real protection.
+
+3. **CLI Access as Implicit Multi-User Bypass:** The CLI tools (`tally.py`, etc.) have direct database access with no approval workflow, meaning server administrators can unilaterally perform all high-value operations.
+
+4. **State Machine as Partial Control:** The state machine prevents certain classes of invalid operations (e.g., double-opening), but it doesn't address who should be authorized to trigger valid transitions.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (P0)
+
+| Priority | Finding | Remediation | Effort |
+|----------|---------|-------------|--------|
+| **P0** | ASVS-235-CRT-001: Missing ownership authorization | Implement `owner_pid`/`authz` checks at `load_election` decorator and all state-changing endpoints | Medium |
+
+### Short-term Actions (P1-P2)
+
+| Priority | Finding | Remediation | Effort |
+|----------|---------|-------------|--------|
+| **P1** | ASVS-235-HGH-002: GET for state-changing operations | Change `/do-open/` and `/do-close/` to POST with CSRF tokens | Low |
+| **P2** | ASVS-235-HGH-001: No multi-user approval | Implement approval request/confirm workflow for open and close operations | High |
+
+### Long-term Actions (P3-P4)
+
+| Priority | Finding | Remediation | Effort |
+|----------|---------|-------------|--------|
+| **P3** | Implement real CSRF tokens | Replace `basic.csrf_token = 'placeholder'` with actual token generation and validation | Medium |
+| **P4** | Add audit logging for all state changes | Log who performed which operation with full context for forensic review | Low |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list (4 files)</summary>
+
+1. `v3/docs/helpful-sql.md` (relevance: 10)
+2. `v3/schema.sql` (relevance: 10)
+3. `v3/server/pages.py` (relevance: 10)
+4. `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