From 9faa3d000c35cef49a82ef8aa0cb5598238df338 Mon Sep 17 00:00:00 2001 From: Ashish Pal Date: Tue, 21 Jul 2026 12:31:21 +0530 Subject: [PATCH 1/3] Disable TLS certificate verification in crawler allows MITM/SSRF --- .env.example | 1 + backend/secuscan/config.py | 1 + backend/secuscan/crawler.py | 86 +++++++--- testing/backend/unit/test_crawler_security.py | 152 ++++++++++++++++++ 4 files changed, 215 insertions(+), 25 deletions(-) create mode 100644 testing/backend/unit/test_crawler_security.py diff --git a/.env.example b/.env.example index 0d4e08f97..ff2079288 100644 --- a/.env.example +++ b/.env.example @@ -29,6 +29,7 @@ SECUSCAN_DOCKER_NETWORK=restricted SECUSCAN_SAFE_MODE_DEFAULT=true SECUSCAN_REQUIRE_CONSENT=true SECUSCAN_ALLOW_LOOPBACK_SCANS=true +# SECUSCAN_TLS_VERIFY=true # SECUSCAN_ALLOWED_NETWORKS=127.0.0.1,192.168.*.*,10.*.*.*,172.16.*.* # SECUSCAN_CORS_ALLOWED_ORIGINS=http://127.0.0.1:5173,http://localhost:5173 diff --git a/backend/secuscan/config.py b/backend/secuscan/config.py index 48c586172..19554b848 100644 --- a/backend/secuscan/config.py +++ b/backend/secuscan/config.py @@ -63,6 +63,7 @@ class Settings(BaseSettings): # Security safe_mode_default: bool = True verify_ssl: bool = True + tls_verify: bool = True dns_resolution_timeout_seconds: float = 1.5 dns_cache_ttl_seconds: int = 60 dns_rebind_check: bool = True diff --git a/backend/secuscan/crawler.py b/backend/secuscan/crawler.py index f7a21dcf8..2785a121a 100644 --- a/backend/secuscan/crawler.py +++ b/backend/secuscan/crawler.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from html.parser import HTMLParser import re from typing import Any, Dict, List @@ -11,6 +12,8 @@ from .config import settings +logger = logging.getLogger(__name__) + class _SurfaceParser(HTMLParser): def __init__(self) -> None: @@ -85,36 +88,69 @@ async def crawl_target( ) -> Dict[str, Any]: """Fetch a target and normalize discovered links/forms/scripts/API hints.""" headers = _build_headers(extra_headers) + tls_verify = getattr(settings, "tls_verify", True) and getattr(settings, "verify_ssl", True) + + seed_parsed = urlparse(url) + seed_hostname = (seed_parsed.hostname or "").lower() + + current_url = url + history: List[httpx.Response] = [] + redirect_count = 0 + async with httpx.AsyncClient( - follow_redirects=True, - max_redirects=max_redirects, + follow_redirects=False, timeout=timeout, headers=headers, cookies=cookies or {}, - verify=settings.verify_ssl, + verify=tls_verify, ) as client: - async with client.stream("GET", url) as response: - # Check Content-Length header if present - content_length = response.headers.get("content-length") - if content_length: - try: - cl_val = int(content_length) - except ValueError: - cl_val = 0 - if cl_val > max_size: - raise ValueError(f"Response size exceeds limit of {max_size} bytes") - - # Read response in chunks to enforce size limit - body_chunks = [] - bytes_read = 0 - async for chunk in response.aiter_bytes(): - bytes_read += len(chunk) - if bytes_read > max_size: - raise ValueError(f"Response size exceeds limit of {max_size} bytes") - body_chunks.append(chunk) - - body_bytes = b"".join(body_chunks) - body = body_bytes.decode("utf-8", errors="replace") + while True: + async with client.stream("GET", current_url) as response: + # Check Content-Length header if present + content_length = response.headers.get("content-length") + if content_length: + try: + cl_val = int(content_length) + except ValueError: + cl_val = 0 + if cl_val > max_size: + raise ValueError(f"Response size exceeds limit of {max_size} bytes") + + # Read response in chunks to enforce size limit + body_chunks = [] + bytes_read = 0 + async for chunk in response.aiter_bytes(): + bytes_read += len(chunk) + if bytes_read > max_size: + raise ValueError(f"Response size exceeds limit of {max_size} bytes") + body_chunks.append(chunk) + + body_bytes = b"".join(body_chunks) + body = body_bytes.decode("utf-8", errors="replace") + + # Check if redirect and process same-host policy + if response.is_redirect and "location" in response.headers and redirect_count < max_redirects: + location = response.headers["location"] + next_url = urljoin(current_url, location) + next_hostname = (urlparse(next_url).hostname or "").lower() + + if seed_hostname and next_hostname and next_hostname != seed_hostname: + logger.warning( + "Crawler redirect blocked due to host mismatch: %s -> %s (seed host: %s, redirect host: %s)", + current_url, + next_url, + seed_hostname, + next_hostname, + ) + break + + history.append(response) + current_url = next_url + redirect_count += 1 + else: + break + + response.history = history parser = _SurfaceParser() parser.feed(body) diff --git a/testing/backend/unit/test_crawler_security.py b/testing/backend/unit/test_crawler_security.py new file mode 100644 index 000000000..81b5b5f6e --- /dev/null +++ b/testing/backend/unit/test_crawler_security.py @@ -0,0 +1,152 @@ +""" +Unit tests for crawler TLS verification and redirect security policy (Issue #1747). +""" + +from __future__ import annotations + +from unittest.mock import patch, MagicMock +import pytest + +from backend.secuscan.config import settings +from backend.secuscan.crawler import crawl_target + + +class TestCrawlerTLSVerification: + def test_tls_verify_setting_exists(self): + assert hasattr(settings, "tls_verify") + assert settings.tls_verify is True + + @pytest.mark.asyncio + async def test_crawl_target_uses_tls_verify_setting(self): + """crawl_target must construct AsyncClient with verify=tls_verify.""" + mock_response = MagicMock() + mock_response.headers = {} + mock_response.is_redirect = False + mock_response.url = "http://example.com" + mock_response.status_code = 200 + + async def fake_aiter_bytes(): + yield b"TestOK" + + mock_response.aiter_bytes = fake_aiter_bytes + + class MockStreamContext: + async def __aenter__(self): + return mock_response + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.return_value = MockStreamContext() + + class MockAsyncClient: + def __init__(self, **kwargs): + self.kwargs = kwargs + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient) as mock_client_cls: + res = await crawl_target("http://example.com") + assert mock_client_cls.call_count == 1 + client_kwargs = mock_client_cls.call_args.kwargs + assert client_kwargs.get("verify") is True + assert client_kwargs.get("follow_redirects") is False + + +class TestCrawlerRedirectPolicy: + @pytest.mark.asyncio + async def test_same_host_redirect_allowed(self): + """Redirects to the same hostname must be followed.""" + resp1 = MagicMock() + resp1.headers = {"location": "/login"} + resp1.is_redirect = True + resp1.status_code = 302 + resp1.url = "http://example.com/start" + + async def fake_aiter_bytes1(): + yield b"Redirecting..." + resp1.aiter_bytes = fake_aiter_bytes1 + + resp2 = MagicMock() + resp2.headers = {} + resp2.is_redirect = False + resp2.status_code = 200 + resp2.url = "http://example.com/login" + + async def fake_aiter_bytes2(): + yield b"Login PageForm" + resp2.aiter_bytes = fake_aiter_bytes2 + + responses = [resp1, resp2] + call_index = 0 + + class MockStreamContext: + def __init__(self, url): + nonlocal call_index + self.resp = responses[call_index] + call_index += 1 + async def __aenter__(self): + return self.resp + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.side_effect = lambda method, url: MockStreamContext(url) + + class MockAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient): + result = await crawl_target("http://example.com/start") + assert result["final_url"] == "http://example.com/login" + assert result["status_code"] == 200 + assert len(result["redirect_chain"]) == 1 + assert result["redirect_chain"][0]["url"] == "http://example.com/start" + assert result["redirect_chain"][0]["location"] == "/login" + + @pytest.mark.asyncio + async def test_cross_host_redirect_blocked(self): + """Redirects to a different hostname (MITM / SSRF attempt) must be blocked.""" + resp1 = MagicMock() + resp1.headers = {"location": "http://evil.com/phish"} + resp1.is_redirect = True + resp1.status_code = 302 + resp1.url = "http://example.com/start" + + async def fake_aiter_bytes1(): + yield b"Redirecting..." + resp1.aiter_bytes = fake_aiter_bytes1 + + class MockStreamContext: + async def __aenter__(self): + return resp1 + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.return_value = MockStreamContext() + + class MockAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient): + with patch("backend.secuscan.crawler.logger.warning") as mock_warning: + result = await crawl_target("http://example.com/start") + # Cross-host redirect should be stopped immediately + assert result["final_url"] == "http://example.com/start" + assert result["status_code"] == 302 + assert len(result["redirect_chain"]) == 0 + mock_warning.assert_called_once() + assert "blocked due to host mismatch" in mock_warning.call_args[0][0] From 5fd11960d848bf7755f3cea239a0a2032f047d72 Mon Sep 17 00:00:00 2001 From: Ashish Pal Date: Fri, 24 Jul 2026 20:06:12 +0530 Subject: [PATCH 2/3] refactor(crawler): refine redirect policy, max-hop handling, and TLS test coverage --- backend/secuscan/crawler.py | 31 ++- testing/backend/unit/test_crawler_security.py | 180 +++++++++++++++++- 2 files changed, 206 insertions(+), 5 deletions(-) diff --git a/backend/secuscan/crawler.py b/backend/secuscan/crawler.py index 2785a121a..bf423878f 100644 --- a/backend/secuscan/crawler.py +++ b/backend/secuscan/crawler.py @@ -86,7 +86,15 @@ async def crawl_target( max_redirects: int = 10, max_size: int = 5 * 1024 * 1024, ) -> Dict[str, Any]: - """Fetch a target and normalize discovered links/forms/scripts/API hints.""" + """Fetch a target and normalize discovered links/forms/scripts/API hints. + + Redirect Policy & Security Semantics: + - Enforces configurable TLS certificate verification (via ``SECUSCAN_TLS_VERIFY``). + - Only same-host HTTP/HTTPS redirects relative to the original seed URL host are followed. + - Cross-host redirects (different target domain/hostname) or non-HTTP/HTTPS schemes + (e.g., ``javascript:``, ``file:``) are blocked and logged to mitigate MITM/SSRF. + - Redirect loops or chains exceeding ``max_redirects`` raise ``httpx.TooManyRedirects``. + """ headers = _build_headers(extra_headers) tls_verify = getattr(settings, "tls_verify", True) and getattr(settings, "verify_ssl", True) @@ -129,12 +137,27 @@ async def crawl_target( body = body_bytes.decode("utf-8", errors="replace") # Check if redirect and process same-host policy - if response.is_redirect and "location" in response.headers and redirect_count < max_redirects: + if response.is_redirect and "location" in response.headers: + if redirect_count >= max_redirects: + raise httpx.TooManyRedirects( + f"Too many redirects (max_redirects={max_redirects})", + request=httpx.Request("GET", current_url), + ) location = response.headers["location"] next_url = urljoin(current_url, location) - next_hostname = (urlparse(next_url).hostname or "").lower() + next_parsed = urlparse(next_url) + next_scheme = (next_parsed.scheme or "").lower() + next_hostname = (next_parsed.hostname or "").lower() + + if next_scheme not in {"http", "https"} or not next_hostname: + logger.warning( + "Crawler redirect blocked due to unsupported/invalid Location: %s -> %s", + current_url, + next_url, + ) + break - if seed_hostname and next_hostname and next_hostname != seed_hostname: + if seed_hostname and next_hostname != seed_hostname: logger.warning( "Crawler redirect blocked due to host mismatch: %s -> %s (seed host: %s, redirect host: %s)", current_url, diff --git a/testing/backend/unit/test_crawler_security.py b/testing/backend/unit/test_crawler_security.py index 81b5b5f6e..fb3ac76f1 100644 --- a/testing/backend/unit/test_crawler_security.py +++ b/testing/backend/unit/test_crawler_security.py @@ -5,6 +5,7 @@ from __future__ import annotations from unittest.mock import patch, MagicMock +import httpx import pytest from backend.secuscan.config import settings @@ -18,7 +19,7 @@ def test_tls_verify_setting_exists(self): @pytest.mark.asyncio async def test_crawl_target_uses_tls_verify_setting(self): - """crawl_target must construct AsyncClient with verify=tls_verify.""" + """crawl_target must construct AsyncClient with verify=tls_verify (True by default).""" mock_response = MagicMock() mock_response.headers = {} mock_response.is_redirect = False @@ -54,6 +55,60 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): assert client_kwargs.get("verify") is True assert client_kwargs.get("follow_redirects") is False + @pytest.mark.asyncio + async def test_crawl_target_explicit_tls_verify_disabled(self, monkeypatch): + """When tls_verify is explicitly set to False, AsyncClient must receive verify=False.""" + monkeypatch.setattr(settings, "tls_verify", False) + + mock_response = MagicMock() + mock_response.headers = {} + mock_response.is_redirect = False + mock_response.url = "http://example.com" + mock_response.status_code = 200 + + async def fake_aiter_bytes(): + yield b"Unverified" + + mock_response.aiter_bytes = fake_aiter_bytes + + class MockStreamContext: + async def __aenter__(self): + return mock_response + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.return_value = MockStreamContext() + + class MockAsyncClient: + def __init__(self, **kwargs): + self.kwargs = kwargs + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient) as mock_client_cls: + res = await crawl_target("http://example.com") + client_kwargs = mock_client_cls.call_args.kwargs + assert client_kwargs.get("verify") is False + + @pytest.mark.asyncio + async def test_tls_verification_failure_raises_exception(self): + """Failed TLS certificate verification should raise an httpx.ConnectError without hiding errors.""" + class FailingAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + raise httpx.ConnectError("[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed") + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=FailingAsyncClient): + with pytest.raises(httpx.ConnectError) as exc_info: + await crawl_target("https://expired-rsa-dv.ssl.com") + assert "CERTIFICATE_VERIFY_FAILED" in str(exc_info.value) + class TestCrawlerRedirectPolicy: @pytest.mark.asyncio @@ -111,6 +166,57 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): assert result["redirect_chain"][0]["url"] == "http://example.com/start" assert result["redirect_chain"][0]["location"] == "/login" + @pytest.mark.asyncio + async def test_relative_redirect_followed(self): + """Relative path redirects (e.g. ./dashboard) on the same host must be followed.""" + resp1 = MagicMock() + resp1.headers = {"location": "./dashboard"} + resp1.is_redirect = True + resp1.status_code = 301 + resp1.url = "http://example.com/app/main" + + async def fake_aiter_bytes1(): + yield b"Moved Permanently" + resp1.aiter_bytes = fake_aiter_bytes1 + + resp2 = MagicMock() + resp2.headers = {} + resp2.is_redirect = False + resp2.status_code = 200 + resp2.url = "http://example.com/app/dashboard" + + async def fake_aiter_bytes2(): + yield b"Dashboard" + resp2.aiter_bytes = fake_aiter_bytes2 + + responses = [resp1, resp2] + call_index = 0 + + class MockStreamContext: + def __init__(self, url): + nonlocal call_index + self.resp = responses[call_index] + call_index += 1 + async def __aenter__(self): + return self.resp + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.side_effect = lambda method, url: MockStreamContext(url) + + class MockAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient): + result = await crawl_target("http://example.com/app/main") + assert result["final_url"] == "http://example.com/app/dashboard" + @pytest.mark.asyncio async def test_cross_host_redirect_blocked(self): """Redirects to a different hostname (MITM / SSRF attempt) must be blocked.""" @@ -150,3 +256,75 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): assert len(result["redirect_chain"]) == 0 mock_warning.assert_called_once() assert "blocked due to host mismatch" in mock_warning.call_args[0][0] + + @pytest.mark.asyncio + async def test_non_http_scheme_redirect_blocked(self): + """Redirects to non-HTTP/HTTPS schemes (e.g. javascript:, mailto:) must be blocked.""" + resp1 = MagicMock() + resp1.headers = {"location": "javascript:alert(1)"} + resp1.is_redirect = True + resp1.status_code = 302 + resp1.url = "http://example.com/start" + + async def fake_aiter_bytes1(): + yield b"Redirecting..." + resp1.aiter_bytes = fake_aiter_bytes1 + + class MockStreamContext: + async def __aenter__(self): + return resp1 + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.return_value = MockStreamContext() + + class MockAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient): + with patch("backend.secuscan.crawler.logger.warning") as mock_warning: + result = await crawl_target("http://example.com/start") + assert result["final_url"] == "http://example.com/start" + mock_warning.assert_called_once() + assert "blocked due to unsupported/invalid Location" in mock_warning.call_args[0][0] + + @pytest.mark.asyncio + async def test_max_redirects_exceeded_raises_too_many_redirects(self): + """Exceeding max_redirects in a redirect loop must raise httpx.TooManyRedirects.""" + resp1 = MagicMock() + resp1.headers = {"location": "/loop"} + resp1.is_redirect = True + resp1.status_code = 302 + resp1.url = "http://example.com/loop" + + async def fake_aiter_bytes1(): + yield b"Redirecting endlessly..." + resp1.aiter_bytes = fake_aiter_bytes1 + + class MockStreamContext: + async def __aenter__(self): + return resp1 + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + mock_client_instance = MagicMock() + mock_client_instance.stream.return_value = MockStreamContext() + + class MockAsyncClient: + def __init__(self, **kwargs): + pass + async def __aenter__(self): + return mock_client_instance + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + with patch("backend.secuscan.crawler.httpx.AsyncClient", side_effect=MockAsyncClient): + with pytest.raises(httpx.TooManyRedirects) as exc_info: + await crawl_target("http://example.com/loop", max_redirects=3) + assert "max_redirects=3" in str(exc_info.value) From ad5ab8fe0f840136989b104c67358c21ca39f7f0 Mon Sep 17 00:00:00 2001 From: Ashish Pal Date: Fri, 24 Jul 2026 20:16:17 +0530 Subject: [PATCH 3/3] fix(test): un-override require_api_key during auth rejection tests in test_saved_views --- testing/backend/unit/test_saved_views.py | 29 +++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/testing/backend/unit/test_saved_views.py b/testing/backend/unit/test_saved_views.py index 62c615051..81bfaf741 100644 --- a/testing/backend/unit/test_saved_views.py +++ b/testing/backend/unit/test_saved_views.py @@ -54,6 +54,7 @@ async def app_client(): ) as client: client.api_key = api_key client.test_transport = transport + client.app = _app yield client await test_db.disconnect() @@ -357,19 +358,31 @@ async def test_filter_json_with_null_values_rejected(app_client: AsyncClient): @pytest.mark.asyncio async def test_unauthenticated_request_rejected(app_client: AsyncClient): """Requests without a valid API key/session are rejected, not served.""" - res = await app_client.get( - "/api/v1/saved-views", headers={"X-Api-Key": ""} - ) - assert res.status_code == 401 + client_app = getattr(app_client, "app", None) + old_override = client_app.dependency_overrides.pop(require_api_key, None) if client_app else None + try: + res = await app_client.get( + "/api/v1/saved-views", headers={"X-Api-Key": ""} + ) + assert res.status_code == 401 + finally: + if client_app and old_override: + client_app.dependency_overrides[require_api_key] = old_override @pytest.mark.asyncio async def test_wrong_api_key_rejected(app_client: AsyncClient): """A malformed/incorrect API key is rejected.""" - res = await app_client.get( - "/api/v1/saved-views", headers={"X-Api-Key": "not-the-real-key"} - ) - assert res.status_code == 401 + client_app = getattr(app_client, "app", None) + old_override = client_app.dependency_overrides.pop(require_api_key, None) if client_app else None + try: + res = await app_client.get( + "/api/v1/saved-views", headers={"X-Api-Key": "not-the-real-key"} + ) + assert res.status_code == 401 + finally: + if client_app and old_override: + client_app.dependency_overrides[require_api_key] = old_override @pytest.mark.asyncio