Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 2979302839 |
@@ -28,3 +28,8 @@
|
||||
**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.
|
||||
|
||||
## 2024-05-28 - [SSRF Bypass via Duplicate Keyword Arguments]
|
||||
**Vulnerability:** The `httpx.AsyncClient` initialization in `app/api/url_upload.py` contained duplicate `event_hooks` keyword arguments. Because Python dictionary literals allow duplicate keys but only keep the last one, the `event_hooks={"response": [validate_redirect]}` definition was silently overwritten by the later `event_hooks={"response": [verify_redirect]}` definition. This removed the `validate_redirect` security hook.
|
||||
**Learning:** Duplicate keyword arguments when instantiating Python objects (like `httpx.AsyncClient`) are caught by the interpreter as `SyntaxError: keyword argument repeated`. However, if duplicate keys are used inside a dictionary literal (e.g. `event_hooks={"response": [hook1], "response": [hook2]}`), Python silently overwrites earlier keys. When dealing with configuration options that take a dictionary of lists (like `event_hooks`), multiple related configuration options must be combined into a single list rather than passed via duplicate kwargs or dictionary keys.
|
||||
**Prevention:** When providing multiple event hooks to `httpx.AsyncClient` (e.g., combining global and local SSRF redirect validators), combine them into a single list for the event key (e.g., `event_hooks={'response': [hook1, hook2]}`) to prevent silent overwriting. Linters should be configured to catch duplicate dictionary keys or keyword arguments.
|
||||
|
||||
+2
-1
@@ -14,8 +14,9 @@ filetype>=1.2.0,<2.0 # File type detection fallback (pure Python)
|
||||
dropbox>=11.36.0 # Dropbox integration
|
||||
azure-ai-documentintelligence # Azure OCR service
|
||||
authlib>=1.6.5 # Authentication - fixed security vulnerabilities (GHSA-xxx)
|
||||
python-dotenv # Environment variables
|
||||
python-dotenv>=1.2.2 # Environment variables
|
||||
starlette>=0.49.1 # ASGI toolkit (used by FastAPI) - fixed DoS vulnerability
|
||||
aiohttp>=3.13.4 # Explicitly pin to fix CVE-2026-34515
|
||||
alembic # Database migrations
|
||||
slowapi>=0.1.9 # Rate limiting middleware for FastAPI
|
||||
|
||||
|
||||
@@ -1,79 +0,0 @@
|
||||
import pytest
|
||||
from unittest.mock import patch, MagicMock, AsyncMock
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_redirect_hook_direct():
|
||||
import httpx
|
||||
from fastapi import HTTPException
|
||||
|
||||
# We will test the inline validate_redirect function by calling process_url with a mocked httpx.AsyncClient
|
||||
# that extracts the hook and calls it directly.
|
||||
from app.api.url_upload import process_url
|
||||
|
||||
# We can capture the validate_redirect function by mocking httpx.AsyncClient
|
||||
hook_funcs = []
|
||||
|
||||
class MockAsyncClient:
|
||||
def __init__(self, **kwargs):
|
||||
if "event_hooks" in kwargs and "response" in kwargs["event_hooks"]:
|
||||
hook_funcs.extend(kwargs["event_hooks"]["response"])
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc_val, exc_tb):
|
||||
pass
|
||||
|
||||
def stream(self, method, url):
|
||||
class MockStreamContext:
|
||||
async def __aenter__(self):
|
||||
response = MagicMock()
|
||||
response.headers = {}
|
||||
response.aiter_bytes = AsyncMock(return_value=[])
|
||||
return response
|
||||
async def __aexit__(self, exc_type, exc_val, exc_tb):
|
||||
pass
|
||||
return MockStreamContext()
|
||||
|
||||
with patch("app.api.url_upload.httpx.AsyncClient", new=MockAsyncClient):
|
||||
from app.api.url_upload import URLUploadRequest
|
||||
from fastapi import Request
|
||||
request = MagicMock(spec=Request)
|
||||
url_request = URLUploadRequest(url="http://example.com")
|
||||
|
||||
try:
|
||||
await process_url(request, url_request)
|
||||
except Exception:
|
||||
pass # we just want to get the hooks out
|
||||
|
||||
assert len(hook_funcs) == 2
|
||||
validate_redirect = hook_funcs[0] # it was the first one
|
||||
|
||||
# Now we can test the hook
|
||||
with patch("app.api.url_upload.validate_url_safety", side_effect=HTTPException(status_code=400, detail="bad")):
|
||||
resp = MagicMock(spec=httpx.Response)
|
||||
resp.is_redirect = True
|
||||
resp.headers = {"Location": "http://bad.com"}
|
||||
resp.url = httpx.URL("http://example.com")
|
||||
resp.request = httpx.Request("GET", "http://example.com")
|
||||
|
||||
with pytest.raises(httpx.RequestError) as exc:
|
||||
await validate_redirect(resp)
|
||||
assert "Unsafe redirect target: bad" in str(exc.value)
|
||||
|
||||
with patch("app.api.url_upload.validate_url_safety", return_value=None):
|
||||
resp = MagicMock(spec=httpx.Response)
|
||||
resp.is_redirect = True
|
||||
resp.headers = {"Location": "http://good.com"}
|
||||
resp.url = httpx.URL("http://example.com")
|
||||
resp.request = httpx.Request("GET", "http://example.com")
|
||||
|
||||
await validate_redirect(resp) # should not raise
|
||||
|
||||
# Test no location
|
||||
resp.headers = {}
|
||||
await validate_redirect(resp) # should not raise
|
||||
|
||||
# Test not redirect
|
||||
resp.is_redirect = False
|
||||
await validate_redirect(resp) # should not raise
|
||||
@@ -924,82 +924,3 @@ class TestURLUploadCoverageGaps:
|
||||
|
||||
# Should not raise any exception and should ignore missing Location header
|
||||
await verify_redirect(resp)
|
||||
import pytest
|
||||
from unittest.mock import patch, MagicMock, AsyncMock
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_validate_redirect_hook_direct():
|
||||
import httpx
|
||||
from fastapi import HTTPException
|
||||
|
||||
# We will test the inline validate_redirect function by calling process_url with a mocked httpx.AsyncClient
|
||||
# that extracts the hook and calls it directly.
|
||||
from app.api.url_upload import process_url
|
||||
|
||||
# We can capture the validate_redirect function by mocking httpx.AsyncClient
|
||||
hook_funcs = []
|
||||
|
||||
class MockAsyncClient:
|
||||
def __init__(self, **kwargs):
|
||||
if "event_hooks" in kwargs and "response" in kwargs["event_hooks"]:
|
||||
hook_funcs.extend(kwargs["event_hooks"]["response"])
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, exc_type, exc_val, exc_tb):
|
||||
pass
|
||||
|
||||
def stream(self, method, url):
|
||||
class MockStreamContext:
|
||||
async def __aenter__(self):
|
||||
response = MagicMock()
|
||||
response.headers = {}
|
||||
response.aiter_bytes = AsyncMock(return_value=[])
|
||||
return response
|
||||
async def __aexit__(self, exc_type, exc_val, exc_tb):
|
||||
pass
|
||||
return MockStreamContext()
|
||||
|
||||
with patch("app.api.url_upload.httpx.AsyncClient", new=MockAsyncClient):
|
||||
from app.api.url_upload import URLUploadRequest
|
||||
from fastapi import Request
|
||||
request = MagicMock(spec=Request)
|
||||
url_request = URLUploadRequest(url="http://example.com")
|
||||
|
||||
try:
|
||||
await process_url(request, url_request)
|
||||
except Exception:
|
||||
pass # we just want to get the hooks out
|
||||
|
||||
assert len(hook_funcs) == 2
|
||||
validate_redirect = hook_funcs[0] # it was the first one
|
||||
|
||||
# Now we can test the hook
|
||||
with patch("app.api.url_upload.validate_url_safety", side_effect=HTTPException(status_code=400, detail="bad")):
|
||||
resp = MagicMock(spec=httpx.Response)
|
||||
resp.is_redirect = True
|
||||
resp.headers = {"Location": "http://bad.com"}
|
||||
resp.url = httpx.URL("http://example.com")
|
||||
resp.request = httpx.Request("GET", "http://example.com")
|
||||
|
||||
with pytest.raises(httpx.RequestError) as exc:
|
||||
await validate_redirect(resp)
|
||||
assert "Unsafe redirect target: bad" in str(exc.value)
|
||||
|
||||
with patch("app.api.url_upload.validate_url_safety", return_value=None):
|
||||
resp = MagicMock(spec=httpx.Response)
|
||||
resp.is_redirect = True
|
||||
resp.headers = {"Location": "http://good.com"}
|
||||
resp.url = httpx.URL("http://example.com")
|
||||
resp.request = httpx.Request("GET", "http://example.com")
|
||||
|
||||
await validate_redirect(resp) # should not raise
|
||||
|
||||
# Test no location
|
||||
resp.headers = {}
|
||||
await validate_redirect(resp) # should not raise
|
||||
|
||||
# Test not redirect
|
||||
resp.is_redirect = False
|
||||
await validate_redirect(resp) # should not raise
|
||||
|
||||
Reference in New Issue
Block a user