diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f3e9114a..7eb6065b 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -122,3 +122,7 @@ **Vulnerability:** DOM XSS via unescaped `severity` string interpolated into `innerHTML` in `scanner/dashboard/index.html`. **Learning:** Even enum-like or seemingly safe meta-fields like `severity` can contain malicious payloads if sourced from user input (findings file) and directly injected into innerHTML. **Prevention:** Always use the `esc()` sanitizer for any dynamically rendered property from `findings.json`, regardless of expected schema types. +## 2026-08-05 - Missing Type Validation for URL Parsers +**Vulnerability:** Core URL validation functions like `_is_safe_url` used `urllib.parse.urlparse` without first ensuring the input was a string. Passing boolean, integer, or `None` types triggered an unhandled `AttributeError`, resulting in a brittle application or API 500 errors instead of securely failing closed. +**Learning:** Functions that act as security boundaries must explicitly validate input types, particularly when passing those inputs to stdlib parser functions that assume specific types and throw non-standard errors (like `AttributeError` instead of `ValueError`). +**Prevention:** Always verify input types with `isinstance(url, str)` at the beginning of validation functions before passing them to parsing logic. diff --git a/appguardrail_core/controlplane.py b/appguardrail_core/controlplane.py index bf74784e..f086abe1 100644 --- a/appguardrail_core/controlplane.py +++ b/appguardrail_core/controlplane.py @@ -221,6 +221,9 @@ def _is_safe_url(url: str) -> bool: import urllib.parse import socket + if not isinstance(url, str): + return False + try: parsed = urllib.parse.urlparse( url diff --git a/scanner/cli/appguardrail.py b/scanner/cli/appguardrail.py index c655195a..b8880429 100644 --- a/scanner/cli/appguardrail.py +++ b/scanner/cli/appguardrail.py @@ -1622,6 +1622,9 @@ def _is_safe_url(url: str) -> bool: import socket import urllib.parse + if not isinstance(url, str): + return False + try: parsed = urllib.parse.urlparse( url diff --git a/tests/test_ssrf_protection.py b/tests/test_ssrf_protection.py index 3462f72f..0a584ac4 100644 --- a/tests/test_ssrf_protection.py +++ b/tests/test_ssrf_protection.py @@ -65,6 +65,14 @@ def test_is_safe_url_reserved_and_not_global_ips(): assert not _is_safe_url("http://0.0.0.0/") +def test_is_safe_url_non_string_inputs(): + assert not _is_safe_url(True) + assert not _is_safe_url(False) + assert not _is_safe_url(123) + assert not _is_safe_url(None) + assert not _is_safe_url(["http://google.com"]) + + def test_push_findings_unsafe_url_handled_properly(monkeypatch, capsys): from scanner.cli.appguardrail import _push_findings