From 06ec91a22d3829ac92f0d7d0104b41a23e1f02bb 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 ad23470fe64..0059fe52bd8 100644 --- a/esp/services/ws_workunits/ws_workunitsHelpers.cpp +++ b/esp/services/ws_workunits/ws_workunitsHelpers.cpp @@ -4386,7 +4386,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 930ded8801e..e08e62066ef 100644 --- a/system/jlib/jstring.cpp +++ b/system/jlib/jstring.cpp @@ -2902,3 +2902,32 @@ const char * stristr (const char *haystack, const char *needle) 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 162679a9e13..ebc90147db7 100644 --- a/system/jlib/jstring.hpp +++ b/system/jlib/jstring.hpp @@ -641,4 +641,7 @@ extern jlib_decl void processOptionString(const char * options, optionCallback c extern jlib_decl const char * stristr(const char *haystack, const char *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