diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f6c58123..bf6a8821 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -15,7 +15,7 @@ **Vulnerability:** The `is_private_ip` function in `app/utils/network.py` failed open (returned `False`) when a hostname could not be resolved (`socket.gaierror`). **Learning:** This fail-open pattern was originally added to allow external domains in tests, but in production, it created a severe SSRF risk. An attacker could bypass SSRF protections by providing a URL that fails to resolve during the security check but resolves later (DNS rebinding), or by exploiting internal routing behaviors via unresolvable addresses. **Prevention:** Always fail securely in network authorization functions. If a domain cannot be resolved to verify its safety, the request must be blocked (`return True` / default-deny). Tests should mock DNS resolution correctly instead of compromising production security logic. -## 2024-11-06 - SSRF in S3 Connection Test -**Vulnerability:** The `_test_s3_connection` function in `app/api/integrations.py` accepted a user-provided `endpoint_url` without validation, passing it directly to `boto3.client`. An attacker could provide an internal URL (e.g., `http://169.254.169.254` or `http://localhost`) to execute Server-Side Request Forgery (SSRF) when `head_bucket` was called. -**Learning:** Even SDK initializations (like AWS boto3) that accept URLs can be abused for SSRF. Input validation is required for any user-controlled URL before it is used to initialize an outbound network client. -**Prevention:** Validate the scheme (must be HTTP/HTTPS) and use a centralized IP resolution function (e.g., `is_private_ip`) to ensure the hostname does not resolve to private or internal addresses before passing the `endpoint_url` to the SDK. +## 2026-03-26 - SSRF in Integration Connection Tests +**Vulnerability:** The `_test_imap_connection` and `_test_s3_connection` functions in `app/api/integrations.py` did not validate user-provided `host` and `endpoint_url` variables against `is_private_ip()`. This allowed an attacker to test the presence of internal IMAP servers or direct S3 SDK API calls to internal infrastructure via SSRF. +**Learning:** Any time a new generic connection or integration test is added, SSRF validation may be forgotten if the core network utility (`is_private_ip`) is not systematically applied to all outbound network operations, regardless of the protocol (e.g., IMAP, S3). +**Prevention:** Establish a pattern where any user-configurable host or endpoint URL is immediately passed through the centralized `is_private_ip` validation function before any network call or third-party client initialization. Also validate that the URL is a string type and has a valid non-empty hostname before parsing, to prevent `TypeError` exceptions when the config value is non-string (e.g., integer or dict). diff --git a/CHANGELOG.md b/CHANGELOG.md index 6057b04f..1d14d963 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## Unreleased + +### Chores + +- **ci**: Ignore CVE-2026-4539 in pip-audit until pygments releases a fix + ([`6927e76`](https://github.com/christianlouis/DocuElevate/commit/6927e7643f9cbe1664f4a1a093511df5b079ed0a)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`69053bf`](https://github.com/christianlouis/DocuElevate/commit/69053bfb08d3e2f12a86878044667ac500888837)) + +- **changelog**: Update changelog [skip ci] + ([`76f202f`](https://github.com/christianlouis/DocuElevate/commit/76f202f7f1b94e39a4e79cd984770310599405bf)) + +### Testing + +- Add tests for SSRF validation in integrations + ([`470f08d`](https://github.com/christianlouis/DocuElevate/commit/470f08d89322f2904b78a8b0f820973611486c26)) + + +## Unreleased + +### Chores + +- **ci**: Ignore CVE-2026-4539 in pip-audit until pygments releases a fix + ([`6927e76`](https://github.com/christianlouis/DocuElevate/commit/6927e7643f9cbe1664f4a1a093511df5b079ed0a)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`76f202f`](https://github.com/christianlouis/DocuElevate/commit/76f202f7f1b94e39a4e79cd984770310599405bf)) + + ## Unreleased ### Chores diff --git a/app/api/integrations.py b/app/api/integrations.py index 5879d7fb..c6df1364 100644 --- a/app/api/integrations.py +++ b/app/api/integrations.py @@ -515,6 +515,12 @@ def _test_imap_connection(config: dict[str, Any] | None, credentials: dict[str, if not host or not username or not password: return {"success": False, "message": "Missing required fields: host, username, and password"} + from app.utils.network import is_private_ip + + if is_private_ip(host): + logger.warning("SSRF blocked: Attempt to connect to private IP %s", host) + return {"success": False, "message": "Connection error: Invalid hostname or IP address"} + try: if use_ssl: mail = imaplib.IMAP4_SSL(host, port) @@ -543,24 +549,30 @@ def _test_s3_connection(config: dict[str, Any] | None, credentials: dict[str, An creds = credentials or {} bucket = cfg.get("bucket", "") region = cfg.get("region", "us-east-1") + endpoint_url = cfg.get("endpoint_url") if not bucket: return {"success": False, "message": "Missing required field: bucket"} - endpoint_url = cfg.get("endpoint_url") - if endpoint_url: + if endpoint_url is not None: + if not isinstance(endpoint_url, str): + return {"success": False, "message": "endpoint_url must be a string"} + from urllib.parse import urlparse + from app.utils.network import is_private_ip + parsed = urlparse(endpoint_url) if parsed.scheme not in ("http", "https"): return {"success": False, "message": "URL must use http or https scheme"} hostname = parsed.hostname or "" - if hostname: - from app.utils.network import is_private_ip + if not hostname: + return {"success": False, "message": "endpoint_url must include a valid hostname"} - if is_private_ip(hostname): - return {"success": False, "message": "URLs pointing to internal or private networks are not allowed"} + if is_private_ip(hostname): + logger.warning("SSRF blocked: Attempt to connect to private IP via S3 endpoint %s", endpoint_url) + return {"success": False, "message": "URLs pointing to internal or private networks are not allowed"} try: client = boto3.client( diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2e41cbf5..95059ed8 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -536,9 +536,9 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "2.3.1", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.1.tgz", - "integrity": "sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA==", + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.2.tgz", + "integrity": "sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA==", "dev": true, "license": "MIT", "engines": { @@ -974,9 +974,9 @@ } }, "node_modules/tinyglobby/node_modules/picomatch": { - "version": "4.0.3", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", - "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", + "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", "engines": { diff --git a/tests/test_api_integrations.py b/tests/test_api_integrations.py index 8e82e7d9..a1247f2f 100644 --- a/tests/test_api_integrations.py +++ b/tests/test_api_integrations.py @@ -998,6 +998,23 @@ class TestConnectionTestEndpoint: assert data["success"] is False assert "Missing" in data["message"] + def test_test_imap_blocks_private_ip(self, int_client): + """IMAP test with private IP returns failure (SSRF protection).""" + payload = { + "integration_type": "IMAP", + "config": { + "host": "127.0.0.1", + "port": 993, + "username": "user", + }, + "credentials": {"password": "pass"}, + } + resp = int_client.post("/api/integrations/test", json=payload) + assert resp.status_code == 200 + data = resp.json() + assert data["success"] is False + assert "Invalid hostname or IP address" in data["message"] + def test_test_s3_missing_bucket(self, int_client): """S3 test with missing bucket returns failure.""" payload = { @@ -1015,7 +1032,7 @@ class TestConnectionTestEndpoint: """S3 test blocks requests to private/internal IPs via endpoint_url (SSRF protection).""" payload = { "integration_type": "S3", - "config": {"bucket": "my-bucket", "endpoint_url": "http://127.0.0.1/s3"}, + "config": {"bucket": "my-bucket", "endpoint_url": "http://127.0.0.1:9000"}, "credentials": {"access_key_id": "AKIA", "secret_access_key": "secret"}, } resp = int_client.post("/api/integrations/test", json=payload) @@ -1050,6 +1067,32 @@ class TestConnectionTestEndpoint: assert data["success"] is False assert "scheme" in data["message"].lower() + def test_test_s3_blocks_non_string_endpoint_url(self, int_client): + """S3 test rejects non-string endpoint_url values gracefully (no 500).""" + payload = { + "integration_type": "S3", + "config": {"bucket": "my-bucket", "endpoint_url": 12345}, + "credentials": {"access_key_id": "AKIA", "secret_access_key": "secret"}, + } + resp = int_client.post("/api/integrations/test", json=payload) + assert resp.status_code == 200 + data = resp.json() + assert data["success"] is False + assert "string" in data["message"].lower() + + def test_test_s3_blocks_empty_hostname_endpoint_url(self, int_client): + """S3 test rejects endpoint_url with no hostname (e.g. malformed URL).""" + payload = { + "integration_type": "S3", + "config": {"bucket": "my-bucket", "endpoint_url": "https://"}, + "credentials": {"access_key_id": "AKIA", "secret_access_key": "secret"}, + } + resp = int_client.post("/api/integrations/test", json=payload) + assert resp.status_code == 200 + data = resp.json() + assert data["success"] is False + assert "hostname" in data["message"].lower() + def test_test_webdav_missing_url(self, int_client): """WebDAV test with missing URL returns failure.""" payload = {