🛡️ Sentinel: [MEDIUM] Fix File Descriptor Leakage in subprocess

.jules/sentinel.md

@@ -89,3 +89,7 @@
 **Vulnerability:** A Log Injection (CRLF) vulnerability existed in a shared exception handler. While Python's `ipaddress` module natively escapes control characters in its `ValueError` exceptions using `!r` formatting, catching broad exceptions (e.g., `except (ValueError, TypeError, RecursionError):`) and logging the `e` object via f-string interpolation (`f"Error: {e}"`) is dangerous. If a future, unrelated `raise ValueError("malicious\ninput")` is added to the try block, the unescaped control characters would be evaluated by the logger, allowing log spoofing.
 **Learning:** Shared, broad exception handlers that catch errors from multiple potential sources must assume that the exception payload is untrusted and un-sanitized. Relying on the safe formatting behavior of one specific underlying module (`ipaddress`) is insufficient defense-in-depth.
 **Prevention:** Always sanitize exception messages caught in broad handlers before logging them by wrapping them in `repr(str(e))`. This ensures any embedded control characters (like `\n` or `\r`) are securely escaped, neutralizing log injection vectors.
+## 2024-06-12 - File Descriptor Leakage via subprocess.call
+**Vulnerability:** The `is_reachable` function executed the `ping` utility using `subprocess.call` with `close_fds=False`. In a multi-threaded Python application (like one using `ThreadPoolExecutor`), if `close_fds` is `False`, the newly spawned child process inherits *all* file descriptors that happen to be open by any thread at the exact moment of the `fork`/`exec`. This can expose sensitive resources—such as open database connections, network sockets, or files with secure information—to the unprivileged child process, leading to File Descriptor Leakage (CWE-403).
+**Learning:** Micro-optimizations, such as avoiding the overhead of closing file descriptors by setting `close_fds=False`, can introduce significant security risks in concurrent environments. The inherited resources can be accessed or manipulated by the child process or any attacker who compromises it.
+**Prevention:** Always set `close_fds=True` (which is the default in Python 3.2+) when spawning child processes using `subprocess.call`, `subprocess.Popen`, or similar functions, particularly in multi-threaded applications, to ensure they start with a clean and secure file descriptor table.

test_testping1.py

@@ -347,7 +347,7 @@ def test_is_reachable_calls_ping_correctly(self, mock_call):
         # Verify that subprocess.call was called with the correct arguments, including the timeout
         mock_call.assert_called_once_with(
             [PING_PATH, '-n', '-q', '-c', '1', '-W', '5', '--', '8.8.8.8'],
-            stdout=DEVNULL_FD, stderr=DEVNULL_FD, close_fds=False, timeout=7
+            stdout=DEVNULL_FD, stderr=DEVNULL_FD, close_fds=True, timeout=7
         )
 
 if __name__ == '__main__':

testping1.py

@@ -220,13 +220,15 @@ def is_reachable(ip, timeout=1):
     # output to DEVNULL instead of using Popen with PIPE.
     # This avoids the Inter-Process Communication (IPC) overhead of capturing
     # stdout/stderr, resulting in ~35% speedup for parallel network scans.
-    # ⚡ Bolt: Disabled close_fds and used cached DEVNULL_FD to avoid the overhead of
-    # iterating and closing all possible file descriptors in the child process
-    # and opening/closing /dev/null per execution.
+    # 🛡️ Sentinel: Enable close_fds to prevent File Descriptor Leakage (CWE-403)
+    # In multi-threaded applications, child processes inherit all file descriptors
+    # currently opened by any thread if close_fds is False. This can expose
+    # sensitive open files, network sockets, or database connections to the
+    # unprivileged ping process.
     try:
         # 🛡️ Sentinel: Add python-level timeout limit as defense-in-depth to prevent
         # worker thread pool exhaustion if the underlying ping process hangs.
-        return subprocess.call(command, stdout=DEVNULL_FD, stderr=DEVNULL_FD, close_fds=False, timeout=timeout_val + 2) == 0
+        return subprocess.call(command, stdout=DEVNULL_FD, stderr=DEVNULL_FD, close_fds=True, timeout=timeout_val + 2) == 0
     except OSError:
         # 🛡️ Sentinel: Fail securely on command execution errors (like FileNotFoundError)
         # to prevent unhandled exceptions crashing the worker thread pool and leaking stack traces.