From 3a762a50ed63df0106d7552bde5fdfc3f64f15d9 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 13 Jul 2021 16:05:43 +0000 Subject: [PATCH] Implement path traversal safeguards for image uploads as well - consolidate safeguards in utils.js --- scripts/s_whiteboard.js | 29 +++++++++++++++-------------- scripts/server-backend.js | 5 +++-- scripts/utils.js | 37 ++++++++++++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/scripts/s_whiteboard.js b/scripts/s_whiteboard.js index 2e2d43f..97ef715 100644 --- a/scripts/s_whiteboard.js +++ b/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) { diff --git a/scripts/server-backend.js b/scripts/server-backend.js index b3b2a27..56ebf67 100644 --- a/scripts/server-backend.js +++ b/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); diff --git a/scripts/utils.js b/scripts/utils.js index d18a01b..31e8c6c 100644 --- a/scripts/utils.js +++ b/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, +};