From 580bfae21c665300f2e79b14abdc641e6e6ceffe Mon Sep 17 00:00:00 2001
From: Terrence Asselin <terrence.asselin@lexisnexisrisk.com>
Date: Wed, 11 Dec 2024 09:30:26 -0600
Subject: [PATCH] HPCC-32999 Address review comments

- Rename sanitize function to better indicate purpose
- Move function to jstring
- Correct unsigned vs signed comparison in for loop

Signed-off-by: Terrence Asselin <terrence.asselin@lexisnexisrisk.com>
---
 .../ws_workunits/ws_workunitsHelpers.cpp      | 30 +------------------
 system/jlib/jstring.cpp                       | 29 ++++++++++++++++++
 system/jlib/jstring.hpp                       |  3 ++
 3 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/esp/services/ws_workunits/ws_workunitsHelpers.cpp b/esp/services/ws_workunits/ws_workunitsHelpers.cpp
index 88e7afe305a..5446e062a8d 100644
--- a/esp/services/ws_workunits/ws_workunitsHelpers.cpp
+++ b/esp/services/ws_workunits/ws_workunitsHelpers.cpp
@@ -4326,34 +4326,6 @@ void readWUComponentLogOptionsReq(T* logReq, WUComponentLogOptions& options)
     }
 }
 
-// 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, 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& sanitizeArg(const char* arg, StringBuffer& sanitized)
-{
-#if defined(__linux__) || defined(__APPLE__)
-    if (!isEmptyString(arg))
-    {
-        size_t len = strlen(arg);
-        sanitized.append('\'');
-        for (int 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;
-}
-
 void CWsWuFileHelper::sendWUComponentLogStreaming(CHttpRequest* request, CHttpResponse* response)
 {
     StringBuffer wuid, fileName;
@@ -4526,7 +4498,7 @@ void CWsWuFileHelper::zipZAPFiles(const char* parentFolder, const char* zapFiles
     if (!isEmptyString(passwordReq))
     {
         StringBuffer sanitizedPassword;
-        sanitizeArg(passwordReq, sanitizedPassword);
+        sanitizeCommandArg(passwordReq, sanitizedPassword);
         zipCommand.append(" --password ").append(sanitizedPassword);
     }
     zipCommand.append(" ").append(zipFileNameWithFullPath).append(" ").append(zapFiles);
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