Hardening: Avoid shell-based popen usage in InspectFile operator

src/operators/inspect_file.cc

@@ -16,28 +16,34 @@
 #include "src/operators/inspect_file.h"
 
 #include <stdio.h>
-
 #include <string>
 #include <iostream>
+#include <sstream>
 
 #include "src/operators/operator.h"
 #include "src/utils/system.h"
 
 #ifdef WIN32
 #include "src/compat/msvc.h"
+#else
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <array>
+#include <vector>
 #endif
 
 namespace modsecurity {
 namespace operators {
 
+
 bool InspectFile::init(const std::string &param2, std::string *error) {
     std::ifstream *iss;
     std::string err;
     std::string err_lua;
 
     m_file = utils::find_resource(m_param, param2, &err);
     iss = new std::ifstream(m_file, std::ios::in);
-
     if (iss->is_open() == false) {
         error->assign("Failed to open file: " + m_param + ". " + err);
         delete iss;
@@ -56,34 +62,110 @@ bool InspectFile::init(const std::string &param2, std::string *error) {
 bool InspectFile::evaluate(Transaction *transaction, const std::string &str) {
     if (m_isScript) {
         return m_lua.run(transaction, str);
-    } else {
-        FILE *in;
-        char buff[512];
-        std::stringstream s;
-        std::string res;
-        std::string openstr;
-
-        openstr.append(m_param);
-        openstr.append(" ");
-        openstr.append(str);
-        if (!(in = popen(openstr.c_str(), "r"))) {
-            return false;
-        }
+    }
 
-        while (fgets(buff, sizeof(buff), in) != NULL) {
-            s << buff;
-        }
+#ifndef WIN32
+    /*
+     * Use fork()+execv() to avoid shell interpretation and PATH ambiguity.
+     * Execute the resolved m_file path directly instead of m_param.
+     */
+    std::array<int, 2> pipefd{};
+    if (pipe(pipefd.data()) == -1) {
+        return false;
+    }
+
+    pid_t pid = fork();
+    if (pid == -1) {
+        close(pipefd[0]);
+        close(pipefd[1]);
+        return false;
+    }
+
+    if (pid == 0) {
+        // Child process
+        close(pipefd[0]);                // Close read end
+        dup2(pipefd[1], STDOUT_FILENO);  // Redirect stdout to pipe
+        close(pipefd[1]);
+
+        // Create mutable copies for execv() argument array
+        std::string file_copy = m_file;
+        std::string str_copy = str;
+
+        std::vector<char *> argv;
+        argv.push_back(file_copy.data());
+        argv.push_back(str_copy.data());
+        argv.push_back(nullptr);
+
+        // Use execv() with the resolved path — avoids PATH lookup ambiguity
+        execv(file_copy.data(), argv.data());
+
+        // execv() failed: exit child immediately
+        _exit(1);
+    }
+
+    // Parent process
+    close(pipefd[1]);  // Close write end
 
-        pclose(in);
+    std::array<char, 512> buff{};
+    std::stringstream s;
 
-        res.append(s.str());
-        if (res.size() > 1 && res[0] != '1') {
-            return true; /* match */
+    // Retry on EINTR so a signal does not silently truncate output
+    ssize_t count = 0;
+    do {
+        count = read(pipefd[0], buff.data(), buff.size());
+        if (count > 0) {
+            s.write(buff.data(), count);
+        } else if (count < 0 && errno == EINTR) {
+            count = 1;  // sentinel: keep looping
         }
+    } while (count > 0);
 
-        /* no match */
+    close(pipefd[0]);
+
+    // Check child exit status; treat non-zero exit as no match
+    int wstatus = 0;
+    if (waitpid(pid, &wstatus, 0) == -1) {
+        return false;
+    }
+    if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) {
+        return false;
+    }
+
+    if (const std::string res = s.str(); res.size() > 1 && res[0] != '1') {
+        return true;
+    }
+
+    return false;
+
+#else
+    /*
+     * Windows fallback: use popen() to invoke the script.
+     */
+    FILE *in;
+    std::array<char, 512> buff{};
+    std::stringstream s;
+
+    std::string openstr;
+    openstr.append(m_param);
+    openstr.append(" ");
+    openstr.append(str);
+
+    if (!(in = popen(openstr.c_str(), "r"))) {
         return false;
     }
+
+    while (fgets(buff.data(), static_cast<int>(buff.size()), in) != NULL) {
+        s << buff.data();
+    }
+
+    pclose(in);
+
+    if (const std::string res = s.str(); res.size() > 1 && res[0] != '1') {
+        return true;
+    }
+
+    return false;
+#endif
 }