diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 64c03f5..b296b3a 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -107,3 +107,8 @@ **Vulnerability:** Server-Side Request Forgery (SSRF) bypass due to `_is_safe_url` only checking `is_loopback` and `is_private`. This fails to correctly evaluate mapped IPv4 addresses disguised as IPv6 (e.g. `[::ffff:127.0.0.1]`) and misses restricted IP designations like `is_reserved` or non `is_global` IPs, allowing SSRF to `0.0.0.0` or `255.255.255.255`. **Learning:** Python's `ipaddress` objects for mapped IPv6 don't inherit properties of their IPv4 wrapped content directly. Using `is_loopback` without checking `.ipv4_mapped` leaves blind spots. **Prevention:** Always extract `getattr(ip, 'ipv4_mapped', None)` before evaluation, and combine checks spanning `is_reserved`, `not is_global`, `is_multicast`, `is_unspecified`, `is_private`, and `is_loopback` to fully protect endpoints. + +## 2024-05-24 - SSRF Bypass via HTTP Redirects +**Vulnerability:** The webhook alert feature (`_send_alert`) validated URLs before sending POST requests, but `urllib.request.urlopen` automatically follows HTTP 307/308 redirects by default. A malicious webhook server could return a redirect to an internal IP address (like 169.254.169.254 or 127.0.0.1) bypassing the `_is_safe_url` validation check and causing an SSRF. +**Learning:** Dynamic runtime URL validation is insufficient if the underlying HTTP client transparently follows redirects to arbitrary new targets. +**Prevention:** Explicitly disable redirects when making server-to-server HTTP requests to user-provided URLs using a custom `urllib.request.HTTPRedirectHandler` or by configuring the HTTP client not to follow redirects. diff --git a/appguardrail_core/controlplane.py b/appguardrail_core/controlplane.py index 07300b0..3dae81d 100644 --- a/appguardrail_core/controlplane.py +++ b/appguardrail_core/controlplane.py @@ -17,8 +17,14 @@ import re import secrets import sqlite3 +import sys from datetime import datetime, timezone -from importlib import resources # nosemgrep: python.lang.compatibility.python37.python37-compatibility-importlib2 + +if sys.version_info < (3, 9): + import importlib_resources as resources +else: + resources = getattr(__import__("importlib", fromlist=["resources"]), "resources") + from typing import Any, Iterable from urllib.parse import parse_qs, urlparse @@ -216,11 +222,13 @@ def _slack_blocks( def _is_safe_url(url: str) -> bool: import ipaddress - import urllib.parse import socket + import urllib.parse try: - parsed = urllib.parse.urlparse(url) # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected + parsed = urllib.parse.urlparse( + url + ) # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected except ValueError: return False @@ -294,14 +302,21 @@ def _send_alert( else: body = payload + class NoRedirectHandler(urllib.request.HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + import urllib.error + + raise urllib.error.URLError("Redirects are blocked to prevent SSRF bypass") + try: - req = urllib.request.Request( # noqa: S310 - Safe URL scheme validated + req = urllib.request.Request( url, data=json.dumps(body).encode("utf-8"), method="POST", headers={"Content-Type": "application/json"}, ) - urllib.request.urlopen( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected + opener = urllib.request.build_opener(NoRedirectHandler) + opener.open( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected req, timeout=10 ) # noqa: S310 - Safe URL scheme validated return True diff --git a/pyproject.toml b/pyproject.toml index 12291d6..b820708 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,8 @@ classifiers = [ "Topic :: Security", "Topic :: Software Development :: Quality Assurance" ] -dependencies = [] +dependencies = [ + "importlib_resources; python_version < \"3.9\"",] [project.urls] Homepage = "https://github.com/ContextualWisdomLab/appguardrail" diff --git a/tests/test_controlplane.py b/tests/test_controlplane.py index eb6b40d..9a089ed 100644 --- a/tests/test_controlplane.py +++ b/tests/test_controlplane.py @@ -352,7 +352,7 @@ def test_slack_blocks_caps_and_escapes(): def test_send_alert_slack_vs_generic(monkeypatch): posted = {} - def _fake_urlopen(req, timeout=None): + def _fake_urlopen(self, req, timeout=None): posted["url"] = req.full_url posted["body"] = json.loads(req.data.decode()) @@ -361,7 +361,7 @@ class _R: # minimal stand-in, urlopen result is ignored return _R() - monkeypatch.setattr(urllib.request, "urlopen", _fake_urlopen) + monkeypatch.setattr(urllib.request.OpenerDirector, "open", _fake_urlopen) generic = { "event": "drift.new_blocking", "org_id": 3, diff --git a/uv.lock b/uv.lock new file mode 100644 index 0000000..61558f3 --- /dev/null +++ b/uv.lock @@ -0,0 +1,10 @@ +version = 1 +revision = 3 +requires-python = ">=3.9" + +[[package]] +name = "appguardrail" +source = { editable = "." } + +[package.metadata] +requires-dist = [{ name = "importlib-resources", marker = "python_full_version < '3.9'" }]