diff --git a/src/node/utils/Minify.js b/src/node/utils/Minify.js index b4d5c8514..02728824d 100644 --- a/src/node/utils/Minify.js +++ b/src/node/utils/Minify.js @@ -29,6 +29,7 @@ const RequireKernel = require('etherpad-require-kernel'); const mime = require('mime-types'); const Threads = require('threads'); const log4js = require('log4js'); +const sanitizePathname = require('./sanitizePathname'); const logger = log4js.getLogger('Minify'); @@ -104,26 +105,6 @@ const requestURIs = (locations, method, headers, callback) => { }); }; -// Normalizes p and ensures that it is a relative path that does not reach outside. See -// https://nvd.nist.gov/vuln/detail/CVE-2015-3297 for additional context. -const sanitizePathname = (p, pathApi = path) => { - // The documentation for path.normalize() says that it resolves '..' and '.' segments. The word - // "resolve" implies that it examines the filesystem to resolve symbolic links, so 'a/../b' might - // not be the same thing as 'b'. Most path normalization functions from other libraries (e.g., - // Python's os.path.normpath()) clearly state that they do not examine the filesystem. Here we - // assume Node.js's path.normalize() does the same; that it is only a simple string manipulation. - p = pathApi.normalize(p); - if (pathApi.isAbsolute(p)) throw new Error(`absolute paths are forbidden: ${p}`); - if (p.split(pathApi.sep)[0] === '..') throw new Error(`directory traversal: ${p}`); - // On Windows, path normalization replaces forwardslashes with backslashes. Convert them back to - // forwardslashes. Node.js treats both the backlash and the forwardslash characters as pathname - // component separators on Windows so this does not change the meaning of the pathname on Windows. - // THIS CONVERSION MUST ONLY BE DONE ON WINDOWS, otherwise on POSIXish systems '..\\' in the input - // pathname would not be normalized away before being converted to '../'. - if (pathApi.sep === '\\') p = p.replace(/\\/g, '/'); - return p; -}; - const compatPaths = { 'js/browser.js': 'js/vendors/browser.js', 'js/farbtastic.js': 'js/vendors/farbtastic.js', @@ -340,7 +321,3 @@ exports.requestURIs = requestURIs; exports.shutdown = async (hookName, context) => { await threadsPool.terminate(); }; - -exports.exportedForTestingOnly = { - sanitizePathname, -}; diff --git a/src/node/utils/sanitizePathname.js b/src/node/utils/sanitizePathname.js new file mode 100644 index 000000000..61b611166 --- /dev/null +++ b/src/node/utils/sanitizePathname.js @@ -0,0 +1,23 @@ +'use strict'; + +const path = require('path'); + +// Normalizes p and ensures that it is a relative path that does not reach outside. See +// https://nvd.nist.gov/vuln/detail/CVE-2015-3297 for additional context. +module.exports = (p, pathApi = path) => { + // The documentation for path.normalize() says that it resolves '..' and '.' segments. The word + // "resolve" implies that it examines the filesystem to resolve symbolic links, so 'a/../b' might + // not be the same thing as 'b'. Most path normalization functions from other libraries (e.g., + // Python's os.path.normpath()) clearly state that they do not examine the filesystem. Here we + // assume Node.js's path.normalize() does the same; that it is only a simple string manipulation. + p = pathApi.normalize(p); + if (pathApi.isAbsolute(p)) throw new Error(`absolute paths are forbidden: ${p}`); + if (p.split(pathApi.sep)[0] === '..') throw new Error(`directory traversal: ${p}`); + // On Windows, path normalization replaces forwardslashes with backslashes. Convert them back to + // forwardslashes. Node.js treats both the backlash and the forwardslash characters as pathname + // component separators on Windows so this does not change the meaning of the pathname on Windows. + // THIS CONVERSION MUST ONLY BE DONE ON WINDOWS, otherwise on POSIXish systems '..\\' in the input + // pathname would not be normalized away before being converted to '../'. + if (pathApi.sep === '\\') p = p.replace(/\\/g, '/'); + return p; +}; diff --git a/src/tests/backend/specs/Minify.js b/src/tests/backend/specs/sanitizePathname.js similarity index 96% rename from src/tests/backend/specs/Minify.js rename to src/tests/backend/specs/sanitizePathname.js index 5834bb94e..767221920 100644 --- a/src/tests/backend/specs/Minify.js +++ b/src/tests/backend/specs/sanitizePathname.js @@ -1,10 +1,8 @@ 'use strict'; -const Minify = require('../../../node/utils/Minify'); const assert = require('assert').strict; const path = require('path'); - -const {sanitizePathname} = Minify.exportedForTestingOnly; +const sanitizePathname = require('../../../node/utils/sanitizePathname'); describe(__filename, function () { describe('absolute paths rejected', function () {