fix(api): resolve merge conflicts, add type safety for endpoint_url in S3 connection test
- Resolve merge conflicts with main (PR #834 also fixed S3 SSRF) - Add isinstance(endpoint_url, str) type check before urlparse to prevent TypeError on non-string values - Reject endpoint_url with empty/missing hostname after parsing (malformed URLs like 'https://') - Keep scheme validation (http/https only) and private IP blocking via is_private_ip() - Add logger.warning for SSRF block events - Add regression tests: non-string endpoint_url and empty hostname cases - Update sentinel.md with consolidated SSRF entry Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+4
-4
@@ -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).
|
||||
|
||||
@@ -10,6 +10,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
<!-- version list -->
|
||||
|
||||
## 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
|
||||
|
||||
+18
-6
@@ -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(
|
||||
|
||||
Generated
+6
-6
@@ -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": {
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
Reference in New Issue
Block a user