From fc94118b2d20a051ef04e2c2db94c426f54f2d6a Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Sun, 10 May 2026 18:16:17 +0200 Subject: [PATCH] fix: prevent mass-assignment user_id spoofing in POST /api/v1/evaluations/feedback (#24508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: prevent mass-assignment user_id spoofing in POST /api/v1/evaluations/feedback Two independent gaps in backend/open_webui/models/feedbacks.py let an authenticated caller forge the `user_id` (and `id`, `version`) on a new feedback record submitted to POST /api/v1/evaluations/feedback: 1. `FeedbackForm` declared `model_config = ConfigDict(extra='allow')`, so Pydantic preserved any extra fields supplied in the request body — including `user_id`, `id`, `version`. The form is the public input boundary for the endpoint and should not accept unknown fields. 2. In `insert_new_feedback`, the dict literal placed `**form_data.model_dump()` AFTER `'id': id`, `'user_id': user_id`, `'version': 0`. Python dict-literal duplicate-key resolution is last-wins, so any of those fields present in `form_data` overwrote the server-derived values. Combined effect: a regular user could POST a feedback record with an arbitrary `user_id`, attributing the rating to any other user. The Elo leaderboard at backend/open_webui/routers/evaluations.py computes model rankings from these records, and the admin export (GET /api/v1/evaluations/feedbacks/export) and admin list (GET /api/v1/evaluations/feedbacks/all) display the spoofed attribution. Two fixes, defense-in-depth: - FeedbackForm: switch `extra='allow'` to `extra='ignore'` so Pydantic drops unknown fields at parse time. Sub-models (RatingData / MetaData / SnapshotData) intentionally keep `extra='allow'` because their contents are deliberately schema-flexible — the spoofing surface was the form, not the sub-payloads. - insert_new_feedback: spread `form_data.model_dump()` first, then overlay server-controlled fields (`id`, `user_id`, `version`, `created_at`, `updated_at`) so the explicit keys win on duplicate-key resolution regardless of what reaches the function. Matches the secure pattern already used in backend/open_webui/models/functions.py:120. Reported by yantongggg in GHSA-rjmp-vjf2-qf4g. Same root-cause class as the prior published GHSA-hr43-rjmr-7wmm (folder mass-assignment, fixed in v0.9.0); that fix did not generalize across the codebase, this fix closes the feedback variant. Co-authored-by: yantongggg * chore: trim comments --------- Co-authored-by: yantongggg --- backend/open_webui/models/feedbacks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/open_webui/models/feedbacks.py b/backend/open_webui/models/feedbacks.py index 02f61f82ee..d8ae4dc9b1 100644 --- a/backend/open_webui/models/feedbacks.py +++ b/backend/open_webui/models/feedbacks.py @@ -103,7 +103,8 @@ class FeedbackForm(BaseModel): data: Optional[RatingData] = None meta: Optional[dict] = None snapshot: Optional[SnapshotData] = None - model_config = ConfigDict(extra='allow') + # ignore: drop client-supplied id/user_id/version/timestamps at parse time. + model_config = ConfigDict(extra='ignore') class UserResponse(BaseModel): @@ -145,12 +146,13 @@ class FeedbackTable: ) -> Optional[FeedbackModel]: async with get_async_db_context(db) as db: id = str(uuid.uuid4()) + # Spread form_data first so server-controlled fields win on duplicate keys. feedback = FeedbackModel( **{ + **form_data.model_dump(), 'id': id, 'user_id': user_id, 'version': 0, - **form_data.model_dump(), 'created_at': int(time.time()), 'updated_at': int(time.time()), }