sanitizePathname: Move to separate module to facilitate reuse

pull/5050/head
Richard Hansen 2021-05-08 17:40:36 -04:00
parent 926da57e34
commit 0d9476529e
3 changed files with 25 additions and 27 deletions

View File

@ -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,
};

View File

@ -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;
};

View File

@ -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 () {