🛡️ Sentinel: [CRITICAL/HIGH] Fix SSRF vulnerability in network scanner

.jules/sentinel.md

@@ -6,3 +6,7 @@
 **Vulnerability:** Unhandled exceptions (e.g., `OSError` from missing `ping` binary) in `subprocess.call` within `ThreadPoolExecutor` workers bubbled up, crashing the aggregator process and leaking internal system details/stack traces. Additionally, a lack of timeout limits posed a Denial of Service (DoS) risk through resource exhaustion, tying up worker threads.
 **Learning:** `ThreadPoolExecutor` does not inherently catch unhandled exceptions in its worker tasks; calling `.result()` or iterating over `as_completed()` will re-raise them. External subprocess calls are a high-risk vector for unpredictable `OSErrors`.
 **Prevention:** Always wrap external system calls (`subprocess.call`, file operations, network requests) inside worker functions with secure `try...except` blocks. Log generic errors to avoid leaking stack traces, and implement upper bounds on input values that govern resource allocation (e.g., thread duration via timeout limits).
+## 2024-05-23 - [Server-Side Request Forgery Prevention in Utils]
+**Vulnerability:** The `is_reachable` utility function validated if an IP was correctly formatted, but did not restrict the semantic destination. This allowed Server-Side Request Forgery (SSRF) where the scanner could be manipulated into pinging loopback (`127.0.0.1`), link-local (e.g., AWS metadata `169.254.169.254`), or multicast addresses.
+**Learning:** Input validation is not just about syntax (like `ipaddress.ip_address`), but also about semantics and intended use. Even a simple `ping` utility can become an SSRF vector if it allows querying internal or restricted network ranges.
+**Prevention:** Always implement explicit allow-lists or block-lists for network destinations based on business logic. Use built-in library properties (like `ip_obj.is_loopback`, `ip_obj.is_link_local`) to robustly filter out non-routable or restricted administrative IP ranges before attempting network requests.

test_testping1.py

@@ -11,7 +11,7 @@ def test_is_reachable_success(self, mock_call):
         # Simulate a successful ping response by returning 0
         mock_call.return_value = 0
 
-        self.assertTrue(is_reachable('127.0.0.1'))
+        self.assertTrue(is_reachable('192.168.1.1'))
 
     @patch('testping1.subprocess.call')
     def test_is_reachable_failure(self, mock_call):
@@ -65,7 +65,7 @@ def test_is_reachable_secure_error_handling(self, mock_call):
         """Test is_reachable handles OSError securely without leaking exceptions."""
         mock_call.side_effect = FileNotFoundError("No such file or directory: 'ping'")
         with self.assertLogs(level='ERROR') as log:
-            self.assertFalse(is_reachable('127.0.0.1'))
+            self.assertFalse(is_reachable('192.168.1.1'))
             self.assertIn("Failed to execute ping command safely.", log.output[0])
             self.assertNotIn("FileNotFoundError", log.output[0])
 
@@ -91,10 +91,20 @@ def test_is_reachable_subprocess_timeout(self, mock_call):
         """Test is_reachable handles subprocess.TimeoutExpired securely."""
         mock_call.side_effect = subprocess.TimeoutExpired(cmd='ping', timeout=7)
         with self.assertLogs(level='ERROR') as log:
-            self.assertFalse(is_reachable('127.0.0.1', timeout=5))
+            self.assertFalse(is_reachable('192.168.1.1', timeout=5))
             self.assertIn("Ping command timed out unexpectedly.", log.output[0])
             self.assertNotIn("TimeoutExpired", log.output[0])
 
+    @patch('testping1.subprocess.call')
+    def test_is_reachable_ssrf_prevention(self, mock_call):
+        """Test is_reachable prevents SSRF by rejecting loopback, multicast, etc."""
+        ssrf_ips = ['127.0.0.1', '169.254.169.254', '224.0.0.1', '0.0.0.0']
+        for ip in ssrf_ips:
+            with self.assertLogs(level='ERROR') as log:
+                self.assertFalse(is_reachable(ip))
+                self.assertIn("IP address not allowed for scanning", log.output[0])
+                mock_call.assert_not_called()
+
     @patch('testping1.subprocess.call')
     def test_is_reachable_calls_ping_correctly(self, mock_call):
         """Test is_reachable calls the ping command with correct arguments."""

testping1.py

@@ -29,6 +29,12 @@ def is_reachable(ip, timeout=1):
         logging.error(f"Invalid IP address format: {repr(ip)}")
         return False
 
+    # 🛡️ Sentinel: Prevent Server-Side Request Forgery (SSRF)
+    # Block loopback, link-local, multicast, and unspecified addresses from being pinged.
+    if ip_obj.is_loopback or ip_obj.is_link_local or ip_obj.is_multicast or ip_obj.is_unspecified:
+        logging.error(f"IP address not allowed for scanning: {ip}")
+        return False
+
     # 🛡️ Sentinel: Validate timeout to prevent argument injection or errors
     try:
         timeout_val = int(timeout)