Merge pull request #128 from AikidoSec/fix-ssrf-https-bug

Fix ssrf https bug

aikido_firewall/helpers/get_port_from_url.py

@@ -5,11 +5,14 @@
 from urllib.parse import urlparse
 
 
-def get_port_from_url(url):
+def get_port_from_url(url, parsed=False):
     """
     Tries to retrieve a port number from the given url
     """
-    parsed_url = urlparse(url)
+    if not parsed:
+        parsed_url = urlparse(url)
+    else:
+        parsed_url = url
 
     # Check if the port is specified and is a valid integer
     if parsed_url.port is not None:

aikido_firewall/helpers/get_port_from_url_test.py

@@ -1,5 +1,6 @@
 import pytest
 from .get_port_from_url import get_port_from_url
+from urllib.parse import urlparse
 
 
 def test_get_port_from_url():
@@ -8,3 +9,13 @@ def test_get_port_from_url():
     assert get_port_from_url("https://test.com:8080/test?abc=123") == 8080
     assert get_port_from_url("https://localhost") == 443
     assert get_port_from_url("ftp://localhost") is None
+
+
+def test_get_port_from_parsed_url():
+    assert get_port_from_url(urlparse("http://localhost:4000"), True) == 4000
+    assert get_port_from_url(urlparse("http://localhost"), True) == 80
+    assert (
+        get_port_from_url(urlparse("https://test.com:8080/test?abc=123"), True) == 8080
+    )
+    assert get_port_from_url(urlparse("https://localhost"), True) == 443
+    assert get_port_from_url(urlparse("ftp://localhost"), True) is None

aikido_firewall/helpers/urls/normalize_url.py

@@ -0,0 +1,26 @@
+"""Helper function file, exports normalize_url"""
+
+from urllib.parse import urlparse, urlunparse
+
+
+def normalize_url(url):
+    """Normalizes the url"""
+    # Parse the URL
+    parsed_url = urlparse(url)
+
+    # Normalize components
+    scheme = parsed_url.scheme.lower()  # Lowercase scheme
+    netloc = parsed_url.netloc.lower()  # Lowercase netloc
+    path = parsed_url.path.rstrip("/")  # Remove trailing slash
+    query = parsed_url.query  # Keep query as is
+    fragment = parsed_url.fragment  # Keep fragment as is
+
+    # Remove default ports (80 for http, 443 for https)
+    if scheme == "http" and parsed_url.port == 80:
+        netloc = netloc.replace(":80", "")
+    elif scheme == "https" and parsed_url.port == 443:
+        netloc = netloc.replace(":443", "")
+
+    # Reconstruct the normalized URL
+    normalized_url = urlunparse((scheme, netloc, path, "", query, fragment))
+    return normalized_url

aikido_firewall/helpers/urls/normalize_url_test.py

@@ -0,0 +1,58 @@
+import pytest
+from .normalize_url import normalize_url
+
+
+def test_normalize_url():
+    # Test with standard URLs
+    assert normalize_url("http://example.com") == "http://example.com"
+    assert normalize_url("https://example.com") == "https://example.com"
+    assert normalize_url("http://example.com/") == "http://example.com"
+    assert normalize_url("http://example.com/path/") == "http://example.com/path"
+    assert normalize_url("http://example.com/path") == "http://example.com/path"
+
+    # Test with lowercase and uppercase schemes
+    assert normalize_url("HTTP://EXAMPLE.COM") == "http://example.com"
+    assert normalize_url("Https://EXAMPLE.COM") == "https://example.com"
+
+    # Test with default ports
+    assert normalize_url("http://example.com:80/path") == "http://example.com/path"
+    assert normalize_url("https://example.com:443/path") == "https://example.com/path"
+
+    # Test with non-default ports
+    assert (
+        normalize_url("http://example.com:8080/path") == "http://example.com:8080/path"
+    )
+    assert (
+        normalize_url("https://example.com:8443/path")
+        == "https://example.com:8443/path"
+    )
+
+    # Test with query parameters
+    assert (
+        normalize_url("http://example.com/path?query=1")
+        == "http://example.com/path?query=1"
+    )
+    assert (
+        normalize_url("http://example.com/path/?query=1")
+        == "http://example.com/path?query=1"
+    )
+
+    # Test with fragments
+    assert (
+        normalize_url("http://example.com/path#fragment")
+        == "http://example.com/path#fragment"
+    )
+    assert (
+        normalize_url("http://example.com/path/?query=1#fragment")
+        == "http://example.com/path?query=1#fragment"
+    )
+
+    # Test with URLs that have trailing slashes and mixed cases
+    assert normalize_url("http://Example.com/Path/") == "http://example.com/Path"
+    assert (
+        normalize_url("http://example.com/path/another/")
+        == "http://example.com/path/another"
+    )
+
+    # Test with empty URL
+    assert normalize_url("") == ""

aikido_firewall/sinks/http_client.py

@@ -26,22 +26,9 @@ def on_http_import(http):
     former_getresponse = copy.deepcopy(http.HTTPConnection.getresponse)
 
     def aik_new_putrequest(_self, method, path, *args, **kwargs):
-        #  Aikido putrequest, gets called before the request went through
-        try:
-            # Set path for aik_new_getresponse :
-            _self.aikido_attr_path = path
-
-            # Create a URL Object :
-            assembled_url = f"http://{_self.host}:{_self.port}{path}"
-            url_object = try_parse_url(assembled_url)
-
-            run_vulnerability_scan(
-                kind="ssrf", op="http.client.putrequest", args=(url_object, _self.port)
-            )
-        except AikidoException as e:
-            raise e
-        except Exception as e:
-            logger.debug("Exception occured in custom putrequest function : %s", e)
+        #  Aikido putrequest, gets called before the request goes through
+        # Set path for aik_new_getresponse :
+        _self.aikido_attr_path = path
         return former_putrequest(_self, method, path, *args, **kwargs)
 
     def aik_new_getresponse(_self):

aikido_firewall/sinks/socket.py

@@ -5,9 +5,7 @@
 import copy
 import importhook
 from aikido_firewall.helpers.logging import logger
-from aikido_firewall.vulnerabilities.ssrf.inspect_getaddrinfo_result import (
-    inspect_getaddrinfo_result,
-)
+from aikido_firewall.vulnerabilities import run_vulnerability_scan
 
 SOCKET_OPERATIONS = [
     "gethostbyname",
@@ -25,8 +23,9 @@ def generate_aikido_function(former_func, op):
     def aik_new_func(*args, **kwargs):
         res = former_func(*args, **kwargs)
         if op == "getaddrinfo":
-            inspect_getaddrinfo_result(dns_results=res, hostname=args[0], port=args[1])
-        logger.debug("Res %s", res)
+            run_vulnerability_scan(
+                kind="ssrf", op="socket.getaddrinfo", args=(res, args[0], args[1])
+            )
         return res
 
     return aik_new_func
@@ -36,9 +35,7 @@ def aik_new_func(*args, **kwargs):
 def on_socket_import(socket):
     """
     Hook 'n wrap on `socket`
-    Our goal is to wrap the following socket functions that take a hostname :
-      -  gethostbyname() -- map a hostname to its IP number
-      -  gethostbyaddr() -- map an IP number or hostname to DNS info
+    Our goal is to wrap the getaddrinfo socket function
     https://github.com/python/cpython/blob/8f19be47b6a50059924e1d7b64277ad3cef4dac7/Lib/socket.py#L10
     Returns : Modified socket object
     """

aikido_firewall/vulnerabilities/__init__.py

@@ -19,7 +19,7 @@
 from aikido_firewall.background_process.ipc_lifecycle_cache import get_cache
 from .sql_injection.context_contains_sql_injection import context_contains_sql_injection
 from .nosql_injection.check_context import check_context_for_nosql_injection
-from .ssrf import scan_for_ssrf_in_request
+from .ssrf.inspect_getaddrinfo_result import inspect_getaddrinfo_result
 from .shell_injection.check_context_for_shell_injection import (
     check_context_for_shell_injection,
 )
@@ -36,9 +36,14 @@ def run_vulnerability_scan(kind, op, args):
     context = get_current_context()
     comms = get_comms()
     lifecycle_cache = get_cache()
-    if not context or not lifecycle_cache:
-        logger.debug("Not running a vulnerability scan due to incomplete data.")
-        logger.debug("%s : %s", kind, op)
+    if not context and kind != "ssrf":
+        # Make a special exception for SSRF, which checks itself if context is set.
+        # This is because some scans/tests for SSRF do not require a context to be set.
+        logger.debug("Not running scans due to incomplete data %s : %s", kind, op)
+        return
+
+    if not lifecycle_cache:
+        logger.debug("Not running scans due to incomplete data %s : %s", kind, op)
         return
 
     if lifecycle_cache.protection_forced_off():
@@ -74,9 +79,10 @@ def run_vulnerability_scan(kind, op, args):
             )
             error_type = AikidoPathTraversal
         elif kind == "ssrf":
-            # args[0] : URL object, args[1] : Port
-            # Report hostname and port to background process :
-            injection_results = scan_for_ssrf_in_request(args[0], args[1], op, context)
+            # args[0] : DNS Results, args[1] : Hostname, args[2] : Port
+            injection_results = inspect_getaddrinfo_result(
+                dns_results=args[0], hostname=args[1], port=args[2]
+            )
             error_type = AikidoSSRF
             blocked_request = is_blocking_enabled() and injection_results
             if not blocked_request:

aikido_firewall/vulnerabilities/ssrf/__init__.py

@@ -1,29 +0,0 @@
-"""Exports scan_for_ssrf_in_request function"""
-
-from aikido_firewall.helpers.logging import logger
-from .check_context_for_ssrf import check_context_for_ssrf
-from .is_redirect_to_private_ip import is_redirect_to_private_ip
-
-
-def scan_for_ssrf_in_request(url, port, operation, context):
-    """Scans for SSRF attacks"""
-
-    # Check if the request is a SSRF :
-    context_contains_ssrf_results = check_context_for_ssrf(
-        url.hostname, port, operation, context
-    )
-    if context_contains_ssrf_results:
-        return context_contains_ssrf_results
-
-    # Check if the request is a SSRF with redirects :
-    logger.debug("Redirects : %s", context.outgoing_req_redirects)
-    redirected_ssrf_results = is_redirect_to_private_ip(url, context)
-    if redirected_ssrf_results:
-        return {
-            "operation": operation,
-            "kind": "ssrf",
-            "source": redirected_ssrf_results["source"],
-            "pathToPayload": redirected_ssrf_results["pathToPayload"],
-            "metadata": {},
-            "payload": redirected_ssrf_results["payload"],
-        }