Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ssrf https bug #128

Merged
merged 22 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
67883df
Remove SSRF code from http_client
Aug 29, 2024
17f3c1c
Remove check_context_for_ssrf bc find_hostname_in_context exists
Aug 29, 2024
dccc44f
Fix run_vulnerability_scan to run inspect_getaddrinfo code
Aug 29, 2024
7035cf5
Create a new split of inspect_dns_results function
Aug 29, 2024
6b51411
Use run_vulnerability_scan in socket.py wrapping
Aug 29, 2024
4022337
inspect_getaddrinfo_result function, just return results
Aug 29, 2024
e79ee35
Update get_port_from_url to allow for already parsed URLs
Aug 30, 2024
d17569d
Move normalize_url to seperate helper file and test it
Aug 30, 2024
51ce844
Only make exception for context for SSRF
Aug 30, 2024
2cc0cd3
Fix 2 tests that still needed monkeypatch in args
Aug 30, 2024
06c453b
Update get_redirect_origin to work with hostname and port
Aug 30, 2024
4d2fdcd
Remove debug statements out of normalize_url helper function
Aug 30, 2024
4643731
Update tests for get_redirect_origin to use hostname and port
Aug 30, 2024
f8b06eb
Update is_redirect_to_private_ip to use hostname and port
Aug 30, 2024
d3b9ce0
Update inspect_getaddrinfo_result to run redirect code
Aug 30, 2024
dc7dea8
Delete function scan_for_ssrf_in_request
Aug 30, 2024
2e648d9
Remove the contains_private_ip function
Aug 30, 2024
7591ec7
Merge branch 'main' into fix-ssrf-https-bug
bitterpanda63 Sep 3, 2024
335d882
Merge branch 'main' into fix-ssrf-https-bug
Sep 5, 2024
7d1e63b
Add comments clarifying branching decision in run_vulnerability_scan
Sep 5, 2024
8408c7f
Move inspect_dns_results down and add some more commentz to it
Sep 5, 2024
0f591c1
Rename findings to attack_findings, add a comment for that, - return
Sep 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions aikido_firewall/helpers/get_port_from_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
11 changes: 11 additions & 0 deletions aikido_firewall/helpers/get_port_from_url_test.py
Original file line number Diff line number Diff line change
@@ -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():
Expand All @@ -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
Empty file.
26 changes: 26 additions & 0 deletions aikido_firewall/helpers/urls/normalize_url.py
Original file line number Diff line number Diff line change
@@ -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
kapyteinaikido marked this conversation as resolved.
Show resolved Hide resolved
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
58 changes: 58 additions & 0 deletions aikido_firewall/helpers/urls/normalize_url_test.py
Original file line number Diff line number Diff line change
@@ -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("") == ""
19 changes: 3 additions & 16 deletions aikido_firewall/sinks/http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,9 @@
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}"
bitterpanda63 marked this conversation as resolved.
Show resolved Hide resolved
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

Check warning on line 31 in aikido_firewall/sinks/http_client.py

View check run for this annotation

Codecov / codecov/patch

aikido_firewall/sinks/http_client.py#L31

Added line #L31 was not covered by tests
return former_putrequest(_self, method, path, *args, **kwargs)

def aik_new_getresponse(_self):
Expand Down
13 changes: 5 additions & 8 deletions aikido_firewall/sinks/socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -25,8 +23,9 @@ def generate_aikido_function(former_func, op):
def aik_new_func(*args, **kwargs):
res = former_func(*args, **kwargs)
if op is "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
Expand All @@ -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
"""
Expand Down
24 changes: 14 additions & 10 deletions aikido_firewall/vulnerabilities/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from aikido_firewall.helpers.blocking_enabled import is_blocking_enabled
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,
)
Expand All @@ -36,9 +36,13 @@
context = get_current_context()
comms = get_comms()
lifecycle_cache = get_cache()
if not context or not comms 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":
bitterpanda63 marked this conversation as resolved.
Show resolved Hide resolved
# Make a special exception for SSRF
logger.debug("Not running scans due to incomplete data %s : %s", kind, op)
return

Check warning on line 42 in aikido_firewall/vulnerabilities/__init__.py

View check run for this annotation

Codecov / codecov/patch

aikido_firewall/vulnerabilities/__init__.py#L41-L42

Added lines #L41 - L42 were not covered by tests

if not comms or not lifecycle_cache:
logger.debug("Not running scans due to incomplete data %s : %s", kind, op)
return

if lifecycle_cache.protection_forced_off():
Expand Down Expand Up @@ -74,15 +78,15 @@
)
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(

Check warning on line 82 in aikido_firewall/vulnerabilities/__init__.py

View check run for this annotation

Codecov / codecov/patch

aikido_firewall/vulnerabilities/__init__.py#L82

Added line #L82 was not covered by tests
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:
get_comms().send_data_to_bg_process(
"HOSTNAMES_ADD", (args[0].hostname, args[1])
)
# Report hostname and port to background process :
get_comms().send_data_to_bg_process("HOSTNAMES_ADD", (args[1], args[2]))

Check warning on line 89 in aikido_firewall/vulnerabilities/__init__.py

View check run for this annotation

Codecov / codecov/patch

aikido_firewall/vulnerabilities/__init__.py#L89

Added line #L89 was not covered by tests
else:
logger.error(
"Vulnerability type %s currently has no scans implemented", kind
Expand Down
29 changes: 0 additions & 29 deletions aikido_firewall/vulnerabilities/ssrf/__init__.py
Original file line number Diff line number Diff line change
@@ -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"],
}
35 changes: 0 additions & 35 deletions aikido_firewall/vulnerabilities/ssrf/check_context_for_ssrf.py

This file was deleted.

This file was deleted.

Loading
Loading