From 1515ab09be2d98127b5bb6ce6c912c57e28be35a Mon Sep 17 00:00:00 2001 From: Gabriel Ngandu-Biseba Date: Thu, 3 Apr 2025 13:48:53 +0200 Subject: [PATCH] Fix codeQL error ------ Call "tmpdir()" a single time inside "database.js" and remove "caFilePath" from the config if the path starts with the temporary directory path as a failsafe Fix whitespace Move the unassignment of " dbConfig.ssl" and " dbConfig.ssl" outside the block responsible to move the CA file from the temporary directory to the data directory Fix inverted "if" condition. Also add "path.resolve" to a path check in order to make sure that we are comparing absolute paths with each others Remove unnecessary check for a ".pem" file, simplify the path check and fix the file copying to itself Add additional path checks to avoid filename exploits Fix issue where the temp directory used to temporarily store the CA file is within the working directory instead of the OS provided temp directory path --- server/database.js | 21 ++++++++++++++------- server/setup-database.js | 3 ++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/server/database.js b/server/database.js index c189af82d..9a9508f8e 100644 --- a/server/database.js +++ b/server/database.js @@ -11,6 +11,7 @@ const { UptimeCalculator } = require("./uptime-calculator"); const dayjs = require("dayjs"); const { SimpleMigrationServer } = require("./utils/simple-migration-server"); const KumaColumnCompiler = require("./utils/knex/lib/dialects/mysql2/schema/mysql2-columncompiler"); +const {tmpdir} = require("node:os"); /** * Database & App Data Folder @@ -183,23 +184,29 @@ class Database { } /** - * @throws The CA file must be a pem file + * @throws The CA file must be a pem file and not contain any illegal characters inside the filename which would allow it to escape the data directory * @typedef {string|undefined} envString * @param {{type: "sqlite"} | {type:envString, hostname:envString, port:envString, database:envString, username:envString, password:envString, caFilePath:envString}} dbConfig the database configuration that should be written * @returns {void} */ static writeDBConfig(dbConfig) { // Move CA file to the data directory - if (dbConfig.caFilePath) { + const temporaryDirectoryPath = tmpdir(); + if (dbConfig.caFilePath && dbConfig.caFile && dbConfig.caFilePath.startsWith(temporaryDirectoryPath)) { const dataCaFilePath = path.resolve(Database.dataDir, "mariadb-ca.pem"); - if (!path.resolve(dbConfig.caFilePath).endsWith(".pem")) { - throw new Error("Invalid CA file, it must be a .pem file"); + const sourcePath = fs.realpathSync(path.resolve(temporaryDirectoryPath, dbConfig.caFilePath)); + if (dataCaFilePath.startsWith(path.resolve(Database.dataDir)) && path.resolve(sourcePath).startsWith(temporaryDirectoryPath)) { + fs.renameSync(sourcePath, dataCaFilePath); + } else { + throw new Error("The file cannot contains any characters that would allow it to escape the data directory"); } - fs.renameSync(fs.realpathSync(path.resolve(dbConfig.caFilePath)), path.resolve(dataCaFilePath)); dbConfig.caFilePath = dataCaFilePath; - dbConfig.ssl = undefined; - dbConfig.caFile = undefined; } + if (dbConfig.caFilePath.startsWith(temporaryDirectoryPath)) { + dbConfig.caFilePath = undefined; + } + dbConfig.ssl = undefined; + dbConfig.caFile = undefined; fs.writeFileSync(path.join(Database.dataDir, "db-config.json"), JSON.stringify(dbConfig, null, 4)); } diff --git a/server/setup-database.js b/server/setup-database.js index 48237907f..152f9f5c3 100644 --- a/server/setup-database.js +++ b/server/setup-database.js @@ -6,6 +6,7 @@ const path = require("path"); const Database = require("./database"); const { allowDevAllOrigin } = require("./util-server"); const mysql = require("mysql2/promise"); +const { tmpdir } = require("node:os"); /** * A standalone express app that is used to setup a database @@ -215,7 +216,7 @@ class SetupDatabase { if (dbConfig.caFile) { const base64Data = dbConfig.caFile.replace(/^data:application\/octet-stream;base64,/, ""); const binaryData = Buffer.from(base64Data, "base64").toString("binary"); - const tempCaDirectory = fs.mkdtempSync("kuma-ca-"); + const tempCaDirectory = fs.mkdtempSync(path.join(tmpdir(), "kuma-ca-")); dbConfig.caFilePath = path.resolve(tempCaDirectory, "ca.pem"); try { fs.writeFileSync(dbConfig.caFilePath, binaryData, "binary");