openvibers/openvibe.live Commits

Four authorization and input-validation fixes from the audit backlog

OP OpenVibers committed 16 Sept 2026 · 523172a917 · 7 files, +249 −20

**Arbitrary file write through a restream destination.** server_url was accepted with nothing but
a trim and a non-empty check, and it becomes ffmpeg's *output* argument. ffmpeg picks its output
protocol from that URL and `-f flv` only fixes the container, so a bare path or a file: URL made
ffmpeg write an FLV wherever the service user can write — overwriting something under public/js/
would have served arbitrary script to every visitor. http: URLs made it talk to any host. Only
rtmp, rtmps and srt with a real host are accepted now, on create, on update and on the values
platforms hand back through OAuth; the manager re-checks before spawning, which also covers rows
written before this existed. Private and loopback hosts stay allowed — restreaming to a box on
your own LAN is a real setup.

**Unauthenticated SSRF through POST /api/media/quote.** The endpoint is optionalAuth, and the
generic branch of normalizeInput hands whatever URL it is given to yt-dlp, which fetches it. That
is a request generator aimed at localhost and at 169.254.169.254. Non-http schemes are now
rejected on entry (new URL() parses file: and gopher: quite happily), and the generic branch
resolves the host and refuses loopback, link-local, RFC1918, CGNAT and the IPv6 equivalents
including v4-mapped forms. It resolves and checks rather than pinning the address the later fetch
uses, so DNS rebinding is not covered — that needs connecting by IP with an explicit Host header,
which yt-dlp does not expose.

**Banned users could still run chat commands.** handleChatMessage dispatched "!" and "/" before
the ban check, and both paths return early. A banned viewer could queue and skip media on the
stream that banned them, and /me put their text straight back into that chat. The ban check moved
above the dispatch.

**A private VOD's context leaked for two minutes at a time.** The 2-minute context cache was read
before the visibility check: the owner opening their own private VOD warmed it, and every
anonymous request for that id was then served from the cache without ever reaching the check. It
authorises first and reads the cache second, and a private VOD's context now goes out as
`private, no-store` instead of `public, max-age=60`.

Also narrows the HTTP-FLV server's Access-Control-Allow-Origin from `*` to the site origin.
Nothing in a browser talks to that port — the player pulls FLV from the app, which proxies from
127.0.0.1 — so the wildcard bought nothing. It cannot be bound to loopback: node-media-server
2.7.4 calls httpServer.listen(port) with no host.

Verified `include_private` on the channel and VOD listings: already gated on owner, admin or
global moderator. No change needed.

Five regression tests added (19 checks pass); each was confirmed to fail with its fix reverted.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMxJm1dB8rdsaMSgNjVa82

Changes

ddiff --git a/server/chat/chat-server.js b/server/chat/chat-server.js
iindex fab56ee7..1ab5fbf5 100644
---- a/server/chat/chat-server.js
++++ b/server/chat/chat-server.js
@@@ -573,6 +573,21 @@ class ChatServer {
// swallows long messages the channel actually allows.
if (!text || text.length > 6000) return;
++ // ── Ban check ────────────────────────────────────────
++ // This has to come before the command dispatch below, not after it. Both command paths
++ // return early, so a banned viewer could still run every "!" utility command — queueing
++ // and skipping media on the stream they were banned from — and every "/" command,
++ // including /me, which puts their text back in the chat they are banned from. A ban means
++ // no input of any kind.
++ if (client.user && db.isUserBanned(client.user.id, client.streamId)) {
++ this.sendTo(ws, { type: 'system', message: 'You are banned from this chat.' });
++ return;
++ }
++ if (!this._isBanExemptAdmin(client) && db.isIpBanned(client.ip, client.streamId)) {
++ this.sendTo(ws, { type: 'system', message: 'You are banned from this chat.' });
++ return;
++ }
++
// ── Stream utility commands (!sr, !queue, !nowplaying, !skip) ──
if (text.startsWith('!')) {
this.handleBangCommand(ws, client, text);
@@@ -585,16 +600,6 @@ class ChatServer {
return;
}
-- // ── Ban check ────────────────────────────────────────
-- if (client.user && db.isUserBanned(client.user.id, client.streamId)) {
-- this.sendTo(ws, { type: 'system', message: 'You are banned from this chat.' });
-- return;
-- }
-- if (!this._isBanExemptAdmin(client) && db.isIpBanned(client.ip, client.streamId)) {
-- this.sendTo(ws, { type: 'system', message: 'You are banned from this chat.' });
-- return;
-- }
--
// ── IP Approval Mode (Anti-VPN) ─────────────────────
if (client.streamId && client.ip) {
try {
ddiff --git a/server/media-proxy/vods.js b/server/media-proxy/vods.js
iindex edeb6dd5..33bcff4b 100644
---- a/server/media-proxy/vods.js
++++ b/server/media-proxy/vods.js
@@@ -366,12 +366,22 @@ router.get('/:id/context', optionalAuth, async (req, res) => {
try {
if (!/^\d+$/.test(req.params.id)) return res.status(404).json({ error: 'VOD not found' });
const id = parseInt(req.params.id, 10);
-- const hit = _ctxCache.get(id);
-- if (hit && Date.now() - hit.at < 120000) { res.set('Cache-Control', 'public, max-age=60'); return res.json(hit.data); }
++ // Authorise first, then read the cache. The other way round — which is how this read — let
++ // a private VOD's whole context leak: the owner opens their own private VOD, the response
++ // is cached under its id, and for the next two minutes any anonymous request for that id
++ // is served from the cache and never reaches the visibility check below.
let vod = null;
try { vod = await media.getVod(id); } catch { vod = null; }
if (!vod) return res.status(404).json({ error: 'VOD not found' });
-- if (vod.visibility === 'private' && !(req.user && (req.user.id === vod.user_id || req.user.role === 'admin'))) return res.status(404).json({ error: 'VOD not found' });
++ const isPrivate = vod.visibility === 'private';
++ if (isPrivate && !(req.user && (req.user.id === vod.user_id || req.user.role === 'admin'))) {
++ return res.status(404).json({ error: 'VOD not found' });
++ }
++ // A private VOD's context must not sit in a shared cache anywhere on the way back either.
++ const cacheHeader = isPrivate ? 'private, no-store' : 'public, max-age=60';
++
++ const hit = _ctxCache.get(id);
++ if (hit && Date.now() - hit.at < 120000) { res.set('Cache-Control', cacheHeader); return res.json(hit.data); }
const stream = vod.stream_id ? db.getStreamById(vod.stream_id) : null;
const streamer = db.getUserById(vod.user_id || (stream && stream.user_id));
const safe = (fn, d) => { try { const v = fn(); return v == null ? d : v; } catch { return d; } };
@@@ -405,7 +415,7 @@ router.get('/:id/context', optionalAuth, async (req, res) => {
};
_ctxCache.set(id, { at: Date.now(), data });
if (_ctxCache.size > 500) { const k = _ctxCache.keys().next().value; _ctxCache.delete(k); }
-- res.set('Cache-Control', 'public, max-age=60');
++ res.set('Cache-Control', cacheHeader);
res.json(data);
} catch (err) {
console.error('[VODs] context error:', err.message);
ddiff --git a/server/media/media-queue.js b/server/media/media-queue.js
iindex 15831c3b..69287fcc 100644
---- a/server/media/media-queue.js
++++ b/server/media/media-queue.js
@@@ -2,6 +2,66 @@ const db = require('../db/database');
const downloader = require('./media-downloader');
const https = require('https');
const http = require('http');
++const dns = require('dns').promises;
++const net = require('net');
++
++/**
++ * Is this address one the server should never be asked to reach on a stranger's behalf?
++ *
++ * Loopback, link-local (169.254.x, which is where every cloud provider's instance-metadata
++ * service lives), the RFC1918 ranges, CGNAT, and the IPv6 equivalents including v4-mapped forms.
++ */
++function isInternalAddress(ip) {
++ if (!ip) return true;
++ const v = net.isIP(ip);
++ if (v === 4) {
++ const p = ip.split('.').map(Number);
++ if (p[0] === 10 || p[0] === 127 || p[0] === 0) return true;
++ if (p[0] === 169 && p[1] === 254) return true;
++ if (p[0] === 172 && p[1] >= 16 && p[1] <= 31) return true;
++ if (p[0] === 192 && p[1] === 168) return true;
++ if (p[0] === 100 && p[1] >= 64 && p[1] <= 127) return true; // CGNAT
++ if (p[0] >= 224) return true; // multicast / reserved
++ return false;
++ }
++ if (v === 6) {
++ const l = ip.toLowerCase();
++ if (l === '::' || l === '::1') return true;
++ if (l.startsWith('fe80') || l.startsWith('fc') || l.startsWith('fd')) return true;
++ const mapped = l.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/);
++ if (mapped) return isInternalAddress(mapped[1]);
++ return false;
++ }
++ return true;
++}
++
++/**
++ * Refuse to probe a URL that points back inside the network.
++ *
++ * POST /api/media/quote is optionalAuth — anyone on the internet can reach it — and the generic
++ * branch of normalizeInput hands whatever they send to yt-dlp, which will fetch it. Without this
++ * that is an unauthenticated request generator aimed at localhost and at 169.254.169.254.
++ *
++ * Note this resolves the name and checks the answers; it does not pin the address that the later
++ * fetch will use, so a DNS entry that changes between the two (rebinding) is not covered. Closing
++ * that properly means resolving once and connecting by IP with an explicit Host header, which is
++ * not something yt-dlp exposes.
++ */
++async function assertFetchableUrl(url) {
++ if (url.protocol !== 'http:' && url.protocol !== 'https:') {
++ throw new Error('Only http and https links can be looked up');
++ }
++ const host = url.hostname.replace(/^\[|\]$/g, '');
++ if (net.isIP(host)) {
++ if (isInternalAddress(host)) throw new Error('That address is not reachable from here');
++ return;
++ }
++ let addrs = [];
++ try { addrs = await dns.lookup(host, { all: true }); } catch { throw new Error('Could not resolve that host'); }
++ if (!addrs.length || addrs.some(a => isInternalAddress(a.address))) {
++ throw new Error('That address is not reachable from here');
++ }
++}
const DEFAULTS = {
enabled: 1,
@@@ -569,6 +629,11 @@ class MediaQueue {
} catch {
throw new Error('Only direct media URLs are supported right now');
}
++ // new URL() happily parses file:, gopher: and anything else with a scheme. Everything
++ // downstream either fetches this or hands it to a viewer's browser to play.
++ if (url.protocol !== 'http:' && url.protocol !== 'https:') {
++ throw new Error('Only http and https links are supported');
++ }
const hostname = url.hostname.replace(/^www\./i, '').toLowerCase();
const href = url.toString();
@@@ -684,7 +749,14 @@ class MediaQueue {
}
// ── Generic yt-dlp support (SoundCloud, Twitch clips, etc.) ──
++ // This is the only branch that sends a request to a host the caller chose, so it is the
++ // one that has to be checked. YouTube and Vimeo above probe canonical URLs we built.
if (downloader.isAvailable()) {
++ try {
++ await assertFetchableUrl(url);
++ } catch (err) {
++ throw new Error(err.message || 'That link cannot be looked up');
++ }
try {
const info = await downloader.getInfo(href);
return {
ddiff --git a/server/streaming/restream-manager.js b/server/streaming/restream-manager.js
iindex 7ef296c1..bf7f072c 100644
---- a/server/streaming/restream-manager.js
++++ b/server/streaming/restream-manager.js
@@@ -348,6 +348,14 @@ class RestreamManager extends EventEmitter {
*/
_buildDestUrl(dest) {
if (!dest.server_url || !dest.stream_key) return null;
++ // The API validates this on the way in; this is the last gate before the value becomes
++ // ffmpeg's output argument, and it also covers rows written before that validation
++ // existed. ffmpeg chooses its output protocol from the URL, so anything that is not a
++ // live-streaming protocol here means writing a file or opening a socket somewhere.
++ if (!/^(rtmps?|srt):\/\/[^\s/]+/i.test(String(dest.server_url).trim())) {
++ console.warn(`[Restream] Refusing destination ${dest.id}: server_url is not an rtmp/rtmps/srt address`);
++ return null;
++ }
let url = dest.server_url.replace(/\/+$/, '');
// Twitch is much more stable over RTMPS. Normalize old RTMP configs
ddiff --git a/server/streaming/restream-routes.js b/server/streaming/restream-routes.js
iindex 4d14c065..b1aeead8 100644
---- a/server/streaming/restream-routes.js
++++ b/server/streaming/restream-routes.js
@@@ -19,6 +19,36 @@ const VALID_QUALITY_PRESETS = ['auto', 'low', 'medium', 'high', 'ultra', 'source
const VALID_ENCODER_PRESETS = ['ultrafast', 'superfast', 'veryfast', 'faster', 'fast', 'medium', 'slow'];
const MAX_DESTINATIONS = 10;
++/**
++ * Ingest URLs we are willing to hand to ffmpeg.
++ *
++ * server_url arrives from the client and ends up as ffmpeg's *output* argument. ffmpeg picks its
++ * output protocol from that URL, and `-f flv` only fixes the container — so a bare path or a
++ * file: URL makes ffmpeg write an FLV file wherever the service user can write, and an http: URL
++ * makes it talk to whatever host is named. Overwriting something under public/js/ would be enough
++ * to serve arbitrary script to every visitor.
++ *
++ * Nothing but a live-streaming protocol with a real host is accepted. Private and loopback hosts
++ * are deliberately still allowed: restreaming to a box on your own LAN is a legitimate setup, and
++ * the local RTMP server is already reachable to any signed-in user by other means.
++ */
++const ALLOWED_INGEST_PROTOCOLS = new Set(['rtmp:', 'rtmps:', 'srt:']);
++function validateIngestUrl(raw) {
++ const value = String(raw || '').trim();
++ if (!value) return { ok: false, error: 'Server URL is required' };
++ if (value.length > 2048) return { ok: false, error: 'Server URL is too long' };
++ // Control characters and whitespace would let a value split into extra ffmpeg arguments or
++ // smuggle a newline into a log line.
++ if (/[\s\u0000-\u001f\u007f]/.test(value)) return { ok: false, error: 'Server URL contains invalid characters' };
++ let u;
++ try { u = new URL(value); } catch { return { ok: false, error: 'Server URL must be a full rtmp:// or rtmps:// address' }; }
++ if (!ALLOWED_INGEST_PROTOCOLS.has(u.protocol)) {
++ return { ok: false, error: `Server URL must start with rtmp://, rtmps:// or srt:// (got "${u.protocol.replace(':', '')}")` };
++ }
++ if (!u.hostname) return { ok: false, error: 'Server URL needs a hostname' };
++ return { ok: true, value };
++}
++
/** Platform presets with default RTMP server URLs and UI metadata. */
const PLATFORM_PRESETS = {
youtube: { name: 'YouTube', defaultServerUrl: 'rtmp://a.rtmp.youtube.com/live2', icon: 'fa-brands fa-youtube', color: '#ff0000' },
@@@ -170,9 +200,9 @@ router.post('/destinations', requireAuth, (req, res) => {
if (!finalUrl && PLATFORM_PRESETS[platform]?.defaultServerUrl) {
finalUrl = PLATFORM_PRESETS[platform].defaultServerUrl;
}
-- if (!finalUrl) {
-- return res.status(400).json({ error: 'Server URL is required' });
-- }
++ const checked = validateIngestUrl(finalUrl);
++ if (!checked.ok) return res.status(400).json({ error: checked.error });
++ finalUrl = checked.value;
const dest = db.createRestreamDestination(req.user.id, {
platform,
@@@ -214,7 +244,16 @@ router.put('/destinations/:id', requireAuth, (req, res) => {
const updates = {};
if (req.body.name !== undefined) updates.name = req.body.name?.trim() || dest.name;
-- if (req.body.server_url !== undefined) updates.server_url = req.body.server_url?.trim() || dest.server_url;
++ if (req.body.server_url !== undefined) {
++ const raw = req.body.server_url?.trim();
++ if (raw) {
++ const v = validateIngestUrl(raw);
++ if (!v.ok) return res.status(400).json({ error: v.error });
++ updates.server_url = v.value;
++ } else {
++ updates.server_url = dest.server_url;
++ }
++ }
if (req.body.stream_key !== undefined && req.body.stream_key.trim()) {
updates.stream_key = req.body.stream_key.trim();
}
@@@ -593,7 +632,11 @@ router.get('/oauth/:platform/callback', async (req, res) => {
connection_id: conn.id,
};
// Only overwrite ingest fields when the platform actually provides them
-- if (info.server_url) destFields.server_url = info.server_url;
++ if (info.server_url) {
++ const v = validateIngestUrl(info.server_url);
++ if (v.ok) destFields.server_url = v.value;
++ else console.warn(`[Restream OAuth] ${platform} returned an ingest URL we will not use: ${v.error}`);
++ }
if (info.stream_key) destFields.stream_key = info.stream_key;
if (existing) {
ddiff --git a/server/streaming/rtmp-server.js b/server/streaming/rtmp-server.js
iindex aa660dec..1e97be0d 100644
---- a/server/streaming/rtmp-server.js
++++ b/server/streaming/rtmp-server.js
@@@ -43,7 +43,19 @@ class RTMPServer extends EventEmitter {
},
http: {
port: config.rtmp.port + 8000, // HTTP-FLV port (9935 by default)
-- allow_origin: '*', // Public media — CORS open (CSP restricts which pages can load it)
++ // Nothing in a browser talks to this port. The web player pulls FLV from
++ // /api/streams/:id/flv on the main app, which proxies from 127.0.0.1 here after
++ // checking the stream is live and resolving the key; the restreamer's ffmpeg also
++ // reads from 127.0.0.1. Neither sends an Origin header or looks at CORS, so a
++ // wildcard bought nothing and meant any page anywhere could read this directly if
++ // the port were ever reachable.
++ //
++ // node-media-server 2.7.4 calls httpServer.listen(port) with no host, so this
++ // cannot be bound to loopback from config — the port is on every interface by
++ // design of the library. It is not reachable from outside today (the provider
++ // edge drops it), but that is somebody else's firewall, not ours, so the origin
++ // is narrowed here as the part we control.
++ allow_origin: config.baseUrl || 'http://127.0.0.1',
mediaroot: './data/media',
},
// NOTE: NMS trans server crashes on v2.7.4 with 'version is not defined'
ddiff --git a/test/security-redaction.test.js b/test/security-redaction.test.js
iindex e5676181..51d3aad8 100644
---- a/test/security-redaction.test.js
++++ b/test/security-redaction.test.js
@@@ -179,4 +179,83 @@ const ok = (name) => { pass++; console.log(' ok -', name); };
ok('restream OAuth callback will not provision onto another account\'s slot');
}
++// ── server_url becomes ffmpeg's output argument, so it must be a streaming URL ─
++{
++ const src = fs.readFileSync(path.join(ROOT, 'server/streaming/restream-routes.js'), 'utf8');
++ const m = src.match(/function validateIngestUrl\(raw\) \{[\s\S]*?\n\}/);
++ assert(m, 'validateIngestUrl should exist');
++ const protoLine = src.match(/const ALLOWED_INGEST_PROTOCOLS = new Set\(\[[^\]]*\]\)/);
++ assert(protoLine, 'the protocol allowlist should exist');
++ // eslint-disable-next-line no-new-func
++ const validate = new Function(`${protoLine[0]}; ${m[0]}; return validateIngestUrl;`)();
++ for (const good of ['rtmp://a.rtmp.youtube.com/live2', 'rtmps://live.twitch.tv/app', 'srt://ingest.example:9000']) {
++ assert.strictEqual(validate(good).ok, true, `${good} should be accepted`);
++ }
++ for (const bad of [
++ '/opt/openvibe.live/public/js/app.js', // ffmpeg would write an FLV over served script
++ 'file:///etc/passwd',
++ 'http://169.254.169.254/latest/meta-data/',
++ 'pipe:1',
++ 'rtmp://', // no host
++ 'rtmp://host/path\nmore', // control character
++ '',
++ ]) {
++ assert.strictEqual(validate(bad).ok, false, `${bad} must be rejected`);
++ }
++ ok('restream ingest URLs accept only rtmp/rtmps/srt with a host');
++
++ // And the manager refuses anything that slipped in before the API validated it.
++ const mgr = fs.readFileSync(path.join(ROOT, 'server/streaming/restream-manager.js'), 'utf8');
++ const build = mgr.match(/_buildDestUrl\(dest\) \{[\s\S]*?\n \}/);
++ assert(build, '_buildDestUrl should exist');
++ assert(/rtmps\?\|srt/.test(build[0]), '_buildDestUrl must re-check the protocol for rows already stored');
++ ok('the restream manager re-checks the protocol before handing it to ffmpeg');
++}
++
++// ── /api/media/quote is unauthenticated, so it must not probe internal hosts ──
++{
++ const src = fs.readFileSync(path.join(ROOT, 'server/media/media-queue.js'), 'utf8');
++ const fn = src.match(/function isInternalAddress\(ip\) \{[\s\S]*?\n\}/);
++ assert(fn, 'isInternalAddress should exist');
++ // eslint-disable-next-line no-new-func
++ const isInternal = new Function('net', `${fn[0]}; return isInternalAddress;`)(require('net'));
++ for (const blocked of ['127.0.0.1', '169.254.169.254', '10.0.0.1', '172.16.5.5', '192.168.1.1', '100.64.0.1', '::1', 'fd00::1', '::ffff:127.0.0.1']) {
++ assert.strictEqual(isInternal(blocked), true, `${blocked} must count as internal`);
++ }
++ for (const allowed of ['8.8.8.8', '1.1.1.1', '172.32.0.1', '2606:4700::1111']) {
++ assert.strictEqual(isInternal(allowed), false, `${allowed} should be reachable`);
++ }
++ assert(/await assertFetchableUrl\(url\)/.test(src), 'the generic yt-dlp branch must check the URL first');
++ assert(/url\.protocol !== 'http:' && url\.protocol !== 'https:'/.test(src), 'non-http schemes must be rejected on entry');
++ ok('media quote refuses internal addresses and non-http schemes');
++}
++
++// ── A ban has to stop commands too, not just plain messages ──────────────────
++{
++ const src = fs.readFileSync(path.join(ROOT, 'server/chat/chat-server.js'), 'utf8');
++ const fnStart = src.indexOf('handleChatMessage(ws, client, msg) {');
++ assert(fnStart > 0, 'handleChatMessage should exist');
++ const body = src.slice(fnStart, fnStart + 3000);
++ const banAt = body.indexOf('db.isUserBanned(client.user.id, client.streamId)');
++ const bangAt = body.indexOf("text.startsWith('!')");
++ const slashAt = body.indexOf("text.startsWith('/')");
++ assert(banAt > 0 && bangAt > 0 && slashAt > 0, 'all three branches should be present');
++ assert(banAt < bangAt && banAt < slashAt,
++ 'the ban check must run before the "!" and "/" command dispatch, which both return early');
++ ok('banned users cannot run ! or / chat commands');
++}
++
++// ── A private VOD's context must be authorised before the cache is consulted ──
++{
++ const src = fs.readFileSync(path.join(ROOT, 'server/media-proxy/vods.js'), 'utf8');
++ const h = src.match(/router\.get\('\/:id\/context'[\s\S]*?res\.json\(data\);/);
++ assert(h, 'the VOD context route should exist');
++ const checkAt = h[0].indexOf("vod.visibility === 'private'");
++ const cacheAt = h[0].indexOf('_ctxCache.get(id)');
++ assert(checkAt > 0 && cacheAt > 0, 'both the visibility check and the cache read should be present');
++ assert(checkAt < cacheAt, 'the visibility check must run before the cache is read');
++ assert(/private, no-store/.test(h[0]), 'a private VOD context must not be sent with a shareable Cache-Control');
++ ok('private VOD context is authorised before the cache, and never shareable');
++}
++
console.log(`\n${pass} checks passed`);