Avoid shell command injection in main.cpp

main.cpp

@@ -27,7 +27,9 @@ void createDirectory(string path),
      markChannelAsRequiringTreatmentIfNeeded(unsigned short threadId, string channelId),
      execute(unsigned short threadId, string command, bool debug = true);
 string getHttps(string url),
-       join(vector<string> parts, string delimiter);
+       join(vector<string> parts, string delimiter),
+       escapeShellArgument(string shellArgument),
+       replaceAll(string str, const string& from, const string& to);
 size_t writeCallback(void* contents, size_t size, size_t nmemb, void* userp);
 bool doesFileExist(string filePath),
      writeFile(unsigned short threadId, string filePath, string option, string toWrite);
@@ -242,7 +244,7 @@ void treatChannels(unsigned short threadId)
         // As I haven't found any well-known library that compress easily a directory, I have chosen to rely on `zip` cli.
         // We precise no `debug`ging, as otherwise the zipping operation doesn't work as expected.
         // As the zipping process isn't recursive, we can't just rely on `ls`, but we are obliged to use `find`.
-        execute(threadId, "cd " + channelToTreatDirectory + " && find | zip ../" + channelToTreat + ".zip -@");
+        execute(threadId, "cd " + escapeShellArgument(channelToTreatDirectory) + " && find | zip " + escapeShellArgument("../" + channelToTreat + ".zip") + " -@");
 
         PRINT("Compression finished, started deleting initial directory...")
         // Get rid of the uncompressed data.
@@ -681,7 +683,7 @@ void treatChannelOrVideo(unsigned short threadId, bool isIdAChannelId, string id
                 // The underscore in `-o` argument is used to not end up with hidden files.
                 // We are obliged to precise the video id after `--`, otherwise if the video id starts with `-` it's considered as an argument.
                 string commandCommonPrefix = "yt-dlp --skip-download ",
-                       commandCommonPostfix = " -o '" + channelCaptionsToTreatDirectory + "_' -- " + videoId;
+                       commandCommonPostfix = " -o " + escapeShellArgument(channelCaptionsToTreatDirectory + "_") + " -- " + escapeShellArgument(videoId);
                 string command = commandCommonPrefix + "--write-sub --sub-lang all,-live_chat" + commandCommonPostfix;
                 execute(threadId, command);
 
@@ -929,3 +931,20 @@ size_t writeCallback(void* contents, size_t size, size_t nmemb, void* userp)
     ((string*)userp)->append((char*)contents, size * nmemb);
     return size * nmemb;
 }
+
+// Source: https://stackoverflow.com/a/3669819
+string escapeShellArgument(string shellArgument)
+{
+    return "'" + replaceAll(shellArgument, "'", "'\\''") + "'";
+}
+
+string replaceAll(string str, const string& from, const string& to)
+{
+    size_t start_pos = 0;
+    while((start_pos = str.find(from, start_pos)) != string::npos)
+    {
+        str.replace(start_pos, from.length(), to);
+        start_pos += to.length(); // Handles case where 'to' is a substring of 'from'
+    }
+    return str;
+}

website/websocket.php

@@ -27,7 +27,7 @@ class Client
         posix_kill($this->pid, SIGTERM);
         $clientFilePath = getClientFilePath($this->id);
         if (file_exists($clientFilePath)) {
-            $fp = fopen($clientFilePath, "r+");
+            $fp = fopen($clientFilePath, 'r+');
             if (flock($fp, LOCK_EX, $WAIT_IF_LOCKED)) {   // acquire an exclusive lock
                 unlink($clientFilePath); // delete file
                 flock($fp, LOCK_UN);     // release the lock
@@ -92,8 +92,6 @@ class MyProcess implements MessageComponentInterface
 
     public function onMessage(ConnectionInterface $from, $msg)
     {
-        // As we are going to use this argument in a shell command, we escape it.
-        $msg = escapeshellarg($msg);
         $client = $this->clients->offsetGet($from);
         // If a previous request was received, we execute the new one with another client for simplicity otherwise with current file deletion approach, we can't tell the worker `search.py` that we don't care about its execution anymore.
         if ($client->pid !== null) {
@@ -105,6 +103,8 @@ class MyProcess implements MessageComponentInterface
         $clientFilePath = getClientFilePath($clientId);
         // Create the worker output file otherwise it would believe that we don't need this worker anymore.
         file_put_contents($clientFilePath, '');
+        // As we are going to use this argument in a shell command, we escape it.
+        $msg = escapeshellarg($msg);
         // Start the independent worker.
         // Redirecting `stdout` is mandatory otherwise `exec` is blocking.
         $client->pid = exec("./search.py $clientId $msg > /dev/null & echo $!");
@@ -114,7 +114,7 @@ class MyProcess implements MessageComponentInterface
             // If the worker output file doesn't exist anymore, then it means that the worker have finished its work and acknowledged that `websocket.php` completely read its output.
             if (file_exists($clientFilePath)) {
                 // `flock` requires `r`eading permission and we need `w`riting one due to `ftruncate` usage.
-                $fp = fopen($clientFilePath, "r+");
+                $fp = fopen($clientFilePath, 'r+');
                 $read = null;
                 if (flock($fp, LOCK_EX, $WAIT_IF_LOCKED)) { // acquire an exclusive lock
                     // We assume that the temporary output is less than 1 MB long.