🛡️ Sentinel: [CRITICAL] Fix Argument Injection Vulnerability

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2024-05-18 - Input Validation for System Commands
+**Vulnerability:** Argument injection risk in `subprocess.Popen` via unsanitized IP parameter.
+**Learning:** `subprocess.Popen` without `shell=True` mitigates shell injection but is still vulnerable to argument injection (e.g., `-h` or other flags).
+**Prevention:** Strictly validate input formats using appropriate libraries (e.g., `ipaddress` for IP strings) before passing them to system commands.

test_testping1.py

@@ -25,6 +25,20 @@ def test_is_reachable_failure(self, mock_popen):
 
         self.assertFalse(is_reachable('10.0.0.1'))
 
+    @patch('testping1.subprocess.Popen')
+    def test_is_reachable_invalid_ip_format(self, mock_popen):
+        """Test is_reachable returns False and does not call Popen for invalid IP."""
+        self.assertFalse(is_reachable('invalid_ip'))
+        mock_popen.assert_not_called()
+
+    @patch('testping1.subprocess.Popen')
+    def test_is_reachable_argument_injection(self, mock_popen):
+        """Test is_reachable prevents argument injection by rejecting invalid IPs."""
+        self.assertFalse(is_reachable('-h'))
+        mock_popen.assert_not_called()
+        self.assertFalse(is_reachable('192.168.1.1; rm -rf /'))
+        mock_popen.assert_not_called()
+
     @patch('testping1.subprocess.Popen')
     def test_is_reachable_calls_ping_correctly(self, mock_popen):
         """Test is_reachable calls the ping command with correct arguments."""

testping1.py

@@ -1,5 +1,7 @@
 import subprocess
 import concurrent.futures
+import ipaddress
+import logging
 from tqdm import tqdm  # Install with `pip install tqdm`
 
 def is_reachable(ip, timeout=1):
@@ -13,7 +15,14 @@ def is_reachable(ip, timeout=1):
         bool: True if the ping is successful, False otherwise.
     """
 
-    command = ["ping", "-c", "1", "-W", str(timeout), ip]  # -W for timeout in seconds (Linux)
+    # 🛡️ Sentinel: Validate IP address to prevent argument injection
+    try:
+        ip_obj = ipaddress.ip_address(ip)
+    except ValueError:
+        logging.error(f"Invalid IP address format: {ip}")
+        return False
+
+    command = ["ping", "-c", "1", "-W", str(timeout), str(ip_obj)]  # -W for timeout in seconds (Linux)
     process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     output, error = process.communicate()