openvibers/openvibe.vip Commits

Security: checkout return URLs stay on this site; page notices are signed

OP OpenVibers committed 23 Sept 2026 · 07ba885e9d · 3 files, +96 −2

POST /api/v1/checkout passed a person's success_url/cancel_url straight to Billing, which
hands them to the payment provider: a provider-hosted checkout link became an open redirect
to any look-alike page. A user principal may now only name pages of BASE_URL (services, which
hold vip.membership.checkout, are unchanged).

Pages rendered any ?ok= / ?error= text from the query string, so a crafted openvibe.vip link
could show words of the sender's choosing on a creator's page. VIP's own redirects now carry an
HMAC of the notice (VIP_FORM_SECRET) and unsigned notices are dropped.

test/security.test.js proves both.

Changes

ddiff --git a/server/api/v1.js b/server/api/v1.js
iindex afce6f6..8135e92 100644
---- a/server/api/v1.js
++++ b/server/api/v1.js
@@@ -186,11 +186,26 @@ function v1Router({ domain, apiAuth }) {
}));
// ── Checkout hand-off (Billing) ──────────────────────────
++ const ownOrigin = (() => { try { return new URL(domain.config.baseUrl).origin; } catch { return null; } })();
++ /**
++ * A return URL a person asked for: only pages of this site. Billing hands these to the payment
++ * provider, which sends the buyer there after paying or cancelling — anything else would make
++ * a provider-hosted checkout link an open redirect to a look-alike page.
++ */
++ function returnUrl(req, v, field) {
++ if (v == null || v === '') return undefined;
++ if (req.principal.kind === 'service') return String(v);
++ let u;
++ try { u = new URL(String(v)); } catch { u = null; }
++ if (!u || !ownOrigin || u.origin !== ownOrigin) fail(422, 'vip.invalid_input', `${field} must be a page of ${ownOrigin || 'this site'}`);
++ return u.toString();
++ }
r.post('/checkout', wrap(async (req, res) => {
const b = req.body || {};
const member = memberSubject(req, b.subject, CAP.checkout);
const out = await checkout.start({
-- member, planId: b.plan_id, provider: b.provider, successUrl: b.success_url, cancelUrl: b.cancel_url,
++ member, planId: b.plan_id, provider: b.provider,
++ successUrl: returnUrl(req, b.success_url, 'success_url'), cancelUrl: returnUrl(req, b.cancel_url, 'cancel_url'),
autoRenew: b.auto_renew === undefined ? true : bool(b.auto_renew), traceparent: trace(req),
});
res.status(201).json(out);
ddiff --git a/server/web/routes.js b/server/web/routes.js
iindex 6e54328..db8a12e 100644
---- a/server/web/routes.js
++++ b/server/web/routes.js
@@@ -13,6 +13,7 @@
* Every POST needs the signed-in viewer and the form's anti-forgery token; it acts through the same
* domain calls as the API and redirects back with a notice (?ok= / ?error=).
*/
++const crypto = require('crypto');
const express = require('express');
const { VipError, bool } = require('../util');
const { viewerMiddleware } = require('./session');
@@@ -38,7 +39,20 @@ function createWebRoutes({ domain, config, layout, userAuth }) {
const send = (res, status, o) => res.status(status).type('html').send(layout.page(o));
const notFound = (req, res) => send(res, 404, { title: 'Not found', robots: 'noindex', viewer: req.viewer, body: pages.errorPage({ status: 404, title: 'Nothing here', message: 'That page does not exist.' }) });
-- const back = (res, path, kind, msg) => res.redirect(303, `${path}${path.includes('?') ? '&' : '?'}${kind}=${encodeURIComponent(msg)}`);
++ // Notices ride in the redirect (?ok= / ?error=) and carry an HMAC of their text, so a crafted
++ // link cannot put words of its choosing on a VIP page (e.g. "send your Vibes to …").
++ const noticeSig = (kind, msg) => crypto.createHmac('sha256', String(config.formSecret || '')).update(`notice|${kind}|${msg}`).digest('base64url').slice(0, 22);
++ const back = (res, path, kind, msg) => res.redirect(303, `${path}${path.includes('?') ? '&' : '?'}${kind}=${encodeURIComponent(msg)}&ns=${noticeSig(kind, msg)}`);
++ router.use((req, res, next) => {
++ for (const kind of ['ok', 'error']) {
++ const msg = req.query[kind];
++ if (msg === undefined) continue;
++ const sig = String(req.query.ns || '');
++ const want = typeof msg === 'string' && config.formSecret ? noticeSig(kind, msg) : null;
++ if (!want || sig.length !== want.length || !crypto.timingSafeEqual(Buffer.from(sig), Buffer.from(want))) delete req.query[kind];
++ }
++ next();
++ });
const list = (v) => (v == null ? [] : Array.isArray(v) ? v : [v]).map(String).filter(Boolean);
const lines = (v) => String(v || '').split('\n').map((s) => s.trim()).filter(Boolean);
ddiff --git a/test/security.test.js b/test/security.test.js
nnew file mode 100644
iindex 0000000..e8f5aeb
---- /dev/null
++++ b/test/security.test.js
@@@ -0,0 +1,65 @@
++'use strict';
++/**
++ * Security review regressions (Track S):
++ * - a person's checkout cannot hand Billing (and so the payment provider) a return URL off this
++ * site: a provider-hosted checkout link must not become an open redirect;
++ * - a crafted ?ok= / ?error= link cannot put text of its choosing on a VIP page; VIP's own
++ * redirects still show their notices.
++ */
++const assert = require('assert');
++const { boot, harness } = require('./helpers/app');
++
++const { test, run } = harness('security');
++
++(async () => {
++ const t = await boot();
++ const creator = t.network.newUser('sec_creator', { role: 'streamer' });
++ const buyer = t.network.newUser('sec_buyer');
++ let plan;
++ const intentBodies = () => t.billing.calls.filter((c) => c.method === 'POST' && c.path === '/api/v1/intents').map((c) => c.body);
++
++ test('setup: a published plan', async () => {
++ plan = (await t.call('POST', '/api/v1/plans', { user: creator, body: { name: 'Club', description: 'Members', benefits: ['Badge'], publish: true } })).json.plan;
++ assert.ok(plan && plan.purchasable);
++ });
++
++ test('a person cannot send the checkout\'s return URLs off-site', async () => {
++ const before = intentBodies().length;
++ for (const [field, url] of [['success_url', 'https://evil.example/phish'], ['cancel_url', 'https://evil.example/phish'], ['success_url', 'javascript:alert(1)'], ['cancel_url', '//evil.example/x']]) {
++ const r = await t.call('POST', '/api/v1/checkout', { user: buyer, body: { plan_id: plan.id, provider: 'stripe', [field]: url } });
++ assert.strictEqual(r.status, 422, `${field}=${url} → ${r.status} ${r.text}`);
++ }
++ assert.strictEqual(intentBodies().length, before, 'nothing reached Billing');
++ });
++
++ test('same-site return URLs and the defaults still work', async () => {
++ const r = await t.call('POST', '/api/v1/checkout', { user: buyer, body: { plan_id: plan.id, provider: 'stripe', success_url: 'http://vip.test/me?joined=1', cancel_url: 'http://vip.test/sec_creator' } });
++ assert.strictEqual(r.status, 201, r.text);
++ const sent = intentBodies().pop();
++ assert.strictEqual(sent.success_url, 'http://vip.test/me?joined=1');
++ assert.strictEqual(sent.cancel_url, 'http://vip.test/sec_creator');
++ const d = await t.call('POST', '/api/v1/checkout', { user: buyer, body: { plan_id: plan.id, provider: 'stripe' } });
++ assert.strictEqual(d.status, 201, d.text);
++ assert.match(intentBodies().pop().success_url, /^http:\/\/vip\.test\//);
++ });
++
++ test('a crafted ?ok= / ?error= link shows nothing', async () => {
++ const spoof = 'Your membership was suspended — send 5000 Vibes to @scammer to restore it';
++ for (const kind of ['ok', 'error']) {
++ const p = await t.page(`/sec_creator?${kind}=${encodeURIComponent(spoof)}`);
++ assert.strictEqual(p.status, 200);
++ assert.ok(!p.text.includes('send 5000 Vibes'), `${kind} notice rendered from the query string`);
++ const forged = await t.page(`/me?${kind}=${encodeURIComponent(spoof)}&ns=AAAAAAAAAAAAAAAAAAAAAA`, { user: buyer });
++ assert.ok(!forged.text.includes('send 5000 Vibes'), `${kind} notice rendered with a forged signature`);
++ }
++ });
++
++ test('VIP\'s own notices still render', async () => {
++ const r = await t.page('/sec_creator/join', { user: buyer, method: 'POST', form: { plan_id: plan.id, provider: 'stripe', _csrf: 'stale' } });
++ assert.strictEqual(r.status, 303);
++ const shown = await t.page(r.location, { user: buyer });
++ assert.match(shown.text, /That form expired/);
++ });
++
++ await run();
++})().catch((e) => { console.error(e); process.exit(1); });