Importing an archive: leftover keys reach the model constructors, so a crafted file writes into other accounts #354

New issue

Closed

opened 2026-10-01 10:46:04 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:04 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading core/importer.py and loading hand-made archives into two test accounts.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

load() takes out the keys it knows about and hands everything else in an entry straight to a model constructor (Model.objects.create(owner=user, ..., **entry)), and for the profile it calls setattr for any attribute the profile has. Nothing restricts the keys to the fields the exporter writes. An archive that carries owner_id, posting_id, application_id, user_id, id or a file field's name therefore sets those columns, and a leftover owner_id wins over the owner=user passed beside it, because Django applies the remaining keyword after the related object. That is mass assignment across the ownership boundary.

Where: core/importer.py, in load(): the profile loop at :366-371 (if hasattr(profile, name) and value: setattr(profile, name, value)); the career sections at :442; skills at :457; restore_contact at :515; companies at :577; listings at :623; applications at :658-663; timeline events at :684 (only id and created_at are removed); interviews at :709-717; offers at :724-731; CVs at :808; letters at :833; uploads at :845; sent documents at :872-877; captures at :902-904. Reached through core/management/commands/import_data.py:25-36, which the wiki documents under Backups and your data → Putting an archive back.
What happens: loading one archive into account A, checked against the snapshot:
1. an experience entry with owner_id set to account B's id creates the row in B's career;
2. an application entry with posting_id set to one of B's listings attaches A's application to B's listing, whose title, description and notes then appear on A's application page;
3. a timeline event with application_id set to one of B's applications lands in B's timeline;
4. account.profile with id and user_id set to B's overwrites B's profile (the headline, and any other field named);
5. account.profile.avatar set to the storage name of B's uploaded CV makes AvatarView serve B's CV to A; when A's account is later deleted, accounts/deletion.py:files_of() collects that name and deletes B's file from disk.
Who can trigger it: an account holder who hands the operator an archive to restore (from another instance, or edited by hand), once the operator runs import_data for them. Also anyone whose file an operator restores without reading it.
Why it matters: the promise that one person's data never reaches another's breaks inside a documented operator flow, in both directions: reading another account's listing and files, writing into another account's timeline and profile, and deleting another account's file.
Fix: build every row from an explicit allow-list per model, the fields core/export.py writes, with every relation resolved only through the importer's own id maps (postings, applications, contacts, resume_map and so on). Drop anything else before the constructor, and in particular every id, *_id, owner, user and file field. For the profile, assign only named plain fields; never a relation and never a file field. Restore files only from the archive's own members (avatar_file, logo_file, file), never from a storage name the JSON supplies.

Related to #321, whose point 4 covers only the importer skipping the username rules.

Done when

tests/security/test_import_isolation.py loads an archive whose entries carry owner_id, posting_id, application_id, id, user_id and avatar pointing at a second account, and asserts that the second account's rows, profile and files are unchanged and that every row created belongs to the importing account. It fails today and passes after the fix.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `core/importer.py` and loading hand-made archives into two test accounts. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. `load()` takes out the keys it knows about and hands everything else in an entry straight to a model constructor (`Model.objects.create(owner=user, ..., **entry)`), and for the profile it calls `setattr` for any attribute the profile has. Nothing restricts the keys to the fields the exporter writes. An archive that carries `owner_id`, `posting_id`, `application_id`, `user_id`, `id` or a file field's name therefore sets those columns, and a leftover `owner_id` wins over the `owner=user` passed beside it, because Django applies the remaining keyword after the related object. That is mass assignment across the ownership boundary. - **Where:** `core/importer.py`, in `load()`: the profile loop at `:366-371` (`if hasattr(profile, name) and value: setattr(profile, name, value)`); the career sections at `:442`; skills at `:457`; `restore_contact` at `:515`; companies at `:577`; listings at `:623`; applications at `:658-663`; timeline events at `:684` (only `id` and `created_at` are removed); interviews at `:709-717`; offers at `:724-731`; CVs at `:808`; letters at `:833`; uploads at `:845`; sent documents at `:872-877`; captures at `:902-904`. Reached through `core/management/commands/import_data.py:25-36`, which the wiki documents under *Backups and your data → Putting an archive back*. - **What happens:** loading one archive into account A, checked against the snapshot: 1. an `experience` entry with `owner_id` set to account B's id creates the row in B's career; 2. an application entry with `posting_id` set to one of B's listings attaches A's application to B's listing, whose title, description and notes then appear on A's application page; 3. a timeline event with `application_id` set to one of B's applications lands in B's timeline; 4. `account.profile` with `id` and `user_id` set to B's overwrites B's profile (the headline, and any other field named); 5. `account.profile.avatar` set to the storage name of B's uploaded CV makes `AvatarView` serve B's CV to A; when A's account is later deleted, `accounts/deletion.py:files_of()` collects that name and deletes B's file from disk. - **Who can trigger it:** an account holder who hands the operator an archive to restore (from another instance, or edited by hand), once the operator runs `import_data` for them. Also anyone whose file an operator restores without reading it. - **Why it matters:** the promise that one person's data never reaches another's breaks inside a documented operator flow, in both directions: reading another account's listing and files, writing into another account's timeline and profile, and deleting another account's file. - **Fix:** build every row from an explicit allow-list per model, the fields `core/export.py` writes, with every relation resolved only through the importer's own id maps (`postings`, `applications`, `contacts`, `resume_map` and so on). Drop anything else before the constructor, and in particular every `id`, `*_id`, `owner`, `user` and file field. For the profile, assign only named plain fields; never a relation and never a file field. Restore files only from the archive's own members (`avatar_file`, `logo_file`, `file`), never from a storage name the JSON supplies. Related to #321, whose point 4 covers only the importer skipping the username rules. ## Done when - `tests/security/test_import_isolation.py` loads an archive whose entries carry `owner_id`, `posting_id`, `application_id`, `id`, `user_id` and `avatar` pointing at a second account, and asserts that the second account's rows, profile and files are unchanged and that every row created belongs to the importing account. It fails today and passes after the fix.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:04 +00:00

tiagoagueda added the

tier

bug

security

labels

2026-10-01 10:46:04 +00:00

tiagoagueda referenced this issue from a commit

2026-10-01 18:27:25 +00:00

A CV chooses which of your details it prints

tiagoagueda referenced this issue