From 810e8dbbbfd2bdf5c4e9ae2c4346af0b723896cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Mon, 10 Mar 2025 15:51:43 -0400 Subject: [PATCH 1/4] fix: sanitize category svg image files --- src/controllers/admin/uploads.js | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index fc6ee9c1f1..e1ad938902 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -3,6 +3,7 @@ const path = require('path'); const nconf = require('nconf'); const fs = require('fs'); +const sanitizeHtml = require('sanitize-html'); const meta = require('../../meta'); const posts = require('../../posts'); @@ -121,11 +122,50 @@ uploadsController.uploadCategoryPicture = async function (req, res, next) { return next(new Error('[[error:invalid-json]]')); } + if (uploadedFile.path.endsWith('.svg')) { + await sanitizeSvg(uploadedFile.path); + } + await validateUpload(uploadedFile, allowedImageTypes); const filename = `category-${params.cid}${path.extname(uploadedFile.name)}`; await uploadImage(filename, 'category', uploadedFile, req, res, next); }; +async function sanitizeSvg(filePath) { + const dirty = await fs.promises.readFile(filePath, 'utf8'); + const clean = sanitizeHtml(dirty, { + allowedTags: [ + 'svg', 'g', 'defs', 'linearGradient', 'radialGradient', 'stop', + 'circle', 'ellipse', 'polygon', 'polyline', 'path', 'rect', + 'line', 'text', 'tspan', 'use', 'symbol', 'clipPath', 'mask', 'pattern', + 'filter', 'feGaussianBlur', 'feOffset', 'feBlend', 'feColorMatrix', 'feMerge', 'feMergeNode', + ], + allowedAttributes: { + '*': [ + // Geometry + 'x', 'y', 'x1', 'x2', 'y1', 'y2', 'cx', 'cy', 'r', 'rx', 'ry', + 'width', 'height', 'd', 'points', 'viewBox', 'transform', + + // Presentation + 'fill', 'stroke', 'stroke-width', 'opacity', + 'stop-color', 'stop-opacity', 'offset', 'style', 'class', + + // Text + 'text-anchor', 'font-size', 'font-family', + + // Misc + 'id', 'clip-path', 'mask', 'filter', 'gradientUnits', 'gradientTransform', + 'xmlns', 'preserveAspectRatio', + ], + }, + parser: { + lowerCaseTags: false, + lowerCaseAttributeNames: false, + }, + }); + await fs.promises.writeFile(filePath, clean); +} + uploadsController.uploadFavicon = async function (req, res, next) { const uploadedFile = req.files.files[0]; const allowedTypes = ['image/x-icon', 'image/vnd.microsoft.icon']; From 6d74ee2f59dfaaf3a3eebf4f67bc36d88e361d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Mon, 10 Mar 2025 16:20:51 -0400 Subject: [PATCH 2/4] refactor: show simple error if path doesn't exist --- src/controllers/admin/uploads.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index e1ad938902..ccdf1ed0f9 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -3,6 +3,7 @@ const path = require('path'); const nconf = require('nconf'); const fs = require('fs'); +const winston = require('winston'); const sanitizeHtml = require('sanitize-html'); const meta = require('../../meta'); @@ -23,8 +24,14 @@ uploadsController.get = async function (req, res, next) { } const itemsPerPage = 20; const page = parseInt(req.query.page, 10) || 1; + let files = []; + try { + files = await fs.promises.readdir(currentFolder); + } catch (err) { + winston.error(err.stack); + return next(new Error('[[error:invalid-path]]')); + } try { - let files = await fs.promises.readdir(currentFolder); files = files.filter(filename => filename !== '.gitignore'); const itemCount = files.length; const start = Math.max(0, (page - 1) * itemsPerPage); From 76896859faba63eee24f1f51bb68035a4d215471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Mon, 10 Mar 2025 16:49:40 -0400 Subject: [PATCH 3/4] fix: check if folder exists when uploading files in acp --- src/controllers/admin/uploads.js | 3 +++ test/uploads.js | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index ccdf1ed0f9..bb777c2f31 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -244,6 +244,9 @@ uploadsController.uploadFile = async function (req, res, next) { return next(new Error('[[error:invalid-json]]')); } + if (!await file.exists(path.join(nconf.get('upload_path'), params.folder))) { + return next(new Error('[[error:invalid-path]]')); + } try { const data = await file.saveFileToLocal(uploadedFile.name, params.folder, uploadedFile.path); res.json([{ url: data.url }]); diff --git a/test/uploads.js b/test/uploads.js index a8e48afac5..76148d25d2 100644 --- a/test/uploads.js +++ b/test/uploads.js @@ -400,6 +400,17 @@ describe('Upload Controllers', () => { assert.strictEqual(body.error, '[[error:invalid-path]]'); }); + it('should fail to upload regular file if directory does not exist', async () => { + const { response, body } = await helpers.uploadFile(`${nconf.get('url')}/api/admin/upload/file`, path.join(__dirname, '../test/files/test.png'), { + params: JSON.stringify({ + folder: 'does-not-exist', + }), + }, jar, csrf_token); + + assert.equal(response.statusCode, 500); + assert.strictEqual(body.error, '[[error:invalid-path]]'); + }); + describe('ACP uploads screen', () => { it('should create a folder', async () => { const { response } = await helpers.createFolder('', 'myfolder', jar, csrf_token); From e775564fc1e728504c008c314aa98ee28e91caae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Mon, 10 Mar 2025 17:59:31 -0400 Subject: [PATCH 4/4] refactor: prevent following symlinks --- src/controllers/admin/uploads.js | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/controllers/admin/uploads.js b/src/controllers/admin/uploads.js index bb777c2f31..ced7385983 100644 --- a/src/controllers/admin/uploads.js +++ b/src/controllers/admin/uploads.js @@ -26,13 +26,13 @@ uploadsController.get = async function (req, res, next) { const page = parseInt(req.query.page, 10) || 1; let files = []; try { - files = await fs.promises.readdir(currentFolder); + await checkSymLinks(req.query.dir) + files = await getFilesInFolder(currentFolder); } catch (err) { winston.error(err.stack); return next(new Error('[[error:invalid-path]]')); } try { - files = files.filter(filename => filename !== '.gitignore'); const itemCount = files.length; const start = Math.max(0, (page - 1) * itemsPerPage); const stop = start + itemsPerPage; @@ -72,6 +72,30 @@ uploadsController.get = async function (req, res, next) { } }; +async function checkSymLinks(folder) { + let dir = path.normalize(folder || ''); + while (dir.length && dir !== '.') { + const nextPath = path.join(nconf.get('upload_path'), dir); + // eslint-disable-next-line no-await-in-loop + const stat = await fs.promises.lstat(nextPath); + if (stat.isSymbolicLink()) { + throw new Error('[[invalid-path]]'); + } + dir = path.dirname(dir); + } +} + +async function getFilesInFolder(folder) { + const dirents = await fs.promises.readdir(folder, { withFileTypes: true }); + const files = []; + for await (const dirent of dirents) { + if (!dirent.isSymbolicLink() && dirent.name !== '.gitignore') { + files.push(dirent.name); + } + } + return files; +} + function buildBreadcrumbs(currentFolder) { const crumbs = []; const parts = currentFolder.replace(nconf.get('upload_path'), '').split(path.sep); @@ -102,14 +126,14 @@ async function getFileData(currentDir, file) { const stat = await fs.promises.stat(pathToFile); let filesInDir = []; if (stat.isDirectory()) { - filesInDir = await fs.promises.readdir(pathToFile); + filesInDir = await getFilesInFolder(pathToFile); } const url = `${nconf.get('upload_url') + currentDir.replace(nconf.get('upload_path'), '')}/${file}`; return { name: file, path: pathToFile.replace(path.join(nconf.get('upload_path'), '/'), ''), url: url, - fileCount: Math.max(0, filesInDir.length - 1), // ignore .gitignore + fileCount: filesInDir.length, size: stat.size, sizeHumanReadable: `${(stat.size / 1024).toFixed(1)}KiB`, isDirectory: stat.isDirectory(),