Skip to content

security(spp_change_request_v2): detail create() is unguarded, so a populated row can be crafted and bound to a detail-less submitted request #538

Description

@gonzalesedwin1123

Found in the adversarial review of PR #536 (#443 work). Pre-existing since 19.0.3.1.10.

_assert_content_editable runs from write() only; create() on the detail models is unguarded, and group_cr_user has create rights on them (ir.model.access.csv). _alters_frozen_field (models/change_request.py ~706-720) accepts binding detail_res_id to any row that already points back at the request. So on a submitted request that has no detail row, a user can create() a fully populated detail with change_request_id = <cr> (values of their choosing, bypassing the freeze entirely) and then bind it through the 19.0.3.1.10 rule.

The legitimate repair path (_ensure_detail, after #536: create with the registrant prefill, under sudo) needs exactly one of these creates; user-driven creates pointing at a non-draft request do not.

Suggested fix: in the detail base create(), refuse a row whose change_request_id is not in draft/revision unless the values are the registrant prefill (or route the repair through a sudo-only helper and refuse all user creates against submitted requests), plus a test that crafts and binds such a row and asserts the refusal. Consider also making _alters_frozen_field accept a first binding only for a row created by the system (e.g. create_uid is OdooBot / created within _ensure_detail).

Related: #321 (registrant_id not frozen post-submit); #443.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions