PST-030: The company identifier formset discloses whether another account's identifier row id exists #759

New issue

Closed

opened 2026-10-10 10:56:06 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-10 10:56:06 +00:00

Owner

Found in the security audit of 2026-10-10 (SECURITY-AUDIT-2026-10-10.md), run against main at c30851b from 8 to 10 October 2026, as PST-030. Severity: Low.

The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it.

Severity: Low
Category: information-disclosure / existence-oracle
Location: src/postulo/jobs/forms.py (491-540)
Found by: ownership review of jobs

Who can do it, and what they need. Any signed-in account on the same instance. Precondition: none beyond a session and one company of their own (or the create form). Verified with a scratch test under /tmp/.../verify/own-jobs/test_identifier_row_oracle.py and test_more.py.

How it works.

Sign in as account A.
POST to /jobs/companies/<own pk>/edit/ (or /jobs/companies/new/) with identifiers-INITIAL_FORMS=1 and identifiers-0-id=<N>.
Django's BaseModelFormSet looks N up among the formset's own rows (scoped to the company, so a foreign row is treated as a new, pk-less instance and silently dropped) but the hidden id ModelChoiceField it adds validates against CompanyIdentifier._default_manager — every account's rows.
If row N belongs to account B the formset is valid and the view answers 302 (company saved; B's row untouched). If row N does not exist the formset is invalid ('Select a valid choice' on the hidden id) and the view answers 200, refusing the whole save.
Comparing the two answers tells A whether identifier row N exists anywhere on the instance; posting TOTAL_FORMS=k rows per request tests k ids at once. Measured: foreign existing id -> 302, missing id -> 200 on both the update and the create view. The profile's PersonIdentifier formset (accounts/forms.py:1409, which carries RowsAlreadyGone) answers 302 for both, which is the behaviour the threat model promises.

Impact. Confirms the existence (and by enumeration the count and growth) of other accounts' company-identifier rows, contradicting the threat model's rule that another person's record is never confirmed to exist. No content is read and nothing is modified: the foreign row is never written to or deleted (verified, including with DELETE ticked).

Evidence.

src/postulo/jobs/forms.py:491 `class BaseCompanyIdentifierFormSet(OneOfEachKind, forms.BaseInlineFormSet):` (no RowsAlreadyGone) vs src/postulo/accounts/forms.py:1409 `class BasePersonIdentifierFormSet(RowsAlreadyGone, OneOfEachKind, forms.BaseInlineFormSet):`. src/postulo/core/formsets.py:66-95 `class RowsAlreadyGone: ... form.already_gone = (self.is_bound and i < self.initial_form_count() and form.instance.pk is None)` / `def _should_delete_form(self, form) -> bool: return getattr(form, "already_gone", False) or super()._should_delete_form(form)` -- its docstring says this is 'the whole of what happens to a key somebody made up, or to another person's'. src/postulo/jobs/views.py:395 `forms
…

Verification (verdict: confirmed; severity after verification: Low). Lines are right: forms.py:491-540 and views.py:388-418 (get_identifiers at 388, refuse_bad_identifiers at 405). Precondition: any signed-in account; the create view works too, but it creates a company as a side effect. The only information disclosed is whether an identifier row with integer pk N exists on the instance, not whose it is or what it holds. Batching with TOTAL_FORMS=k only answers 'at least one is missing', so bisection is needed. Remediation: add RowsAlreadyGone to BaseCompanyIdentifierFormSet and add the company identifier formset to tests/security/test_formset_rows.py.

Suggested remediation. Add RowsAlreadyGone to BaseCompanyIdentifierFormSet exactly as BasePersonIdentifierFormSet does, so a made-up key and another person's key both become a silently passed-over row and the page answers the same way for either. Extend tests/security/test_formset_rows.py to the company form (jobs:company_update and jobs:company_create) and, for every inline formset, assert that a non-existent id and another account's id produce the same status code and the same state.

Found in the security audit of 2026-10-10 (`SECURITY-AUDIT-2026-10-10.md`), run against `main` at `c30851b` from 8 to 10 October 2026, as **PST-030**. Severity: **Low**. The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it. --- - **Severity:** Low - **Category:** information-disclosure / existence-oracle - **Location:** `src/postulo/jobs/forms.py` (491-540) - **Found by:** ownership review of jobs **Who can do it, and what they need.** Any signed-in account on the same instance. Precondition: none beyond a session and one company of their own (or the create form). Verified with a scratch test under /tmp/.../verify/own-jobs/test_identifier_row_oracle.py and test_more.py. **How it works.** 1. Sign in as account A. 2. POST to /jobs/companies/<own pk>/edit/ (or /jobs/companies/new/) with identifiers-INITIAL_FORMS=1 and identifiers-0-id=<N>. 3. Django's BaseModelFormSet looks N up among the formset's own rows (scoped to the company, so a foreign row is treated as a new, pk-less instance and silently dropped) but the hidden `id` ModelChoiceField it adds validates against CompanyIdentifier._default_manager — every account's rows. 4. If row N belongs to account B the formset is valid and the view answers 302 (company saved; B's row untouched). If row N does not exist the formset is invalid ('Select a valid choice' on the hidden id) and the view answers 200, refusing the whole save. 5. Comparing the two answers tells A whether identifier row N exists anywhere on the instance; posting TOTAL_FORMS=k rows per request tests k ids at once. Measured: foreign existing id -> 302, missing id -> 200 on both the update and the create view. The profile's PersonIdentifier formset (accounts/forms.py:1409, which carries RowsAlreadyGone) answers 302 for both, which is the behaviour the threat model promises. **Impact.** Confirms the existence (and by enumeration the count and growth) of other accounts' company-identifier rows, contradicting the threat model's rule that another person's record is never confirmed to exist. No content is read and nothing is modified: the foreign row is never written to or deleted (verified, including with DELETE ticked). **Evidence.** ```text src/postulo/jobs/forms.py:491 `class BaseCompanyIdentifierFormSet(OneOfEachKind, forms.BaseInlineFormSet):` (no RowsAlreadyGone) vs src/postulo/accounts/forms.py:1409 `class BasePersonIdentifierFormSet(RowsAlreadyGone, OneOfEachKind, forms.BaseInlineFormSet):`. src/postulo/core/formsets.py:66-95 `class RowsAlreadyGone: ... form.already_gone = (self.is_bound and i < self.initial_form_count() and form.instance.pk is None)` / `def _should_delete_form(self, form) -> bool: return getattr(form, "already_gone", False) or super()._should_delete_form(form)` -- its docstring says this is 'the whole of what happens to a key somebody made up, or to another person's'. src/postulo/jobs/views.py:395 `forms … ``` **Verification** (verdict: confirmed; severity after verification: Low). Lines are right: forms.py:491-540 and views.py:388-418 (get_identifiers at 388, refuse_bad_identifiers at 405). Precondition: any signed-in account; the create view works too, but it creates a company as a side effect. The only information disclosed is whether an identifier row with integer pk N exists on the instance, not whose it is or what it holds. Batching with TOTAL_FORMS=k only answers 'at least one is missing', so bisection is needed. Remediation: add RowsAlreadyGone to BaseCompanyIdentifierFormSet and add the company identifier formset to tests/security/test_formset_rows.py. **Suggested remediation.** Add `RowsAlreadyGone` to `BaseCompanyIdentifierFormSet` exactly as `BasePersonIdentifierFormSet` does, so a made-up key and another person's key both become a silently passed-over row and the page answers the same way for either. Extend tests/security/test_formset_rows.py to the company form (jobs:company_update and jobs:company_create) and, for every inline formset, assert that a non-existent id and another account's id produce the same status code and the same state.

tiagoagueda added the

bug

security

labels

2026-10-10 10:56:06 +00:00

tiagoagueda added this to the 0.6.0 milestone

2026-10-10 10:57:38 +00:00

tiagoagueda referenced this issue

2026-10-10 10:58:40 +00:00

Security audit of 2026-10-10: index of PST-001 to PST-098 (#730–#827) #828

tiagoagueda referenced this issue

2026-10-10 16:01:27 +00:00