A form payload's id picks the uuid, and a scoped draft save then writes over any draft with RBAC off #4664

Found during a Strict review of #4580 (release development → beta). Present on development at 8e7bbd87; introduced in #4563, #4566.

Impact

With no mapping pairsOf() passes the payload's id through, SaveObject takes it as the uuid, and a scoped draft save writes with _rbac: false, so any object in the destination schema can be overwritten.

The chain

pairsOf() drops only _ and @ keys when a write has no mapping, so a submitter's id reaches $plan['object']. writeAll() passes uuid: null for a fresh submit or a new draft, and SaveObject::extractUuidAndSelfData() (SaveObject.php:3538-3539) then takes $data['id'] as the uuid. SaveObject looks that uuid up with _rbac: false and, when it exists, takes the update branch (handleObjectUpdate, SaveObject.php:3330).

For a scope-value owner (decision 195), writeAll() saves with _rbac: false. refuseSomebodyElsesDraft() returns early when RBAC is off, and DraftStatusPolicy::resolveStatus() accepts draft → draft. So a portal resident whose form has no mapping can send { "id": "<another person's draft uuid>", ... } to saveDraft() and overwrite that draft in the destination schema. The victim can be another resident or a Nextcloud user. The draft keeps its owner, so the victim later finds and submits content they did not type. The comment at lines 685-697 ("Updating or sending it was authorised by DraftFinder::own() before this write") does not hold on this path: own() is only asked when $draftId is set.

The same key affects a fresh submit under RBAC. A uuid the caller cannot read goes down checkSavePermissions()'s create branch (ObjectService.php:2275-2287). It is judged by the schema's create rule (for an anonymous visitor on a public form: the public create rule) and then stored over the hidden object. refuseSomebodyElsesDraft() closes that only for drafts.

I could not check whether the in-process callers (the portal) ever pass mapping: null. The spec explicitly allows it ("without a mapping the payload minus _ and @ keys is the object"), and FormsController::payload() forwards id too. Uncertain, so a blocker under Strict.

Impact: a form submitter (anonymous on a public form, or a portal resident through a scoped draft) can change existing objects of the destination schema that they cannot read, including other people's drafts.

Suggested fix: in pairsOf(), also drop id and uuid when no mapping is given, so a submit never chooses its uuid. The only way to name an existing object is then $draftId, which goes through DraftFinder::own(). As a backstop, in writeAll() refuse when $uuid === null and the object still carries an id.

Done when:

• a new FormSubmitServiceTest case: saveDraft() with a scoped owner, mapping: null and payload id = another owner's draft uuid either creates a new object with a fresh uuid or is refused, and the other draft's data is unchanged. The test goes red with the id filter removed
• the same for submit() with an anonymous subject and the id of an existing object

Reachability: a scope-value owner is only created through DraftOwner::portalSubject() / scoped() by an in-process caller; no caller in this repository creates one, so the write path is reached through the portal app's use of saveDraft(). Reported as a blocker under Strict because the payload id is not stripped on a write that runs with RBAC off.

Verified against 8e7bbd87eede51a19cd16b1eda5f1f9cbe21dfc0.

Reproduction

Verified by reading the code at 8e7bbd87; not executed.

Done when

• FormSubmitServiceTest: saveDraft() with a scoped owner, mapping null and payload id = another owner's draft uuid leaves that draft unchanged; red with the id filter removed
• FormSubmitServiceTest: anonymous submit() with payload id of an existing object does not update it

Review thread

#4580 (comment)