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 02:02:17 UTC
(tooling-agents) branch main updated: ASVS L3 audit: 15.3.3 (api_and_mass_assignment)
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 2ea5b61 ASVS L3 audit: 15.3.3 (api_and_mass_assignment)
2ea5b61 is described below
commit 2ea5b61f602ffac4fcdeb3dd38bd7fb0596ac28e
Author: ASF Tooling Team <ro...@apache.org>
AuthorDate: Wed Apr 22 04:02:12 2026 +0200
ASVS L3 audit: 15.3.3 (api_and_mass_assignment)
---
.../v3/d0aa7e9/api_and_mass_assignment/15.3.3.md | 362 +++++++++++++++++++++
1 file changed, 362 insertions(+)
diff --git a/repos/apache/ASVS/reports/steve/v3/d0aa7e9/api_and_mass_assignment/15.3.3.md b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/api_and_mass_assignment/15.3.3.md
new file mode 100644
index 0000000..e919aed
--- /dev/null
+++ b/repos/apache/ASVS/reports/steve/v3/d0aa7e9/api_and_mass_assignment/15.3.3.md
@@ -0,0 +1,362 @@
+# ASVS 15.3.3 Mass Assignment Protection Audit Report
+
+## Executive Summary
+
+- **Repository:** apache/steve
+- **Audit Date:** Apr 22, 2026
+- **Auditor:** Tooling Agents
+- **ASVS Requirement:** 15.3.3 — Verify that the application has countermeasures to protect against mass assignment attacks by limiting allowed fields per controller and action, e.g., it is not possible to insert or update a field value when it was not intended to be part of that action.
+ - **Level:** 2
+ - **Section:** Defensive Coding
+ - **Chapter:** Secure Coding and Architecture
+
+**Files analyzed:** 3 relevant out of 3 total, 0 skipped
+
+**Finding Counts:**
+
+| Severity | Count |
+|----------|-------|
+| Critical | 0 |
+| High | 0 |
+| Medium | 2 |
+| Low | 0 |
+| **Total** | **2** |
+
+**Summary:** The application uses explicit field extraction from form data in each handler and explicit method parameters in the Election model, providing a de facto defense against mass assignment. However, there is no systematic field whitelisting mechanism, and the vote submission handler has a gap where issue-level eligibility isn't enforced at the controller level before attempting writes.
+
+---
+
+## Security Controls Inventory
+
+| Control | Defined Location | Coverage | Status |
+|---------|-----------------|----------|--------|
+| Explicit field extraction from forms | Each POST handler in `pages.py` | All form-handling endpoints | ✅ Applied consistently |
+| Explicit model method parameters (no **kwargs) | `election.py` all methods | All model operations | ✅ Applied consistently |
+| Output field filtering (exclude salt/opened_key) | `election.py:get_metadata()`, `get_issue()` | Metadata/issue retrieval | ✅ Applied consistently |
+| Server-side derivation of sensitive fields | `pages.py:do_create_endpoint()` | Election creation | ✅ owner_pid from session |
+| State assertions preventing modification | `election.py` all mutating methods | All write operations | ✅ Applied consistently |
+| Field whitelist/schema validation per action | — | — | ❌ **Not implemented** |
+| Vote-eligible issue filtering at controller level | — | — | ❌ **Not implemented** |
+
+---
+
+## Critical File Review
+
+### `v3/server/pages.py` — Form-Handling Endpoints
+
+| Endpoint | Form Fields Captured | Fields Actually Used | Unused Input Rejected? |
+|----------|---------------------|---------------------|----------------------|
+| `POST /do-create-election` (line ~493) | All form fields | `form.title` | ❌ No |
+| `POST /do-add-issue/<eid>` (line ~549) | All form fields | `form.title`, `form.description` | ❌ No |
+| `POST /do-edit-issue/<eid>/<iid>` (line ~572) | All form fields | `form.title`, `form.description` | ❌ No |
+| `POST /do-vote/<eid>` (line ~453) | All form fields | Any `vote-*` prefixed key | ❌ No |
+| `POST /do-set-open_at/<eid>` (line ~439) | All JSON fields | `data.get('date')` | ❌ No |
+| `POST /do-set-close_at/<eid>` (line ~446) | All JSON fields | `data.get('date')` | ❌ No |
+
+### `v3/steve/election.py` — Model Methods
+
+| Method | Parameters | Accepts **kwargs? | State Guard? |
+|--------|-----------|-------------------|--------------|
+| `create()` (line ~370) | `title, owner_pid, authz, open_at, close_at` | ❌ No | N/A (class method) |
+| `add_issue()` (line ~148) | `title, description, vtype, kv` | ❌ No | ✅ `assert is_editable()` |
+| `edit_issue()` (line ~166) | `iid, title, description, vtype, kv` | ❌ No | ✅ `assert is_editable()` |
+| `add_vote()` (line ~216) | `pid, iid, votestring` | ❌ No | ✅ `_all_metadata(S_OPEN)` |
+| `set_open_at()` (line ~437) | `timestamp` | ❌ No | ❌ No |
+| `set_close_at()` (line ~441) | `timestamp` | ❌ No | ❌ No |
+
+---
+
+## Findings
+
+### Medium Severity
+
+#### ASVS-1533-MED-001: No Explicit Field Whitelist Enforcement on Form-Handling Endpoints
+
+**Severity:** MEDIUM
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Functions:** `do_create_endpoint()` (line ~493), `do_add_issue_endpoint()` (line ~549), `do_edit_issue_endpoint()` (line ~572), `do_vote_endpoint()` (line ~453)
+
+**Related Functions Checked:**
+- `steve.election.Election.create()` (`election.py` line ~370)
+- `election.add_issue()` (`election.py` line ~148)
+- `election.edit_issue()` (`election.py` line ~166)
+
+**Vulnerable Code:**
+```python
+# do_create_endpoint (~line 500)
+form = edict(await quart.request.form)
+election = steve.election.Election.create(DB_FNAME, form.title, result.uid)
+
+# do_add_issue_endpoint (~line 557)
+form = edict(await quart.request.form)
+vtype = 'yna'
+kv = None
+iid = election.add_issue(form.title, form.description, vtype, kv)
+
+# do_edit_issue_endpoint (~line 580)
+form = edict(await quart.request.form)
+election.edit_issue(issue.iid, form.title, form.description, issue.vtype, issue.kv)
+```
+
+**Data Flow:**
+```
+HTTP POST body (all fields)
+ → edict(await quart.request.form) (captures everything)
+ → form.specific_field (selective access, no whitelist rejection)
+```
+
+**Description:**
+
+All form-handling POST endpoints capture the complete form submission into an `EasyDict` object without validating or restricting the set of allowed fields. While individual handlers currently extract only specific fields (e.g., `form.title`, `form.description`), unexpected fields are silently accepted rather than rejected. The `EasyDict` class makes any form field accessible as an attribute, meaning any code that accesses `form.attacker_field` will succeed if the attacker included it in [...]
+
+The code comments explicitly acknowledge that field handling is incomplete:
+```python
+### do better with these
+vtype = 'yna'
+kv = None
+### for STV, there is a SEATS form parameter. Create empty LABELMAP.
+```
+
+This indicates planned expansion of accepted fields without an established whitelisting pattern to govern which fields become accepted.
+
+**Proof of Concept:**
+```http
+POST /do-create-election HTTP/1.1
+Content-Type: application/x-www-form-urlencoded
+
+title=Legitimate+Election&authz=public&open_at=2025-01-01&close_at=2025-12-31
+```
+Currently, the extra fields (`authz`, `open_at`, `close_at`) are captured in the `form` edict but not used. However, if a developer later changes the handler to pass `form.authz` or `form.open_at` to `Election.create()`, the attacker's values would be used without additional validation—the EasyDict pattern makes this a single-line change with no compile-time or runtime guard.
+
+**Impact:**
+- **Current:** No active exploit—extra fields are ignored by current handlers
+- **Structural risk:** Any future code accessing `form.<field>` immediately trusts attacker input; no systematic defense prevents mass assignment when handlers evolve
+- **ASVS 15.3.3 compliance gap:** The requirement calls for "countermeasures to protect against mass assignment attacks by limiting allowed fields per controller and action"—passive non-use of extra fields is not an active countermeasure
+
+**Remediation:**
+
+Implement explicit field whitelisting per action. Example using a helper function:
+
+```python
+ALLOWED_FIELDS = {
+ 'create_election': {'title'},
+ 'add_issue': {'title', 'description'},
+ 'edit_issue': {'title', 'description'},
+ 'vote': set(), # Dynamic vote-* fields handled separately
+}
+
+def extract_allowed_fields(form_data, action_name):
+ """Extract only whitelisted fields from form data, rejecting unexpected fields."""
+ allowed = ALLOWED_FIELDS.get(action_name, set())
+ unexpected = set(form_data.keys()) - allowed
+ if unexpected:
+ _LOGGER.warning(f'Unexpected form fields rejected for {action_name}: {unexpected}')
+ quart.abort(400, f'Unexpected fields: {", ".join(unexpected)}')
+ return {k: form_data[k] for k in allowed if k in form_data}
+
+# Usage in handler:
+@APP.post('/do-create-election')
+@asfquart.auth.require({R.pmc_member})
+async def do_create_endpoint():
+ result = await basic_info()
+ raw_form = await quart.request.form
+ fields = extract_allowed_fields(raw_form, 'create_election')
+ election = steve.election.Election.create(DB_FNAME, fields['title'], result.uid)
+```
+
+---
+
+#### ASVS-1533-MED-002: Vote Submission Handler Does Not Restrict Writable Issue IDs to Voter's Eligible Subset
+
+**Severity:** MEDIUM
+
+**Location:**
+- **File:** `v3/server/pages.py`
+- **Function:** `do_vote_endpoint()` (line ~453)
+
+**Related Functions Checked:**
+- `v3/steve/election.py`, method `add_vote()` (line ~216)
+- `election.list_issues()` (line ~130)
+- `election.q_get_mayvote.first_row()` (called from `add_vote`)
+
+**Vulnerable Code (Handler):**
+```python
+# pages.py ~line 468-478
+votes = {}
+for key, value in form.items():
+ if key.startswith('vote-'):
+ iid = key.split('-', 1)[1]
+ votes[iid] = value
+
+issues = election.list_issues()
+issue_dict = {i.iid: i for i in issues} # ALL issues, not voter-eligible subset
+
+for iid, votestring in votes.items():
+ if iid not in issue_dict: # Checks existence in election, NOT voter eligibility
+ await flash_danger(f'Invalid issue ID: {iid}')
+ return quart.redirect(f'/vote-on/{election.eid}', code=303)
+ try:
+ election.add_vote(result.uid, iid, votestring)
+```
+
+**Vulnerable Code (Model):**
+```python
+# election.py ~line 216-228
+def add_vote(self, pid: str, iid: str, votestring: str):
+ md = self._all_metadata(self.S_OPEN)
+ ### validate VOTESTRING for ISSUE.TYPE voting # <-- acknowledged missing validation
+ mayvote = self.q_get_mayvote.first_row(pid, iid)
+ vote_token = crypto.gen_vote_token(md.opened_key, pid, iid, mayvote.salt) # AttributeError if mayvote is None
+```
+
+**Data Flow:**
+```
+Form keys (vote-<attacker_controlled_iid>)
+ → handler validates iid exists in election (not voter-eligible)
+ → add_vote(pid, iid, votestring)
+ → q_get_mayvote.first_row(pid, iid) returns None for ineligible voter
+ → None.salt raises AttributeError
+ → caught by generic except Exception
+```
+
+**Description:**
+
+The vote handler accepts `vote-*` form fields where the issue ID portion is entirely user-controlled from the form key name. The handler validates the issue ID against ALL issues in the election (`issue_dict`), not the subset the voter is eligible for. The actual eligibility check is in the model's `add_vote()` method, but it manifests as an `AttributeError` when accessing `.salt` on a `None` mayvote result—not as an explicit authorization decision.
+
+This creates two problems:
+1. **Gap Type B (Control exists but relied upon via exception):** The mayvote check exists but isn't called explicitly—eligibility enforcement happens as a side effect of `None` attribute access
+2. **Partial batch processing:** The loop processes votes sequentially with early return on error. If an attacker submits legitimate votes first, then an ineligible vote, the legitimate votes are committed but later votes in the batch are not, leaving the voter in a partial state
+
+**Proof of Concept:**
+```http
+POST /do-vote/abc123 HTTP/1.1
+Content-Type: application/x-www-form-urlencoded
+Cookie: session=<valid_session>
+
+vote-eligible_issue_1=yes&vote-eligible_issue_2=no&vote-INELIGIBLE_issue=yes
+```
+
+If `INELIGIBLE_issue` exists in the election but the voter isn't eligible for it:
+- `vote-eligible_issue_1=yes` → committed to database ✅
+- `vote-eligible_issue_2=no` → committed to database ✅
+- `vote-INELIGIBLE_issue=yes` → `add_vote` → `mayvote=None` → `AttributeError` → generic error → redirect
+
+Result: votes 1 and 2 are persisted, vote 3 is rejected, and the user sees a generic "Error submitting vote" message without understanding why.
+
+**Impact:**
+- The ineligible vote is NOT recorded (the model-layer check prevents the write)
+- Legitimate votes submitted before the failure in the same batch ARE persisted without user awareness
+- The controller layer does not limit which issue IDs (fields) are valid per the specific voter's authorization, violating the ASVS 15.3.3 principle of limiting allowed fields per action
+- Error handling reveals that the voter is not eligible for a specific issue (information disclosure)
+
+**Remediation:**
+
+Pre-filter eligible issue IDs at the controller level before processing any votes:
+
+```python
+@APP.post('/do-vote/<eid>')
+@asfquart.auth.require({R.committer})
+@load_election
+async def do_vote_endpoint(election):
+ result = await basic_info()
+ form = edict(await quart.request.form)
+
+ # Pre-load voter's eligible issues
+ election.q_find_issues.perform(result.uid, election.eid)
+ eligible_iids = {row.iid for row in election.q_find_issues.fetchall()}
+
+ votes = {}
+ for key, value in form.items():
+ if key.startswith('vote-'):
+ iid = key.split('-', 1)[1]
+ if iid not in eligible_iids:
+ _LOGGER.warning(
+ f'User[U:{result.uid}] submitted vote for ineligible issue[I:{iid}]'
+ )
+ await flash_danger(f'You are not eligible to vote on issue {iid}.')
+ return quart.redirect(f'/vote-on/{election.eid}', code=303)
+ votes[iid] = value
+
+ # Process only validated votes (consider wrapping in transaction)
+ for iid, votestring in votes.items():
+ election.add_vote(result.uid, iid, votestring)
+
+ await flash_success('Votes submitted successfully!')
+ return quart.redirect('/voter', code=303)
+```
+
+Additionally, add explicit eligibility check in `add_vote`:
+```python
+def add_vote(self, pid: str, iid: str, votestring: str):
+ md = self._all_metadata(self.S_OPEN)
+ mayvote = self.q_get_mayvote.first_row(pid, iid)
+ if mayvote is None:
+ raise VoterNotEligible(pid, iid) # Explicit exception, not AttributeError
+ # ... rest of method
+```
+
+---
+
+## Positive Security Patterns
+
+| Pattern | Location | Description |
+|---------|----------|-------------|
+| **Output field filtering** | `election.py:get_metadata()` (line ~123) | Explicitly constructs return dict, excluding `salt` and `opened_key` |
+| **Issue data filtering** | `election.py:get_issue()` (line ~138) | Returns only `title, description, type, kv` with comment "NEVER return issue.salt" |
+| **Server-side owner derivation** | `pages.py:do_create_endpoint()` (line ~493) | `owner_pid` is set from `result.uid` (session), never from form data |
+| **Explicit model parameters** | `election.py` all methods | No `**kwargs` or dict-unpacking; all parameters are explicitly named |
+| **State machine enforcement** | `election.py:is_editable()`, `is_open()` | Mutating methods assert correct state before processing |
+| **Immutable fields on edit** | `pages.py:do_edit_issue_endpoint()` (line ~572) | `vtype` and `kv` sourced from existing issue (database), not from form |
+| **Selective query columns** | `election.py:owned_elections()` (line ~401) | Comment: "contains subset of columns. We don't want to return the SALT or OPENED_KEY values" |
+
+---
+
+## Architecture Observations
+
+1. **Two-layer defense pattern**: The application provides mass assignment protection at both the handler layer (explicit field extraction) and the model layer (explicit method parameters). This defense-in-depth is effective but informal.
+
+2. **EasyDict pattern risk**: Using `EasyDict` as the form data container makes any form field accessible as a Python attribute. While currently fields are explicitly selected, this pattern doesn't fail-safe—accessing an attacker-supplied field name succeeds silently if it's in the form data.
+
+3. **Incomplete TODO items**: Multiple `### check authz` comments indicate that authorization checks are not yet implemented in management endpoints. While not mass assignment per se, the absence of ownership verification means any committer can manage any election's issues, expanding the attack surface for field manipulation.
+
+4. **Vote type validation gap**: The `add_vote` method includes the comment `### validate VOTESTRING for ISSUE.TYPE voting`, acknowledging that vote content validation is not yet implemented. While not mass assignment, this means vote fields accept arbitrary string values rather than being constrained to valid vote type values (yes/no/abstain for YNA, valid rankings for STV).
+
+---
+
+## Recommendations Summary
+
+### Immediate Actions
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| 1 | Add explicit eligibility check in `add_vote()` before accessing `mayvote.salt` | Low | Prevents `AttributeError` on None; explicit authorization check |
+| 2 | Pre-filter eligible issue IDs in `do_vote_endpoint` before processing votes | Low | Controller-level field restriction per voter |
+
+### Short-term Actions
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| 3 | Implement field whitelist helper function for all form-handling endpoints | Medium | Systematic mass assignment defense; rejects unexpected fields |
+| 4 | Add votestring validation against issue vote type in `add_vote()` | Medium | Prevents invalid vote content from being encrypted and stored |
+| 5 | Remove debug `print('FORM:', form)` statements in `do_add_issue_endpoint` and `do_edit_issue_endpoint` | Low | Prevents full form data leakage to stdout/logs |
+
+### Long-term Actions
+
+| Priority | Recommendation | Effort | Impact |
+|----------|---------------|--------|--------|
+| 6 | Implement ownership/authorization checks on management endpoints (`### check authz` TODOs) | High | Restricts who can modify election fields |
+
+---
+
+## Appendix: Files Analyzed
+
+<details>
+<summary>Click to expand file list (3 files)</summary>
+
+- `v3/server/api.py` (relevance: 10)
+- `v3/server/pages.py` (relevance: 10)
+- `v3/steve/election.py` (relevance: 10)
+
+</details>
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org