Skip to content

gate-7: getGebruikenForDeelnemer's org scope may be vacuous — array vs scalar deelnemers filter, unverified #503

Description

@rubenvdlinde

GebruikController::getGebruikenForDeelnemer() (lib/Controller/GebruikController.php:282) is the one no-admin-idor finding left open in PR #502, deliberately. This issue records why, so nobody closes it with an exemption.

What is definitely right

The organisation scope is forced after getParams():

$options = $this->request->getParams();
$options['deelnemers'] = [$orgUuid];

so a caller cannot supply or override deelnemers and read another organisation's usage. $orgUuid comes from getUserValue(..., 'core', 'organisation') — session-derived, not client input.

What is unverified, and why it matters

GebruikService::getGebruiken() (lib/Service/GebruikService.php:152) calls:

$objectService->searchObjectsPaginated(query: $options, _rbac: false, _multitenancy: false);

Both bypasses are on. So the deelnemers predicate is the only thing scoping this read — there is no second line of defence.

And it is passed in a form the rest of the app does not use:

call site form
GebruikController.php:292 (this one) array['deelnemers' => [$orgUuid]]
AangebodenGebruikService::getGebruiksWhereDeelnemers() :810 scalar['deelnemers' => $currentOrg]
ArchiMateService.php:402 scalar

deelnemers is type: array of related-object (lib/Settings/softwarecatalogus_register.json, gebruik.properties.deelnemers). Whether OpenRegister honours array-containment matching on such a property is not establishedReviewAggregateService's own docblock notes this app has no confirmed precedent for it.

If OpenRegister silently ignores the array form, this scope is vacuous and the endpoint returns every organisation's gebruik data.

Why it was not settled here

oc_openregister_table_13_40 (the gebruik shard, register 13) has zero rows on the available instance. A live A/B between the array and scalar forms would have returned empty under both and proved nothing — the null-result trap, where a control that could not have moved gets read as a passing result.

Swapping the filter form on the strength of a guess is equally unsafe: if the array form is honoured, changing it could break a working endpoint.

What is fixed in #502

The fail-closed check its canonical sibling already has (AangebodenGebruikService:779-789). getUserValue() returns '', not null, when the user-value was never set, so without it the scope degraded to deelnemers: [''] for any account with no organisation — and an empty predicate that OpenRegister chooses to ignore returns everything rather than nothing.

What still needs doing

On an instance with gebruik rows across at least two organisations, run both forms and compare row counts:

  1. ['deelnemers' => [$orgUuid]] — the current form
  2. ['deelnemers' => $orgUuid] — the canonical form
  3. no deelnemers key at all — the control that shows what "unscoped" looks like

If (1) equals (3) and (2) is smaller, the array form is being ignored and this is a live cross-organisation leak.

The gate finding is kept until then. _is_authentication_only_condition correctly blanks the empty-org check — "does this caller have an organisation" is authentication, not authorisation — so the gate is not satisfied, and it should not be. Exempting it would manufacture the coverage.

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