From 5409eb314c4e072b9760b8d30b985fa0bb96a006 Mon Sep 17 00:00:00 2001 From: Tom Hunkapiller Date: Fri, 10 Apr 2015 19:25:52 -0500 Subject: [PATCH 1/3] fix an issue in the path handling that allowed directory traversal --- src/node/hooks/express/tests.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/node/hooks/express/tests.js b/src/node/hooks/express/tests.js index 3157d68e..dfe02c2a 100644 --- a/src/node/hooks/express/tests.js +++ b/src/node/hooks/express/tests.js @@ -23,6 +23,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { }); + var rootTestFolder = path.join(npm.root, "..", "/tests/frontend/"); var url2FilePath = function(url){ var subPath = url.substr("/tests/frontend".length); if (subPath == ""){ @@ -30,8 +31,11 @@ exports.expressCreateServer = function (hook_name, args, cb) { } subPath = subPath.split("?")[0]; - var filePath = path.normalize(npm.root + "/../tests/frontend/") - filePath += subPath.replace("..", ""); + var filePath = path.normalize(path.join(rootTestFolder, subPath)); + // make sure we jail the paths to the test folder, otherwise serve index + if (filePath.indexOf(rootTestFolder) !== 0) { + filePath = path.normalize(path.join(rootTestFolder, "index.html")); + } return filePath; } From 0b0a34908197a0a58c5d16765661000bd24cb39c Mon Sep 17 00:00:00 2001 From: Tom Hunkapiller Date: Fri, 10 Apr 2015 20:19:26 -0500 Subject: [PATCH 2/3] clearer comments about the path handling behavior --- src/node/hooks/express/tests.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/node/hooks/express/tests.js b/src/node/hooks/express/tests.js index dfe02c2a..151c99fa 100644 --- a/src/node/hooks/express/tests.js +++ b/src/node/hooks/express/tests.js @@ -23,7 +23,10 @@ exports.expressCreateServer = function (hook_name, args, cb) { }); - var rootTestFolder = path.join(npm.root, "..", "/tests/frontend/"); + + // path.join seems to normalize by default, but we'll just be explicit + var rootTestFolder = path.normalize(path.join(npm.root, "../tests/frontend/")); + var url2FilePath = function(url){ var subPath = url.substr("/tests/frontend".length); if (subPath == ""){ @@ -34,7 +37,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { var filePath = path.normalize(path.join(rootTestFolder, subPath)); // make sure we jail the paths to the test folder, otherwise serve index if (filePath.indexOf(rootTestFolder) !== 0) { - filePath = path.normalize(path.join(rootTestFolder, "index.html")); + filePath = path.join(rootTestFolder, "index.html"); } return filePath; } From 1b9a51c879bfa33f5dc509fa923c202ccdd60c57 Mon Sep 17 00:00:00 2001 From: Stefan Date: Sat, 11 Apr 2015 10:19:02 +0200 Subject: [PATCH 3/3] Release version 1.5.4 --- CHANGELOG.md | 3 +++ src/package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d06aa87..926e3d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 1.5.4 + * SECURITY: Also don't allow read files on directory traversal on frontend tests path + # 1.5.3 * NEW: Accessibility support for Screen readers, includes new fonts and keyboard shortcuts * NEW: API endpoint for Append Chat Message and Chat Backend Tests diff --git a/src/package.json b/src/package.json index 4e3c6f24..ed9ba957 100644 --- a/src/package.json +++ b/src/package.json @@ -54,5 +54,5 @@ "repository" : { "type" : "git", "url" : "http://github.com/ether/etherpad-lite.git" }, - "version" : "1.5.3" + "version" : "1.5.4" }