Compare commits

..

1 Commits

Author SHA1 Message Date
google-labs-jules[bot] c5c5284bd0 🛡️ Sentinel: [HIGH] Fix SSRF bypass via HTTP redirects in URL upload
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
2026-05-17 13:37:55 +00:00
11 changed files with 25 additions and 211 deletions
+4 -9
View File
@@ -19,12 +19,7 @@
**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.
## 2024-05-27 - SSRF Bypass via HTTP Redirects
**Vulnerability:** In `app/api/url_upload.py`, the `validate_url_safety` function was correctly verifying the initially requested URL to prevent fetching internal IPs or cloud metadata endpoints. However, the subsequent `httpx.AsyncClient` was configured with `follow_redirects=True` without validating the destination of those redirects. An attacker could bypass SSRF protections by providing a URL to an attacker-controlled server that responds with a 301/302 redirect pointing to an internal target (e.g., `http://127.0.0.1` or `http://169.254.169.254`).
**Learning:** Checking the URL before sending the request is insufficient if the HTTP client automatically follows redirects. The target of every single redirect must be subject to the same strict validation as the initial request.
**Prevention:** Avoid `follow_redirects=True` for user-provided URLs when possible. If redirects must be followed, attach an event hook (e.g., `event_hooks={"response": [hook_function]}`) to the `httpx` client to intercept the response, calculate the redirect destination from the `Location` header, and run the URL safety validation logic before the redirect is actually followed.
## 2026-03-27 - SSRF Bypass via HTTP Redirects in httpx
**Vulnerability:** The `/process-url` endpoint used `httpx.AsyncClient(follow_redirects=True)` after validating the initial user-provided URL against SSRF protections. However, it did not validate the target URLs of any subsequent HTTP redirects, allowing an attacker to provide a safe URL that redirects to an internal/private IP, bypassing the security check.
**Learning:** Initial URL validation is insufficient when the HTTP client is configured to follow redirects automatically. The client must be explicitly configured to validate every redirect target.
**Prevention:** When using `httpx.AsyncClient(follow_redirects=True)` for user-provided URLs, always implement a redirect validator hook function (e.g., using `event_hooks={'response': [validate_redirect]}`) that resolves the `Location` header and passes it through the same SSRF validation logic before the redirect is followed.
## 2026-03-27 - SSRF Bypass via HTTP Redirects
**Vulnerability:** The `process_url` endpoint in `app/api/url_upload.py` used `httpx.AsyncClient` with `follow_redirects=True`. While the initial user-provided URL was validated against SSRF, if the remote server returned an HTTP redirect to an internal IP (like 127.0.0.1 or an AWS metadata endpoint), the HTTP client would automatically follow the redirect without validating the new target URL.
**Learning:** Initial URL validation is insufficient if the HTTP client automatically follows redirects. Attackers can easily set up external servers that respond with `302 Found` pointing to internal network addresses.
**Prevention:** If `follow_redirects=True` is required, always implement an event hook (e.g., `event_hooks={"response": [check_redirect]}`) to intercept redirect responses, extract the `Location` header, and validate the target URL using `is_private_ip` or `validate_url_safety` before the client follows it.
+1 -1
View File
@@ -1 +1 @@
2026-04-07T09:34:53Z
2026-03-25T07:54:28Z
-70
View File
@@ -12,76 +12,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Unreleased
## v0.172.9 (2026-04-07)
### Bug Fixes
- **api**: Resolve merge conflicts, add type safety for endpoint_url in S3 connection test
([`57db4c7`](https://github.com/christianlouis/DocuElevate/commit/57db4c7c82f4a8df2e7e5e5505e1d5c01768fc16))
### Chores
- **ci**: Ignore CVE-2026-4539 in pip-audit until pygments releases a fix
([`6927e76`](https://github.com/christianlouis/DocuElevate/commit/6927e7643f9cbe1664f4a1a093511df5b079ed0a))
### Code Style
- Apply ruff auto-fix
([`8295279`](https://github.com/christianlouis/DocuElevate/commit/8295279ec93570da4eb0445ede8084d1eb2aba99))
- Sort imports in test_url_upload.py
([`bdfa3ba`](https://github.com/christianlouis/DocuElevate/commit/bdfa3ba1e0a5702414e3b449fbde6a6d3149557a))
### Documentation
- **changelog**: Update changelog [skip ci]
([`c6e0b80`](https://github.com/christianlouis/DocuElevate/commit/c6e0b80becab81a75aea4ee78f5aaf8b6ac54854))
- **changelog**: Update changelog [skip ci]
([`9b9882c`](https://github.com/christianlouis/DocuElevate/commit/9b9882c4d62691d0ddd20444e3b77bfe6eecc8c3))
- **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 coverage for url_upload redirect SSRF bypass prevention hook
([`152ee15`](https://github.com/christianlouis/DocuElevate/commit/152ee15b06ebf7beb6216423b4c8d93ec2243165))
- 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]
([`9b9882c`](https://github.com/christianlouis/DocuElevate/commit/9b9882c4d62691d0ddd20444e3b77bfe6eecc8c3))
- **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
+1 -1
View File
@@ -1 +1 @@
3bd8a52
12a35f9
+6 -6
View File
@@ -1,10 +1,10 @@
DocuElevate Build Information
==============================
Version: 0.172.9
Build Date: 2026-04-07T09:34:53Z
Git Commit: 3bd8a52ea201b33d6071c9b3a7fdace582e65fd5
Git Short SHA: 3bd8a52
Version: 0.172.8
Build Date: 2026-03-25T07:54:28Z
Git Commit: 12a35f9b301a5a4430265f32f6552bd131e0d5e2
Git Short SHA: 12a35f9
Git Branch: main
Commit Date: 2026-04-07T11:34:28+02:00
Build Timestamp: 2026-04-07T09:34:53Z
Commit Date: 2026-03-25T08:54:06+01:00
Build Timestamp: 2026-03-25T07:54:28Z
==============================
+1 -1
View File
@@ -1 +1 @@
0.172.9
0.172.8
+10 -33
View File
@@ -106,25 +106,6 @@ def validate_file_type(content_type: str, filename: str) -> bool:
return False
async def verify_redirect(response: httpx.Response) -> None:
"""
Event hook to intercept redirects and validate the new destination URL.
Prevents SSRF bypasses via redirects to internal networks or metadata endpoints.
"""
if response.status_code in (301, 302, 303, 307, 308):
location = response.headers.get("Location")
if location:
# Resolve relative redirects
new_url = str(response.url.join(location))
# Validate the new URL
try:
validate_url_safety(new_url)
except HTTPException as e:
# Map the validation error to an httpx exception so it can be handled
# properly by the caller, avoiding raw HTTPExceptions escaping the client scope
raise httpx.RequestError(f"Redirect to unsafe URL blocked: {e.detail}", request=response.request) from e
@router.post("/process-url")
@require_login
async def process_url(
@@ -156,6 +137,15 @@ async def process_url(
# Validate URL safety (SSRF protection)
validate_url_safety(url)
# Event hook to intercept and validate redirects
async def check_redirect(response: httpx.Response):
if response.is_redirect:
location = response.headers.get("Location")
if location:
redirect_url = str(response.url.join(location))
# Validate the redirect target
validate_url_safety(redirect_url)
# Parse URL to extract filename if not provided
if url_request.filename:
original_filename = url_request.filename
@@ -170,19 +160,6 @@ async def process_url(
if not safe_filename:
safe_filename = "download"
# Hook to validate redirects and prevent SSRF
async def validate_redirect(response: httpx.Response):
if response.is_redirect:
location = response.headers.get("Location")
if location:
# Resolve relative URLs
next_url = urllib.parse.urljoin(str(response.url), location)
try:
validate_url_safety(next_url)
except HTTPException as e:
# Reraise as a RequestError so httpx aborts the request
raise httpx.RequestError(f"Unsafe redirect target: {e.detail}", request=response.request)
# Download file with security measures
# Initialize target_path to None to prevent UnboundLocalError in exception handlers
# that may execute before target_path is assigned during error cases
@@ -197,7 +174,7 @@ async def process_url(
headers={
"User-Agent": "DocuElevate/1.0", # Identify ourselves
},
event_hooks={"response": [validate_redirect, verify_redirect]},
event_hooks={"response": [check_redirect]},
) as client:
async with client.stream("GET", url) as response:
response.raise_for_status()
-34
View File
@@ -1,34 +0,0 @@
import sys
from unittest.mock import MagicMock
from fastapi.templating import Jinja2Templates
import os
# We don't really need a real path, but let's mock it
os.makedirs("templates", exist_ok=True)
with open("templates/files.html", "w") as f:
f.write("Hello")
templates = Jinja2Templates(directory="templates")
original_template_response = templates.TemplateResponse
def template_response_with_version(*args, **kwargs):
if len(args) == 2 and isinstance(args[0], str) and isinstance(args[1], dict):
context = args[1]
request = context.get("request")
if request is not None:
# THIS IS MY FIX
print("Running fix logic")
return original_template_response(request=request, name=args[0], context=context, **kwargs)
print("Running original fallback logic")
return original_template_response(*args, **kwargs)
templates.TemplateResponse = template_response_with_version
req = MagicMock()
try:
templates.TemplateResponse("files.html", {"request": req})
print("SUCCESS")
except Exception as e:
import traceback
traceback.print_exc()
+1 -1
View File
@@ -34,7 +34,7 @@ pip-audit>=2.7.0 # Dependency vulnerability scanning against OSV/PyPA advisory
pre-commit>=3.6.0
# License compliance
pip-licenses==5.5.5 # For license compliance checking
pip-licenses==5.5.1 # For license compliance checking
# Release automation
python-semantic-release>=9.0.0
+1 -1
View File
@@ -51,7 +51,7 @@ pytesseract>=0.3.10 # Python wrapper for Tesseract OCR
pdf2image>=1.17.0 # Convert PDF pages to images (used by Tesseract and EasyOCR providers)
ocrmypdf>=16.0.0,<18.0.0 # Post-processing: embeds searchable text layers into PDFs via Tesseract
meilisearch>=0.31.0 # Full-text search engine client
stripe>=7.0.0,<16.0.0 # Stripe billing SDK (MIT license)
stripe>=7.0.0,<15.0.0 # Stripe billing SDK (MIT license)
# Error and performance monitoring
sentry-sdk[fastapi,celery,sqlalchemy]>=2.20.0,<3.0.0
-54
View File
@@ -879,57 +879,3 @@ class TestURLUploadCoverageGaps:
# Generic exception (not HTTPException/OSError/RequestException) is caught and returns 500
assert response.status_code == 500
assert "Unexpected error" in response.json()["detail"]
@pytest.mark.asyncio
async def test_verify_redirect_allows_safe_url(self):
"""Test verify_redirect allows safe redirects (lines 115, 118, 120-121)"""
import httpx
from app.api.url_upload import verify_redirect
req = httpx.Request("GET", "http://example.com")
resp = httpx.Response(301, headers={"Location": "https://google.com"}, request=req)
# Should not raise any exception
await verify_redirect(resp)
@pytest.mark.asyncio
@patch("app.api.url_upload.validate_url_safety")
async def test_verify_redirect_blocks_unsafe_url(self, mock_validate):
"""Test verify_redirect blocks unsafe redirects (lines 122-125)"""
import httpx
from fastapi import HTTPException
from app.api.url_upload import verify_redirect
mock_validate.side_effect = HTTPException(status_code=400, detail="Unsafe URL")
req = httpx.Request("GET", "http://example.com")
resp = httpx.Response(301, headers={"Location": "http://127.0.0.1"}, request=req)
with pytest.raises(httpx.RequestError) as exc_info:
await verify_redirect(resp)
assert "Redirect to unsafe URL blocked" in str(exc_info.value)
@pytest.mark.asyncio
async def test_verify_redirect_ignores_non_redirects(self):
"""Test verify_redirect ignores 200 OK responses"""
import httpx
from app.api.url_upload import verify_redirect
req = httpx.Request("GET", "http://example.com")
resp = httpx.Response(200, request=req)
# Should not raise any exception and should ignore missing Location header
await verify_redirect(resp)
@patch("app.api.url_upload.httpx.AsyncClient")
def test_client_init_combines_hooks(self, mock_client, client):
"""Test that httpx.AsyncClient is initialized with combined event hooks"""
response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"})
# we cannot easily assert the exact functions inside event_hooks closure/local function definition
# so we will just test it initializes without error, and coverage will hit the single event_hooks line