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 17:45:22 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 8.3.1 (ch08_general)
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 5e9725b ASVS L3 audit: 8.3.1 (ch08_general)
5e9725b is described below
commit 5e9725b189e5865ddf28cbf29e5a63221af6bded
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 19:45:04 2026 +0200
ASVS L3 audit: 8.3.1 (ch08_general)
---
.../reports/steve/v3/d0aa7e9/ch08_general/8.3.1.md | 801 +++++++++++++++++++++
1 file changed, 801 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/ch08_general/8.3.1.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/ch08_general/8.3.1.md
new file mode 100644
index 0000000..e246ef5
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/ch08_general/8.3.1.md
@@ -0,0 +1,801 @@
+# ASVS 8.3.1 Security Audit Report
+## Apache STeVe — Operation-Level Authorization Analysis
+
+---
+
+## Executive Summary
+
+- **Repository:** apache/steve
+- **Audit Date:** Apr 22, 2026
+- **Auditor:** Tooling Agents
+- **ASVS Requirement:** 8.3.1 — Verify that the application enforces authorization rules at a trusted service layer and doesn't rely on controls that an untrusted consumer could manipulate, such as client-side JavaScript.
+ - **Level:** 1
+ - **Section:** Operation Level Authorization
+ - **Section Description:** The immediate application of authorization changes in the appropriate tier of an application's architecture is crucial to preventing unauthorized actions, especially in dynamic environments.
+ - **Chapter:** Authorization
+- **Files Analyzed:** 8 relevant out of 49 total, 12 skipped
+
+### Finding Counts
+
+| Severity | Count | IDs |
+|----------|-------|-----|
+| **Critical** | 1 | ASVS-831-CRIT-001 |
+| **High** | 1 | ASVS-831-HIGH-001 |
+| **Medium** | 1 | ASVS-831-MED-001 |
+| **Low** | 0 | — |
+| **Total** | 3 | |
+
+---
+
+## Security Controls Inventory
+
+### Defined Authorization Controls
+
+| Control | Location | Type | Purpose | Coverage Status |
+|---------|----------|------|---------|-----------------|
+| `@asfquart.auth.require({R.committer})` | `pages.py` (multiple) | Authentication + Role | Requires ASF committer role | ✅ Applied to most endpoints |
+| `@asfquart.auth.require({R.pmc_member})` | `pages.py:423` | Authentication + Role | Requires PMC member role | ⚠️ Only on creation endpoint |
+| `@asfquart.auth.require` | `pages.py:549,557,574` | Authentication | Requires valid session | ✅ Applied consistently |
+| `owner_pid` field | `election.py:255` | Data (unused) | Election owner tracked in metadata | ❌ **Never enforced** |
+| `q_get_mayvote` | `election.py:229` | Data-layer authz | Voter eligibility per issue | ✅ Enforced in vote recording |
+| `q_find_issues` | `pages.py:249` | Data-layer authz | Voter eligibility per election | ✅ Enforced in ballot display |
+| `assert self.is_editable()` | `election.py` (multiple) | State guard | Election state enforcement | ⚠️ Uses `assert` (strippable) |
+| `_all_metadata(required_state)` | `election.py:183` | State guard | Exception-based state enforcement | ✅ Proper implementation (2 uses) |
+
+### Coverage Analysis — Entry Points vs. Controls Applied
+
+| Endpoint | Route | Auth | Ownership Check | Voter Eligibility | State Check |
+|----------|-------|------|----------------|-------------------|-------------|
+| `do_open_endpoint` | `GET /do-open/<eid>` | `R.committer` | ❌ **MISSING** | N/A | `assert` only |
+| `do_close_endpoint` | `GET /do-close/<eid>` | `R.committer` | ❌ **MISSING** | N/A | `assert` only |
+| `do_add_issue_endpoint` | `POST /do-add-issue/<eid>` | `R.committer` | ❌ **MISSING** | N/A | `assert` only |
+| `do_edit_issue_endpoint` | `POST /do-edit-issue/<eid>/<iid>` | `R.committer` | ❌ **MISSING** | N/A | `assert` only |
+| `do_delete_issue_endpoint` | `POST /do-delete-issue/<eid>/<iid>` | `R.committer` | ❌ **MISSING** | N/A | `assert` only |
+| `do_set_open_at_endpoint` | `POST /do-set-open_at/<eid>` | `R.committer` | ❌ **MISSING** | N/A | None |
+| `do_set_close_at_endpoint` | `POST /do-set-close_at/<eid>` | `R.committer` | ❌ **MISSING** | N/A | None |
+| `manage_page` | `GET /manage/<eid>` | `R.committer` | ❌ **MISSING** | N/A | N/A |
+| `manage_stv_page` | `GET /manage-stv/<eid>/<iid>` | `R.committer` | ❌ **MISSING** | N/A | N/A |
+| `do_vote_endpoint` | `POST /do-vote/<eid>` | `R.committer` | N/A | ✅ implicit | ✅ `_all_metadata(S_OPEN)` |
+| `vote_on_page` | `GET /vote-on/<eid>` | `R.committer` | N/A | ✅ explicit | N/A |
+| `serve_doc` | `GET /docs/<iid>/<docname>` | session | N/A | ✅ explicit | N/A |
+| `do_create_endpoint` | `POST /do-create-election` | `R.pmc_member` | N/A (creates) | N/A | N/A |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py` — Authorization Coverage Matrix
+
+| Line | Function | `### check authz` Present | Authorization Implemented | Gap Type |
+|------|----------|--------------------------|--------------------------|----------|
+| ~83 | `_set_election_date` | ✅ Yes | ❌ No | **Type B** |
+| ~175 | `load_election` (decorator) | ✅ Yes | ❌ No | **Type B** |
+| ~205 | `load_election_issue` (decorator) | ✅ Yes | ❌ No | **Type B** |
+| ~378 | `do_vote_endpoint` | ✅ Yes | ⚠️ Partial (implicit via `add_vote`) | Minor |
+| ~437 | `do_create_endpoint` | ✅ Yes | ⚠️ N/A (creates, so ownership N/A) | Minor |
+| ~451 | `do_open_endpoint` | ✅ Yes | ❌ No | **Type B** |
+| ~471 | `do_close_endpoint` | ✅ Yes | ❌ No | **Type B** |
+| ~489 | `do_add_issue_endpoint` | ✅ Yes | ❌ No | **Type B** |
+| ~513 | `do_edit_issue_endpoint` | ✅ Yes | ❌ No | **Type B** |
+| ~536 | `do_delete_issue_endpoint` | ✅ Yes | ❌ No | **Type B** |
+
+**Summary:** 10 `### check authz` TODO comments identified. 0 implemented.
+
+### `v3/steve/election.py` — State Enforcement Matrix
+
+| Method | State Check | Mechanism | Strippable by `-O`? | Security Impact |
+|--------|------------|-----------|---------------------|-----------------|
+| `delete` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `open` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `close` | `assert self.is_open()` | `assert` | ⚠️ Yes | State bypass possible |
+| `add_issue` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `edit_issue` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `delete_issue` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `add_voter` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `add_salts` | `assert self.is_editable()` | `assert` | ⚠️ Yes | State bypass possible |
+| `add_vote` | `_all_metadata(self.S_OPEN)` | `raise` | ✅ No | Properly enforced |
+| `tally_issue` | `_all_metadata(self.S_CLOSED)` | `raise` | ✅ No | Properly enforced |
+
+---
+
+## Findings
+
+### ASVS-831-CRIT-001 — Missing Object-Level Authorization on All Election Management Endpoints
+
+**Severity:** CRITICAL
+**Gap Type:** Type B (Control EXISTS but NOT CALLED)
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Affected Functions & Lines:**
+ - `load_election` decorator (line ~161): `### check authz` — never implemented
+ - `load_election_issue` decorator (line ~187): `### check authz` — never implemented
+ - `do_open_endpoint` (line ~445): `### check authz`
+ - `do_close_endpoint` (line ~465): `### check authz`
+ - `do_add_issue_endpoint` (line ~483): `### check authz`
+ - `do_edit_issue_endpoint` (line ~507): `### check authz`
+ - `do_delete_issue_endpoint` (line ~530): `### check authz`
+ - `_set_election_date` (line ~83): `### check authz`
+
+**Related Functions Checked:**
+- `Election.get_metadata()` — confirms `owner_pid` field exists and is populated
+- `Election.owned_elections()` — confirms ownership filtering is implemented for UI display
+- All management endpoint handlers — none perform ownership verification
+
+**Description:**
+
+The application implements authentication via `@asfquart.auth.require({R.committer})` but completely lacks object-level authorization for election management operations. While the system tracks election ownership via the `owner_pid` field in metadata and filters the admin UI to show only owned elections, the underlying API endpoints accept any valid election ID without verifying the current user is the owner.
+
+The codebase contains 10 instances of `### check authz` comments marking locations where authorization checks were planned but never implemented. The `load_election` decorator (the primary entry point for all management operations) explicitly includes this TODO comment at the exact location where ownership verification should occur.
+
+**Vulnerable Code:**
+
+```python
+# v3/server/pages.py, line ~161
+def load_election(func):
+ @functools.wraps(func)
+ async def loader(eid):
+ try:
+ e = steve.election.Election(DB_FNAME, eid)
+ except steve.election.ElectionNotFound:
+ # ... 404 handling ...
+
+ _LOGGER.debug(f'Loaded: {e}')
+
+ ### check authz # <-- PLANNED BUT NEVER IMPLEMENTED
+
+ return await func(e)
+ return loader
+```
+
+**The Control That EXISTS But Is NOT Called:**
+
+```python
+# v3/steve/election.py, line ~255
+def get_metadata(self):
+ md = self._all_metadata()
+ return edict(
+ ...
+ owner_pid=md.owner_pid, # <-- Ownership data EXISTS
+ ...
+ )
+
+# v3/server/pages.py, line ~285 (admin_page)
+owned = steve.election.Election.owned_elections(DB_FNAME, result.uid)
+# Only shows user's own elections in the UI — but API has no such filter
+```
+
+**Data Flow:**
+
+```
+Source: URL path parameter <eid> (user-controllable)
+ ↓
+Load: Election loaded from database (any valid EID succeeds)
+ ↓
+Missing Control: No check that session.uid == election.owner_pid
+ ↓
+Sink: State-changing operations execute for any authenticated committer
+```
+
+**Proof of Concept:**
+
+Alice (PMC member) creates election `abc123`. Bob (committer, not PMC member, not election owner) performs:
+
+```http
+# View another user's election management page
+GET /manage/abc123 HTTP/1.1
+Cookie: session=<bob_session>
+
+→ 200 OK (full management interface rendered)
+
+# Add a malicious issue to Alice's election
+POST /do-add-issue/abc123 HTTP/1.1
+Cookie: session=<bob_session>
+Content-Type: application/x-www-form-urlencoded
+
+title=Tampered+Issue&description=Injected+by+Bob
+
+→ 303 Redirect (issue created successfully)
+
+# Open Alice's election prematurely
+GET /do-open/abc123 HTTP/1.1
+Cookie: session=<bob_session>
+
+→ 303 Redirect (election opened, irreversible)
+
+# Close Alice's election, ending voting
+GET /do-close/abc123 HTTP/1.1
+Cookie: session=<bob_session>
+
+→ 303 Redirect (election closed, irreversible)
+```
+
+**Impact:**
+
+Any authenticated ASF committer can fully control ANY election in the system:
+- Add, edit, or delete issues in elections they don't own
+- Open elections prematurely (before proper configuration)
+- Close elections early (suppressing legitimate votes)
+- Modify scheduled open/close dates
+- Access management interfaces for all elections
+
+This completely undermines election integrity because management authorization relies solely on client-side UI filtering (which elections are displayed), not on server-side enforcement. An attacker who knows valid election IDs can manipulate any election in the system.
+
+**Remediation:**
+
+**Immediate (P0):** Implement ownership verification in the `load_election` decorator at the exact location marked by the `### check authz` comment:
+
+```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()
+ result.title = 'Unknown Election'
+ result.eid = eid
+ raise_404(T_BAD_EID, result)
+
+ return await func(e)
+ return loader
+
+
+def load_election_owned(func):
+ """Decorator that loads an Election AND verifies the current user owns it."""
+ @functools.wraps(func)
+ async def loader(eid):
+ try:
+ e = steve.election.Election(DB_FNAME, eid)
+ except steve.election.ElectionNotFound:
+ result = await basic_info()
+ result.title = 'Unknown Election'
+ result.eid = eid
+ raise_404(T_BAD_EID, result)
+
+ # AUTHORIZATION: verify the current user is the election owner
+ result = await basic_info()
+ metadata = e.get_metadata()
+ if result.uid != metadata.owner_pid:
+ _LOGGER.warning(
+ f'User[U:{result.uid}] denied access to election[E:{eid}]'
+ f' (owner: {metadata.owner_pid})'
+ )
+ quart.abort(403)
+
+ return await func(e)
+ return loader
+```
+
+**Apply to all management endpoints:**
+
+```python
+@APP.post('/do-add-issue/<eid>')
+@asfquart.auth.require({R.pmc_member}) # Match creation privilege
+@load_election_owned # Verify ownership
+async def do_add_issue_endpoint(election):
+ ...
+
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_open_endpoint(election):
+ ...
+
+@APP.get('/do-close/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_close_endpoint(election):
+ ...
+
+@APP.post('/do-edit-issue/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def do_edit_issue_endpoint(election, issue):
+ ...
+
+@APP.post('/do-delete-issue/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def do_delete_issue_endpoint(election, issue):
+ ...
+
+@APP.post('/do-set-open_at/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_set_open_at_endpoint(election):
+ ...
+
+@APP.post('/do-set-close_at/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_set_close_at_endpoint(election):
+ ...
+
+@APP.get('/manage/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def manage_page(election):
+ ...
+
+@APP.get('/manage-stv/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def manage_stv_page(election, issue):
+ ...
+```
+
+---
+
+### ASVS-831-HIGH-001 — Irreversible State Changes via GET Requests with Client-Side-Only Confirmation
+
+**Severity:** HIGH
+**Gap Type:** Type A (Entry point with NO server-side authorization control beyond authentication)
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Functions:**
+ - `do_open_endpoint` (line ~445)
+ - `do_close_endpoint` (line ~465)
+- **Template File:** `v3/server/templates/manage.ezt` (line ~262)
+
+**Related Functions Checked:**
+- `election.open()` — irreversible state transition
+- `election.close()` — irreversible state transition
+- Client-side confirmation modal implementation
+
+**Description:**
+
+Critical state-changing operations (opening and closing elections) are implemented as GET endpoints with only client-side JavaScript confirmation dialogs. The server-side handlers perform no verification beyond authentication, and the use of GET methods means these operations can be triggered via simple URL navigation, image tags, iframe embeds, or browser prefetch mechanisms — completely bypassing the client-side confirmation.
+
+Combined with the missing ownership check (ASVS-831-CRIT-001), this allows any authenticated committer's browser session to be weaponized to open or close any election through cross-site request forgery or social engineering.
+
+**Vulnerable Code:**
+
+```python
+# v3/server/pages.py, line ~445
+@APP.get('/do-open/<eid>') # <-- GET method for state change
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_open_endpoint(election):
+ result = await basic_info()
+ ### check authz # <-- No authorization check
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+ election.open(pdb) # <-- Irreversible operation executes
+ ...
+
+# v3/server/pages.py, line ~465
+@APP.get('/do-close/<eid>') # <-- GET method for state change
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_close_endpoint(election):
+ result = await basic_info()
+ ### check authz # <-- No authorization check
+ election.close() # <-- Irreversible operation executes
+ ...
+```
+
+**Client-side-only control:**
+
+```javascript
+// v3/server/templates/manage.ezt, line ~262
+const confirmOpen = document.getElementById('confirm-open');
+confirmOpen.addEventListener('click', () => {
+ openModal.hide();
+ window.location.href = '/do-open/[eid]'; // Simple GET navigation
+});
+```
+
+**Data Flow:**
+
+```
+Source: GET /do-open/<eid> (triggerable via link, img, iframe, prefetch)
+ ↓
+Authorization: Only @asfquart.auth.require({R.committer}) — no ownership check
+ ↓
+Client-side "barrier": JavaScript confirmation modal (trivially bypassed)
+ ↓
+Sink: election.open() — permanently transitions election to open state
+```
+
+**Proof of Concept:**
+
+An attacker who knows a committer's session is active can trigger open/close via:
+
+```html
+<!-- Embedded in any page the victim committer visits -->
+<img src="https://voting.example.org/do-open/abc123" style="display:none">
+<!-- or -->
+<img src="https://voting.example.org/do-close/abc123" style="display:none">
+
+<!-- Or via link in phishing email -->
+<a href="https://voting.example.org/do-open/abc123">
+ View Election Results
+</a>
+
+<!-- Or via iframe -->
+<iframe src="https://voting.example.org/do-close/abc123"
+ style="display:none"></iframe>
+```
+
+Because these are GET endpoints, browsers will follow the URL automatically when:
+- The user clicks a link
+- An image tag loads
+- An iframe is rendered
+- Browser prefetch is triggered
+- Any JavaScript performs navigation
+
+No form submission, CSRF token, or JavaScript confirmation is involved. The server-side handler executes the irreversible state change without any additional verification.
+
+**Impact:**
+
+Elections can be opened prematurely (before issues are properly configured and voter lists are finalized) or closed prematurely (suppressing legitimate votes and preventing eligible voters from participating), all without any server-side authorization beyond authentication.
+
+The only protection is a client-side JavaScript confirmation dialog, which is completely bypassed when the URL is loaded directly through any of the attack vectors described above.
+
+Combined with ASVS-831-CRIT-001 (no ownership check), this means ANY committer's browser session can be leveraged to open or close ANY election through:
+- Social engineering (clicking a malicious link)
+- Cross-site request forgery (visiting a page with embedded attack code)
+- Browser prefetch mechanisms
+- Email-based attacks
+
+**Remediation:**
+
+**Immediate (P0):** Convert to POST endpoints and add ownership verification:
+
+```python
+@APP.post('/do-open/<eid>') # POST, not GET
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned # Ownership check
+async def do_open_endpoint(election):
+ result = await basic_info()
+ pdb = steve.persondb.PersonDB.open(DB_FNAME)
+ election.open(pdb)
+ return quart.redirect(f'/manage/{election.eid}')
+
+@APP.post('/do-close/<eid>') # POST, not GET
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned # Ownership check
+async def do_close_endpoint(election):
+ result = await basic_info()
+ election.close()
+ return quart.redirect(f'/manage/{election.eid}')
+```
+
+**Update template JavaScript to use form submission:**
+
+```javascript
+// v3/server/templates/manage.ezt
+const confirmOpen = document.getElementById('confirm-open');
+confirmOpen.addEventListener('click', () => {
+ openModal.hide();
+ fetch('/do-open/[eid]', {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/x-www-form-urlencoded',
+ 'X-CSRFToken': csrfToken // When CSRF tokens are implemented
+ },
+ }).then(() => window.location.reload());
+});
+
+const confirmClose = document.getElementById('confirm-close');
+confirmClose.addEventListener('click', () => {
+ closeModal.hide();
+ fetch('/do-close/[eid]', {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/x-www-form-urlencoded',
+ 'X-CSRFToken': csrfToken
+ },
+ }).then(() => window.location.reload());
+});
+```
+
+---
+
+### ASVS-831-MED-001 — Authorization Tier Inconsistency: Lower Privilege Required for Management Than Creation
+
+**Severity:** MEDIUM
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Functions:** All management endpoints vs. `do_create_endpoint`
+
+**Related Functions Checked:**
+- `do_create_endpoint` (line ~423) — requires `R.pmc_member`
+- `do_open_endpoint` (line ~445) — requires only `R.committer`
+- `do_close_endpoint` (line ~465) — requires only `R.committer`
+- `do_add_issue_endpoint` (line ~483) — requires only `R.committer`
+- `do_edit_issue_endpoint` (line ~507) — requires only `R.committer`
+- `do_delete_issue_endpoint` (line ~530) — requires only `R.committer`
+- `do_set_open_at_endpoint` — requires only `R.committer`
+- `do_set_close_at_endpoint` — requires only `R.committer`
+
+**Description:**
+
+The application has an inverted authorization model where creating an election requires higher privileges (`R.pmc_member`) than performing all subsequent management operations (`R.committer`). This means users who lack sufficient privileges to create elections can nonetheless fully manage, modify, open, close, and delete issues from any existing election.
+
+Every management endpoint includes a comment acknowledging this issue: `### need general solution`.
+
+**Vulnerable Code:**
+
+```python
+# Election CREATION requires PMC member (line ~423)
+@APP.post('/do-create-election')
+@asfquart.auth.require({R.pmc_member}) # Higher privilege
+async def do_create_endpoint():
+ ...
+
+# Election MANAGEMENT requires only committer (lines ~445, ~465, ~483, etc.)
+@APP.get('/do-open/<eid>')
+@asfquart.auth.require({R.committer}) # Lower privilege
+@load_election
+async def do_open_endpoint(election):
+ ...
+
+@APP.post('/do-add-issue/<eid>')
+@asfquart.auth.require({R.committer}) # Lower privilege
+@load_election
+async def do_add_issue_endpoint(election):
+ ...
+```
+
+**Data Flow:**
+
+```
+Source: Any committer-level user (lower tier than PMC member)
+ ↓
+Cannot create elections (R.pmc_member required)
+ ↓
+CAN manage/modify/open/close any election (only R.committer required)
+ ↓
+Authorization tier inversion: more sensitive operations require less privilege
+```
+
+**Proof of Concept:**
+
+A committer who is NOT a PMC member:
+
+```http
+# Cannot create:
+POST /do-create-election HTTP/1.1
+Cookie: session=<committer_session>
+
+→ 403 Forbidden (R.pmc_member required)
+
+# But CAN fully manage any existing election:
+POST /do-add-issue/abc123 HTTP/1.1
+Cookie: session=<committer_session>
+Content-Type: application/x-www-form-urlencoded
+
+title=New+Issue&description=Added+by+committer
+
+→ 303 Success (only R.committer needed)
+
+GET /do-open/abc123 HTTP/1.1
+Cookie: session=<committer_session>
+
+→ 303 Success
+
+GET /do-close/abc123 HTTP/1.1
+Cookie: session=<committer_session>
+
+→ 303 Success
+
+POST /do-delete-issue/abc123/issue1 HTTP/1.1
+Cookie: session=<committer_session>
+
+→ 303 Success
+```
+
+**Impact:**
+
+The authorization model is inverted: creation of elections (a lower-impact, reversible operation that simply initializes a new election) requires higher privilege than opening/closing elections and modifying issues (higher-impact, irreversible operations that affect election integrity and voter participation).
+
+A committer who should only have voter-level access can perform all administrative operations on any election. This violates the principle of least privilege and creates an inconsistent security model where the barrier to entry (creation) is higher than the barrier to destructive actions (management).
+
+While this is partially mitigated by the fact that committers are generally trusted community members, it still represents a significant authorization flaw, especially when combined with the missing ownership checks.
+
+**Remediation:**
+
+**Short-term (P1):** Align management endpoint authorization with creation, and add ownership checks:
+
+```python
+# All management endpoints should require at minimum the same role as creation
+@APP.post('/do-add-issue/<eid>')
+@asfquart.auth.require({R.pmc_member}) # Same as creation
+@load_election_owned # Plus ownership verification
+async def do_add_issue_endpoint(election):
+ ...
+
+@APP.post('/do-edit-issue/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def do_edit_issue_endpoint(election, issue):
+ ...
+
+@APP.post('/do-delete-issue/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def do_delete_issue_endpoint(election, issue):
+ ...
+
+@APP.post('/do-open/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_open_endpoint(election):
+ ...
+
+@APP.post('/do-close/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_close_endpoint(election):
+ ...
+
+@APP.post('/do-set-open_at/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_set_open_at_endpoint(election):
+ ...
+
+@APP.post('/do-set-close_at/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def do_set_close_at_endpoint(election):
+ ...
+
+@APP.get('/manage/<eid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+async def manage_page(election):
+ ...
+
+@APP.get('/manage-stv/<eid>/<iid>')
+@asfquart.auth.require({R.pmc_member})
+@load_election_owned
+@load_election_issue
+async def manage_stv_page(election, issue):
+ ...
+```
+
+**Long-term (P2):** Consider implementing a more granular role-based access control system that distinguishes between:
+- Election creators (can create new elections)
+- Election administrators (can manage elections they own)
+- Voters (can view and vote on elections where they're eligible)
+- System administrators (can manage all elections)
+
+---
+
+## Positive Security Patterns
+
+1. **Voter eligibility enforcement on view page** (`vote_on_page`, line ~249): Explicit check using `q_find_issues` before rendering ballot — proper server-side authorization that cannot be bypassed by client manipulation.
+
+2. **Document serving authorization** (`serve_doc`, line ~574): Per-document authorization check via `q_get_mayvote.first_row(result.uid, iid)` before serving files — good defense-in-depth that prevents unauthorized access to election materials.
+
+3. **Vote recording integrity** (`add_vote`, `election.py:226`): The `q_get_mayvote` query acts as a server-side eligibility gate — votes cannot be recorded without a mayvote entry, regardless of client manipulation.
+
+4. **Election tampering detection** (`is_tampered`, `election.py:268`): Integrity verification using Argon2 hash of election data before tallying operations — prevents undetected modification of election results.
+
+5. **Cryptographic vote isolation**: Per-voter salts, vote tokens, and encrypted vote storage prevent correlation even with database access — strong privacy protection for voter anonymity.
+
+6. **SortableJS integrity attribute** (`vote-on.ezt`): Subresource integrity hash on third-party JavaScript dependency — prevents supply chain attacks via CDN compromise.
+
+7. **Principle of least information in API** (`get_metadata`, `election.py:255`): Salt and `opened_key` are excluded from the metadata returned to handlers — reduces information leakage.
+
+8. **Exception-based state enforcement** (`_all_metadata(required_state)`, `election.py:183`): Proper use of exceptions for critical state checks in `add_vote` and `tally_issue` — cannot be stripped by Python optimization flags.
+
+---
+
+## Architecture Observations
+
+### 1. Authorization Model is Authentication-Only
+
+The system currently has a two-tier authentication model (`committer` vs `pmc_member`) but lacks object-level authorization entirely. The codebase explicitly plans for this (`### check authz` appears 10 times) but it was never implemented. This represents a significant gap between design intent and implementation.
+
+### 2. UI Filtering Creates False Confidence
+
+The admin page only shows elections owned by the current user via `owned_elections()`, but the underlying API endpoints accept any valid election ID. This creates an illusion of access control at the presentation layer that does not exist at the API layer — a classic "security by obscurity" pattern where the UI hides functionality that remains accessible via direct API calls.
+
+### 3. State Enforcement Uses `assert` Statements
+
+Seven of nine state-checking call sites use Python `assert` statements, which are stripped by the `-O` optimization flag. Only `add_vote` and `tally_issue` use proper exception-based state checks via `_all_metadata(required_state)`. This means state enforcement could be accidentally disabled in production if Python is run with optimization enabled.
+
+### 4. Planned CSRF Protection Not Implemented
+
+Templates include `csrfToken: 'placeholder'` throughout, indicating CSRF protection was planned but never implemented. Combined with GET-based state changes, this creates multiple attack vectors for unauthorized operations.
+
+### 5. Data Layer Authorization vs. Presentation Layer Authorization
+
+The system demonstrates strong data-layer authorization for voting operations (voter eligibility is enforced at the database query level via `q_get_mayvote` and `q_find_issues`), but completely lacks presentation-layer authorization for management operations. This inconsistency suggests different parts of the codebase were developed with different security models in mind.
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions (P0) — Deploy Within 24 Hours
+
+| Action | Addresses Finding | Effort | Impact |
+|--------|------------------|--------|--------|
+| Implement `load_election_owned` decorator with ownership verification | ASVS-831-CRIT-001 | 2 hours | Blocks unauthorized election management |
+| Convert `/do-open/<eid>` and `/do-close/<eid>` from GET to POST | ASVS-831-HIGH-001 | 1 hour | Prevents CSRF-based election manipulation |
+| Update client-side JavaScript to use POST requests with fetch() | ASVS-831-HIGH-001 | 1 hour | Enforces server-side state change protection |
+
+### Short-Term Actions (P1) — Deploy Within 1 Week
+
+| Action | Addresses | Effort | Impact |
+|--------|-----------|--------|--------|
+| Align all management endpoint role requirements to `R.pmc_member` | ASVS-831-MED-001 | 2 hours | Fixes privilege inversion |
+| Replace all `assert` state checks with exception-based `_all_metadata(required_state)` | Architecture hardening | 4 hours | Prevents optimization-based bypass |
+| Add explicit eligibility check in `do_vote_endpoint` before processing votes | Code clarity | 1 hour | Fail-fast validation |
+| Implement audit logging for all management operations | Defense-in-depth | 4 hours | Detection and forensics |
+
+### Long-Term Actions (P2) — Deploy Within 1 Month
+
+| Action | Addresses | Effort | Impact |
+|--------|-----------|--------|--------|
+| Implement functional CSRF token generation and validation | Defense-in-depth | 8 hours | Comprehensive CSRF protection |
+| Design and implement granular RBAC system | Architecture improvement | 16 hours | Principle of least privilege |
+| Add rate limiting on state-changing endpoints | DoS prevention | 4 hours | Abuse prevention |
+| Implement comprehensive authorization test suite | Quality assurance | 16 hours | Regression prevention |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand full file list (8 files)</summary>
+
+### High Relevance (Score 8-10)
+
+1. **`v3/server/pages.py`** (relevance: 10)
+ - Primary entry point for all HTTP endpoints
+ - Contains all `### check authz` TODO comments
+ - Implements authentication decorators but lacks authorization
+ - 10 security-sensitive functions reviewed
+
+2. **`v3/steve/election.py`** (relevance: 8)
+ - Core election data model and state management
+ - Contains `owner_pid` field (unused for authorization)
+ - Implements state enforcement (mix of `assert` and exceptions)
+ - Contains voter eligibility queries used for authorization
+
+### Medium Relevance (Score 5-7)
+
+3. **`v3/server/templates/manage.ezt`** (relevance: 6)
+ - Management interface template
+ - Contains client-side confirmation dialogs
+ - Demonstrates UI-based access control (insufficient)
+
+4. **`v3/server/templates/manage-stv.ezt`** (relevance: 5)
+ - STV-specific management interface
+ - Similar client-side patterns to manage.ezt
+
+5. **`v3/server/templates/vote-on.ezt`** (relevance: 5)
+ - Voting interface template
+ - Shows proper server-side authorization pattern
+ - Includes SortableJS with SRI
+
+### Low Relevance (Score 4)
+
+6. **`v3/server/bin/tally.py`** (relevance: 4)
+ - Offline tallying script
+ - Uses proper state enforcement (`_all_metadata(S_CLOSED)`)
+
+7. **`v3/server/templates/admin.ezt`** (relevance: 4)
+ - Admin dashboard template
+ - Shows `owned_elections` filtering (UI-only)
+
+8. **`v3/server/templates/voter.ezt`** (relevance: 4)
+ - Voter dashboard template
+ - Shows proper eligibility-based filtering
+
+</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