Merge pull request #108 from lightswitch05/security/more-filesystem-path-checking

Implement path traversal safeguards for image uploads as well - consolidate safeguards in utils.js

scripts/s_whiteboard.js

@@ -1,7 +1,7 @@
 //This file is only for saving the whiteboard.
 const fs = require("fs");
-const path = require("path");
 const config = require("./config/config");
+const { getSafeFilePath } = require("./utils");
 const FILE_DATABASE_FOLDER = "savedBoards";
 
 var savedBoards = {};
@@ -10,12 +10,22 @@ var saveDelay = {};
 
 if (config.backend.enableFileDatabase) {
     // make sure that folder with saved boards exists
-    fs.mkdirSync("savedBoards", {
+    fs.mkdirSync(FILE_DATABASE_FOLDER, {
         // this option also mutes an error if path exists
         recursive: true,
     });
 }
 
+/**
+ * Get the file path for a whiteboard.
+ * @param {string} wid Whiteboard id to get the path for
+ * @returns {string} File path to the whiteboard
+ * @throws {Error} if wid contains potentially unsafe directory characters
+ */
+function fileDatabasePath(wid) {
+    return getSafeFilePath(FILE_DATABASE_FOLDER, wid + ".json");
+}
+
 module.exports = {
     handleEventsAndData: function (content) {
         var tool = content["t"]; //Tool witch is used
@@ -26,7 +36,7 @@ module.exports = {
             delete savedBoards[wid];
             delete savedUndos[wid];
             // delete the corresponding file too
-            fs.unlink("savedBoards/" + wid + ".json", function (err) {
+            fs.unlink(fileDatabasePath(wid), function (err) {
                 if (err) {
                     return console.log(err);
                 }
@@ -122,7 +132,7 @@ module.exports = {
                     saveDelay[wid] = false;
                     if (savedBoards[wid]) {
                         fs.writeFile(
-                            "savedBoards/" + wid + ".json",
+                            fileDatabasePath(wid),
                             JSON.stringify(savedBoards[wid]),
                             (err) => {
                                 if (err) {
@@ -146,16 +156,7 @@ module.exports = {
         // try to load from DB
         if (config.backend.enableFileDatabase) {
             //read saved board from file
-            var fileName = wid + ".json";
-            var filePath = FILE_DATABASE_FOLDER + "/" + fileName;
-            if (
-                path.dirname(filePath) !== FILE_DATABASE_FOLDER ||
-                path.basename(fileName) !== fileName
-            ) {
-                var errorMessage = "Attempted path traversal attack: ";
-                console.log(errorMessage, filePath);
-                throw new Error(errorMessage + filePath);
-            }
+            var filePath = fileDatabasePath(wid);
             if (fs.existsSync(filePath)) {
                 var data = fs.readFileSync(filePath);
                 if (data) {

scripts/server-backend.js

@@ -3,6 +3,7 @@ const path = require("path");
 const config = require("./config/config");
 const ReadOnlyBackendService = require("./services/ReadOnlyBackendService");
 const WhiteboardInfoBackendService = require("./services/WhiteboardInfoBackendService");
+const { getSafeFilePath } = require("./utils");
 
 function startBackendServer(port) {
     var fs = require("fs-extra");
@@ -225,7 +226,7 @@ function startBackendServer(port) {
             webdavaccess = false;
         }
 
-        const savingDir = path.join("./public/uploads", readOnlyWid);
+        const savingDir = getSafeFilePath("public/uploads", readOnlyWid);
         fs.ensureDir(savingDir, function (err) {
             if (err) {
                 console.log("Could not create upload folder!", err);
@@ -238,7 +239,7 @@ function startBackendServer(port) {
                     .replace(/^data:image\/png;base64,/, "")
                     .replace(/^data:image\/jpeg;base64,/, "");
                 console.log(filename, "uploaded");
-                const savingPath = path.join(savingDir, filename);
+                const savingPath = getSafeFilePath(savingDir, filename);
                 fs.writeFile(savingPath, imagedata, "base64", function (err) {
                     if (err) {
                         console.log("error", err);

scripts/utils.js

@@ -1,4 +1,6 @@
-function getArgs() {
+const path = require("path");
+
+const getArgs = function () {
     const args = {};
     process.argv.slice(2, process.argv.length).forEach((arg) => {
         // long arg
@@ -15,6 +17,35 @@ function getArgs() {
         }
     });
     return args;
-}
+};
 
-module.exports.getArgs = getArgs;
+/**
+ * Creates a safe filepath given a trusted rootPath and untrusted singleFileSegment.
+ * Prevents directory traversal attacks.
+ *
+ * @param {string} rootPath Root path - can be relative or absolute
+ * @param {string} singleFileSegment A single file or folder segment - it should not have any path information
+ * @return {string} A safe to use path combined of rootPath and singleFileSegment
+ * @throws {Error} If singleFileSegment contains potentially unsafe directory characters or path information
+ */
+const getSafeFilePath = function (rootPath, singleFileSegment) {
+    var filePath = path.join(rootPath, singleFileSegment);
+    if (
+        path.dirname(filePath) !== rootPath ||
+        path.basename(filePath) !== singleFileSegment ||
+        path.normalize(singleFileSegment) !== singleFileSegment
+    ) {
+        var errorMessage = "Attempted path traversal attack: ";
+        console.log(errorMessage, {
+            rootPath: rootPath,
+            singleFileSegment: singleFileSegment,
+        });
+        throw new Error(errorMessage + singleFileSegment);
+    }
+    return filePath;
+};
+
+module.exports = {
+    getArgs,
+    getSafeFilePath,
+};