openvibers/openvibe.wiki Commits
Security: a renamed or deleted space's old slug cannot be claimed by another space
Changes
@@@ -169,6 +169,15 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),return out;}++ /**++ * A renamed (or deleted) space's old slug keeps answering with its redirect (or 410): nobody but++ * that space may take it, or the old links would serve someone else's pages.++ */++ function checkSlugNotRetired(slug, spaceId = null) {++ const r = svc.resolveRedirect(`/s/${slug}`);++ if (r && r.entityId !== spaceId) fail(409, 'space.slug_retired', `The space slug "${slug}" belonged to another space and still leads there`);++ }++function wrapContentError(fn) {try { return fn(); } catch (err) {if (err instanceof content.ContentError) fail(422, err.code, err.message);@@@ -399,6 +408,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),const owner = kind === 'user' ? actor.subject : (actor && actor.kind === 'service' ? actor.service : 'svc:wiki');return tx(() => {if (q.spaceBySlug.get(cleanSlug)) fail(409, 'space.slug_taken', `The space slug "${cleanSlug}" is taken`);++ checkSlugNotRetired(cleanSlug);const t = now();const id = spaceIdNew(t);q.insertSpace.run({ id, slug: cleanSlug, name: cleanName, description: description == null ? null : String(description).slice(0, 1000), kind, visibility: vis, owner, created_by: actorId(actor), now: t });@@@ -426,6 +436,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),if (!content.SPACE_SLUG_RE.test(next.slug)) fail(422, 'space.invalid_slug', 'A space slug is 2–63 lowercase letters, digits and dashes');if (next.slug !== space.slug) {if (q.spaceBySlug.get(next.slug)) fail(409, 'space.slug_taken', `The space slug "${next.slug}" is taken`);++ checkSlugNotRetired(next.slug, space.id);redirects.recordMove(space.id, `/s/${space.slug}`, `/s/${next.slug}`, { reason: 'space_renamed' });for (const p of q.pagesOfSpace.all(space.id)) redirects.recordMove(p.id, `/w/${space.slug}/${p.slug}`, `/w/${next.slug}/${p.slug}`, { reason: 'space_renamed' });}nnew file mode 100644@@@ -0,0 +1,37 @@++'use strict';++/**++ * Security regressions: a renamed space's old address stays its redirect; another person cannot++ * claim it (by creating a space or renaming theirs) and serve their pages at the old links.++ */++const assert = require('assert');++const H = require('./helpers');++++(async () => {++ const h = await H.boot();++ const owner = { kind: 'user', subject: H.subject(), staff: false };++ const mallory = { kind: 'user', subject: H.subject(), staff: false };++ try {++ const space = h.svc.createSpace({ name: 'Handbook', slug: 'handbook' }, owner);++ const { page } = h.svc.createPage(space.id, { title: 'Onboarding', body: H.LONG }, owner);++ h.svc.publish(page.id, {}, owner);++ h.svc.updateSpace(space.id, { slug: 'guide' }, owner);++++ // Another person cannot take the retired slug, by creating a space or by renaming theirs.++ assert.throws(() => h.svc.createSpace({ name: 'Handbook', slug: 'handbook' }, mallory), (e) => e.status === 409 && e.code === 'space.slug_retired');++ const own = h.svc.createSpace({ name: 'Mine', slug: 'mine' }, mallory);++ assert.throws(() => h.svc.updateSpace(own.id, { slug: 'handbook' }, mallory), (e) => e.status === 409 && e.code === 'space.slug_retired');++ const r = await H.req(h, 'POST', '/api/v1/spaces', { token: H.userToken({ subject: mallory.subject }), body: { name: 'Handbook', slug: 'handbook' } });++ assert.strictEqual(r.status, 409);++++ // The old links still lead to the real space and page.++ assert.strictEqual((await H.req(h, 'GET', '/s/handbook')).headers.get('location'), '/s/guide');++ assert.strictEqual((await H.req(h, 'GET', '/w/handbook/onboarding')).headers.get('location'), '/w/guide/onboarding');++++ // The space itself may take its old slug back.++ h.svc.updateSpace(space.id, { slug: 'handbook' }, owner);++ assert.strictEqual((await H.req(h, 'GET', '/w/handbook/onboarding')).status, 200);++ console.log('security: ok');++ } finally {++ await h.stop();++ }++})().catch((e) => { console.error(e); process.exit(1); });
@@ -169,6 +169,15 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
@@ -169,6 +169,15 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
return out;
return out;
}
}
+ /**
+ * A renamed (or deleted) space's old slug keeps answering with its redirect (or 410): nobody but
+ * that space may take it, or the old links would serve someone else's pages.
+ */
+ function checkSlugNotRetired(slug, spaceId = null) {
+ const r = svc.resolveRedirect(`/s/${slug}`);
+ if (r && r.entityId !== spaceId) fail(409, 'space.slug_retired', `The space slug "${slug}" belonged to another space and still leads there`);
+ }
+
function wrapContentError(fn) {
function wrapContentError(fn) {
try { return fn(); } catch (err) {
try { return fn(); } catch (err) {
if (err instanceof content.ContentError) fail(422, err.code, err.message);
if (err instanceof content.ContentError) fail(422, err.code, err.message);
@@ -399,6 +408,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
@@ -399,6 +408,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
const owner = kind === 'user' ? actor.subject : (actor && actor.kind === 'service' ? actor.service : 'svc:wiki');
const owner = kind === 'user' ? actor.subject : (actor && actor.kind === 'service' ? actor.service : 'svc:wiki');
return tx(() => {
return tx(() => {
if (q.spaceBySlug.get(cleanSlug)) fail(409, 'space.slug_taken', `The space slug "${cleanSlug}" is taken`);
if (q.spaceBySlug.get(cleanSlug)) fail(409, 'space.slug_taken', `The space slug "${cleanSlug}" is taken`);
+ checkSlugNotRetired(cleanSlug);
const t = now();
const t = now();
const id = spaceIdNew(t);
const id = spaceIdNew(t);
q.insertSpace.run({ id, slug: cleanSlug, name: cleanName, description: description == null ? null : String(description).slice(0, 1000), kind, visibility: vis, owner, created_by: actorId(actor), now: t });
q.insertSpace.run({ id, slug: cleanSlug, name: cleanName, description: description == null ? null : String(description).slice(0, 1000), kind, visibility: vis, owner, created_by: actorId(actor), now: t });
@@ -426,6 +436,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
@@ -426,6 +436,7 @@ function createWikiService({ db, stores, outbox, config, now = () => Date.now(),
if (!content.SPACE_SLUG_RE.test(next.slug)) fail(422, 'space.invalid_slug', 'A space slug is 2–63 lowercase letters, digits and dashes');
if (!content.SPACE_SLUG_RE.test(next.slug)) fail(422, 'space.invalid_slug', 'A space slug is 2–63 lowercase letters, digits and dashes');
if (next.slug !== space.slug) {
if (next.slug !== space.slug) {
if (q.spaceBySlug.get(next.slug)) fail(409, 'space.slug_taken', `The space slug "${next.slug}" is taken`);
if (q.spaceBySlug.get(next.slug)) fail(409, 'space.slug_taken', `The space slug "${next.slug}" is taken`);
+ checkSlugNotRetired(next.slug, space.id);
redirects.recordMove(space.id, `/s/${space.slug}`, `/s/${next.slug}`, { reason: 'space_renamed' });
redirects.recordMove(space.id, `/s/${space.slug}`, `/s/${next.slug}`, { reason: 'space_renamed' });
for (const p of q.pagesOfSpace.all(space.id)) redirects.recordMove(p.id, `/w/${space.slug}/${p.slug}`, `/w/${next.slug}/${p.slug}`, { reason: 'space_renamed' });
for (const p of q.pagesOfSpace.all(space.id)) redirects.recordMove(p.id, `/w/${space.slug}/${p.slug}`, `/w/${next.slug}/${p.slug}`, { reason: 'space_renamed' });
}
}
new file mode 100644
new file mode 100644
@@ -0,0 +1,37 @@
@@ -0,0 +1,37 @@
+'use strict';
+/**
+ * Security regressions: a renamed space's old address stays its redirect; another person cannot
+ * claim it (by creating a space or renaming theirs) and serve their pages at the old links.
+ */
+const assert = require('assert');
+const H = require('./helpers');
+
+(async () => {
+ const h = await H.boot();
+ const owner = { kind: 'user', subject: H.subject(), staff: false };
+ const mallory = { kind: 'user', subject: H.subject(), staff: false };
+ try {
+ const space = h.svc.createSpace({ name: 'Handbook', slug: 'handbook' }, owner);
+ const { page } = h.svc.createPage(space.id, { title: 'Onboarding', body: H.LONG }, owner);
+ h.svc.publish(page.id, {}, owner);
+ h.svc.updateSpace(space.id, { slug: 'guide' }, owner);
+
+ // Another person cannot take the retired slug, by creating a space or by renaming theirs.
+ assert.throws(() => h.svc.createSpace({ name: 'Handbook', slug: 'handbook' }, mallory), (e) => e.status === 409 && e.code === 'space.slug_retired');
+ const own = h.svc.createSpace({ name: 'Mine', slug: 'mine' }, mallory);
+ assert.throws(() => h.svc.updateSpace(own.id, { slug: 'handbook' }, mallory), (e) => e.status === 409 && e.code === 'space.slug_retired');
+ const r = await H.req(h, 'POST', '/api/v1/spaces', { token: H.userToken({ subject: mallory.subject }), body: { name: 'Handbook', slug: 'handbook' } });
+ assert.strictEqual(r.status, 409);
+
+ // The old links still lead to the real space and page.
+ assert.strictEqual((await H.req(h, 'GET', '/s/handbook')).headers.get('location'), '/s/guide');
+ assert.strictEqual((await H.req(h, 'GET', '/w/handbook/onboarding')).headers.get('location'), '/w/guide/onboarding');
+
+ // The space itself may take its old slug back.
+ h.svc.updateSpace(space.id, { slug: 'handbook' }, owner);
+ assert.strictEqual((await H.req(h, 'GET', '/w/handbook/onboarding')).status, 200);
+ console.log('security: ok');
+ } finally {
+ await h.stop();
+ }
+})().catch((e) => { console.error(e); process.exit(1); });