openvibers/openvibe.media Commits

Media: app assets people uploaded never run as a page (inline only for images, audio, video; nosniff, sandbox CSP); thumbnail ids cannot name a path (#62)

OP OpenVibers committed 10 Oct 2026 · 6bed1daa87 · 4 files, +87 −1

From an independent security review of Media.

- **HIGH: bytes people chose could run as a page on openvibe.media.** `GET /a/:id` (app assets: streamers' emotes and sounds, uploaded through an app) served the uploader's Content-Type with no `nosniff` and no Content-Disposition. An "emote" uploaded as HTML or SVG would have run on this origin. Now:
  - only images (never SVG), audio and video are inline; anything else downloads;
  - every asset carries `nosniff` and a sandboxing CSP (`default-src 'none'; sandbox`);
  - every file response from `streamFileWithRange` defaults to `X-Content-Type-Options: nosniff`.

  The Object API v2 path (`/o/:id`), used by MediaHub person uploads and Workshop badges, already did this.
- **MEDIUM: thumbnail path traversal.** A live thumbnail's id becomes a filename (`stream-<app>-<id>.jpg`), and Express decodes `%2f` in route params. An id like `..%2f..%2fx` wrote outside the thumbnails directory. Now:
  - the route accepts only `[A-Za-z0-9_-]{1,64}`;
  - the service refuses anything else and checks that the file stays in the thumbnails directory.

The new `test/user-content-serving.test.js` covers both.

Changes

ddiff --git a/server/public/routes.js b/server/public/routes.js
iindex 6e13d11..b45c46b 100644
---- a/server/public/routes.js
++++ b/server/public/routes.js
@@@ -111,6 +111,7 @@ function streamFileWithRange(req, res, filePath, extraHeaders = {}) {
              'Accept-Ranges': 'bytes',
              'Content-Length': chunkSize,
              'Content-Type': contentType,
++            'X-Content-Type-Options': 'nosniff',
              ...extraHeaders,
          });
          sendFileStream(res, filePath, { start, end: Math.min(end, stat.size - 1) });
@@@ -119,6 +120,7 @@ function streamFileWithRange(req, res, filePath, extraHeaders = {}) {
              'Content-Length': stat.size,
              'Content-Type': contentType,
              'Accept-Ranges': 'bytes',
++            'X-Content-Type-Options': 'nosniff',
              ...extraHeaders,
          });
          sendFileStream(res, filePath);
@@@ -355,8 +357,16 @@ router.get('/a/:id', async (req, res) => {
          const a = await db.getAssetById(parseInt(req.params.id, 10));
          if (!a || !a.file_path || !fs.existsSync(a.file_path)) return res.status(404).json({ error: 'Not found' });
          // Content is replaced under the same URL on re-upload — cache a day, not immutable.
++        // An asset's type is the uploader's word (a streamer's emote or sound, through an app): only images (never
++        // SVG), audio and video are shown inline, anything else downloads, and the bytes never run as a page on this
++        // origin (nosniff, a sandboxing CSP).
++        const mime = a.mime || 'application/octet-stream';
++        const inline = /^(image\/(?!svg)|audio\/|video\/)/i.test(mime);
          streamFileWithRange(req, res, a.file_path, {
--            'Content-Type': a.mime || 'application/octet-stream',
++            'Content-Type': mime,
++            'Content-Disposition': inline ? 'inline' : 'attachment',
++            'Content-Security-Policy': "default-src 'none'; sandbox",
++            'X-Content-Type-Options': 'nosniff',
              'Cache-Control': 'public, max-age=86400',
              'Access-Control-Allow-Origin': '*',
              'X-Robots-Tag': 'noindex',
ddiff --git a/server/thumbnails/routes.js b/server/thumbnails/routes.js
iindex c207cc4..6516c52 100644
---- a/server/thumbnails/routes.js
++++ b/server/thumbnails/routes.js
@@@ -43,6 +43,8 @@ router.post('/:kind/:id', tenantAuth({ verb: 'write', allowUser: true }), upload
      try {
          const kind = String(req.params.kind || '').toLowerCase();
          const id = String(req.params.id);
++        // The id names a file (stream-<app>-<id>.jpg): Express decodes %2f in a route param, so only a plain id passes.
++        if (!/^[A-Za-z0-9_-]{1,64}$/.test(id)) return res.status(400).json({ error: 'Invalid id' });
          const imageData = _imageFromRequest(req);
  
          // ── Live stream thumbnail (upload from the owning app's broadcaster) ──
ddiff --git a/server/thumbnails/thumbnail-service.js b/server/thumbnails/thumbnail-service.js
iindex 89ed273..6a05f24 100644
---- a/server/thumbnails/thumbnail-service.js
++++ b/server/thumbnails/thumbnail-service.js
@@@ -231,8 +231,10 @@ function _removeOldThumb(oldUrl, newUrl) {
   */
  function saveLiveThumbnail(appId, streamId, imageData) {
      try {
++        if (!/^[A-Za-z0-9_-]{1,64}$/.test(String(streamId)) || !/^[A-Za-z0-9_-]{1,64}$/.test(String(appId))) return null;
          const filename = `stream-${appId}-${streamId}.jpg`;
          const outPath = path.join(THUMB_DIR, filename);
++        if (path.dirname(path.resolve(outPath)) !== path.resolve(THUMB_DIR)) return null;
  
          // Rate-limit rewrites (the predecessor's client-write cooldown)
          try {
ddiff --git a/test/user-content-serving.test.js b/test/user-content-serving.test.js
nnew file mode 100644
iindex 0000000..8a344ba
---- /dev/null
++++ b/test/user-content-serving.test.js
@@@ -0,0 +1,72 @@
++'use strict';
++/**
++ * Bytes people chose never run as a page on openvibe.media (security review 2026-10-10):
++ *   - GET /a/:id serves only images (never SVG), audio and video inline; anything else (an "emote" uploaded as HTML)
++ *     downloads, always with nosniff and a sandboxing CSP;
++ *   - every file response carries X-Content-Type-Options: nosniff;
++ *   - a live thumbnail's id names a file, so an id that decodes to a path ("../x") is refused.
++ */
++const assert = require('assert');
++const fs = require('fs');
++const os = require('os');
++const path = require('path');
++const http = require('http');
++const express = require('express');
++
++(async () => {
++    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ov-media-usercontent-'));
++    const dir = (n) => { const d = path.join(tmp, n); fs.mkdirSync(d, { recursive: true }); return d; };
++    Object.assign(process.env, {
++        VOD_PATH: dir('vods'), CLIPS_PATH: dir('clips'), FILES_PATH: dir('files'),
++        THUMBNAILS_PATH: dir('thumbnails'), PASTES_PATH: dir('pastes'), OBJECTS_PATH: dir('objects'), MEDIA_INVARIANT_SCAN_HOURS: '0',
++    });
++    const db = require('../server/db/database');
++    await db.upsertApp({ app_id: 'live', api_key: 'live-key-usercontent' });
++    const files = dir('assets');
++    const asset = async (name, mime, body) => {
++        const p = path.join(files, name);
++        fs.writeFileSync(p, body);
++        return Number((await db.run(`INSERT INTO assets (app_id, kind, name, file_path, mime) VALUES ('live', 'emote', ?, ?, ?) RETURNING id`, [name, p, mime])).lastInsertRowid);
++    };
++    const html = await asset('evil.html', 'text/html', '<script>alert(document.domain)</script>');
++    const svg = await asset('evil.svg', 'image/svg+xml', '<svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)"/>');
++    const png = await asset('ok.png', 'image/png', Buffer.from('89504e470d0a1a0a', 'hex'));
++
++    const app = express();
++    app.use(express.json());
++    app.use('/', require('../server/public/routes'));
++    app.use('/api/v1/:app/thumbnails', require('../server/thumbnails/routes'));
++    const server = app.listen(0, '127.0.0.1');
++    await new Promise((r) => server.once('listening', r));
++    const base = `http://127.0.0.1:${server.address().port}`;
++    try {
++        let r = await fetch(`${base}/a/${html}`);
++        assert.strictEqual(r.status, 200);
++        assert.strictEqual(r.headers.get('content-disposition'), 'attachment', 'HTML downloads');
++        assert.strictEqual(r.headers.get('x-content-type-options'), 'nosniff');
++        assert.match(r.headers.get('content-security-policy') || '', /sandbox/);
++        r = await fetch(`${base}/a/${svg}`);
++        assert.strictEqual(r.headers.get('content-disposition'), 'attachment', 'SVG is never inline');
++        r = await fetch(`${base}/a/${png}`);
++        assert.strictEqual(r.headers.get('content-disposition'), 'inline', 'an image is shown inline');
++        assert.strictEqual(r.headers.get('x-content-type-options'), 'nosniff');
++
++        const post = (id) => new Promise((resolve, reject) => {
++            const body = JSON.stringify({ image: 'data:image/jpeg;base64,/9j/4AAQ' });
++            const req = http.request({ host: '127.0.0.1', port: server.address().port, path: `/api/v1/live/thumbnails/live/${id}`, method: 'POST',
++                headers: { Authorization: 'Bearer live-key-usercontent', 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body) } }, (res) => { res.resume(); res.on('end', () => resolve(res.statusCode)); });
++            req.on('error', reject);
++            req.end(body);
++        });
++        assert.strictEqual(await post('..%2f..%2fescape'), 400, 'a path in the id is refused');
++        assert.strictEqual(await post('a%2fb'), 400);
++        const thumbs = require('../server/thumbnails/thumbnail-service');
++        assert.strictEqual(thumbs.saveLiveThumbnail('live', '../../escape', Buffer.from('x')), null, 'the service refuses it too');
++        assert.ok(!fs.existsSync(path.join(tmp, 'escape.jpg')) && !fs.readdirSync(tmp).some((f) => f.includes('escape')));
++        console.log('user content serving: /a/ downloads anything but images, audio and video; nosniff everywhere; thumbnail ids are plain');
++    } finally {
++        server.close();
++        fs.rmSync(tmp, { recursive: true, force: true });
++    }
++    process.exit(0);
++})().catch((err) => { console.error(err); process.exit(1); });