From 1f7c32194065f4627bc1c20f1c3bc598ab310e01 Mon Sep 17 00:00:00 2001 From: Terrence Asselin Date: Fri, 6 Dec 2024 14:47:18 -0600 Subject: [PATCH] HPCC-32999 Sanitize user provided password to ZAP file - Single-quote the entire password to preserve all characters but prevent interpretation as metacharacters. - Replace any single quote in the original string with a sequence of characters that breaks the single-quoted password into three parts (1) a single-quoted prefix (2) a double-quoted single quote and (3) a single-quoted suffix - The shell quote-removal deletes double-quotes around any single quote at the same time that it de-quotes the single-quoted prefix and suffix, leaving any single quote inside the password string that is tokenized as a single argument. This will require a different approach for Windows targets as a future task. Signed-off-by: Terrence Asselin --- .../ws_workunits/ws_workunitsHelpers.cpp | 6 +++- system/jlib/jstring.cpp | 29 +++++++++++++++++++ system/jlib/jstring.hpp | 3 ++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/esp/services/ws_workunits/ws_workunitsHelpers.cpp b/esp/services/ws_workunits/ws_workunitsHelpers.cpp index a018886f466..37a5efb8c3e 100644 --- a/esp/services/ws_workunits/ws_workunitsHelpers.cpp +++ b/esp/services/ws_workunits/ws_workunitsHelpers.cpp @@ -4486,7 +4486,11 @@ void CWsWuFileHelper::zipZAPFiles(const char* parentFolder, const char* zapFiles else zipCommand.setf("cd %s\nzip -r", parentFolder); if (!isEmptyString(passwordReq)) - zipCommand.append(" --password ").append(passwordReq); + { + StringBuffer sanitizedPassword; + sanitizeCommandArg(passwordReq, sanitizedPassword); + zipCommand.append(" --password ").append(sanitizedPassword); + } zipCommand.append(" ").append(zipFileNameWithFullPath).append(" ").append(zapFiles); int zipRet = system(zipCommand); if (zipRet != 0) diff --git a/system/jlib/jstring.cpp b/system/jlib/jstring.cpp index 7e581f2503a..1f6492b4bef 100644 --- a/system/jlib/jstring.cpp +++ b/system/jlib/jstring.cpp @@ -2983,3 +2983,32 @@ const void * jmemmem(size_t lenHaystack, const void * haystack, size_t lenNeedle return nullptr; } + +/** + * For preventing command injection, sanitize the argument to be passed to the system command. + * - Quote the entire argument with single quotes to prevent interpretation of shell metacharacters. + * - Since a single-quoted string can't contain single quotes, even escaped, replace each single + * quote in the argument with the sequence '"'"' . That closes the single quoted string, appends + * a literal single quote, and reopens the single quoted string + */ +StringBuffer& sanitizeCommandArg(const char* arg, StringBuffer& sanitized) +{ +#if defined(__linux__) || defined(__APPLE__) + if (!isEmptyString(arg)) + { + size_t len = strlen(arg); + sanitized.append('\''); + for (size_t i = 0; i < len; i++) + { + if (arg[i] == '\'') + sanitized.append(R"('"'"')"); + else + sanitized.append(arg[i]); + } + sanitized.append('\''); + } +#else + sanitized.append(arg); +#endif + return sanitized; +} diff --git a/system/jlib/jstring.hpp b/system/jlib/jstring.hpp index d61b7fb7a1f..f1ece8d0caa 100644 --- a/system/jlib/jstring.hpp +++ b/system/jlib/jstring.hpp @@ -659,4 +659,7 @@ extern jlib_decl void ensureSeparator(StringBuffer & out, char separator); //Search for one block of bytes within another block of bytes - memmem is not standard, so we provide our own extern jlib_decl const void * jmemmem(size_t lenHaystack, const void * haystack, size_t lenNeedle, const void *needle); +// For preventing command injection, sanitize the argument to be passed to the system command +extern jlib_decl StringBuffer& sanitizeCommandArg(const char* arg, StringBuffer& sanitized); + #endif