Skip to content

Commit b510d2a

Browse files
authored
fix(serverless): block SSRF in job-input downloads (#533)
* fix(serverless): block SSRF in job-input downloads Job input carries arbitrary URLs that the worker downloads. Without restriction a job can point the worker at cloud instance-metadata (169.254.169.254) or other non-public addresses (CWE-918). Add runpod/serverless/utils/rp_ssrf.py: an http(s) scheme allowlist, DNS resolution that fails closed on any non-global IP (link-local, loopback, RFC1918, CGNAT, reserved, multicast, IPv6 ULA), a connection adapter that pins the socket to the validated IP to defeat DNS rebinding, per-hop redirect re-validation, and a streamed size cap. Route both rp_download fetch sites through it, and stream file() to disk instead of buffering the whole untrusted body in memory. Blocks raise SSRFError (a ValueError, not a RequestException) so they fail loudly rather than being retried or silently dropped. Configurable via RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS (escape hatch, off by default) and RUNPOD_MAX_DOWNLOAD_BYTES (default 5 GiB). * fix(serverless): close pinned SSRF sessions to prevent FD leaks Address PR #533 review feedback. - Tie each single-use SyncClientSession's lifecycle to its response via _bind_session_to_response, so closing the response (with-block or explicit) also closes the session; without it the session's pools/FDs leaked once per download in long-lived workers. - Close the session on redirect hops and on request exceptions; make teardown best-effort so a cleanup failure cannot mask the caller's real error. - Close response and session deterministically in the pinned-IP adapter test. - Add tests for the request-exception path and the error-masking guarantee. * fix(serverless): tolerate malformed Content-Length in downloads Content-Length is remote-controlled, so int() on it can raise ValueError and abort an otherwise valid download. In file() this was a new failure path (main never parsed the header there); in download_files_from_urls the ValueError also escaped both the backoff retry and the RequestException handler, failing the whole batch. Parse defensively via parse_content_length(), falling back to 0 so chunk sizing still works. The size limit is unaffected: it is enforced while streaming by iter_content_capped(). * fix(serverless): fall back across validated IPs when pinning Pinning to ips[0] turned a multi-address host into a hard failure when the first address is unroutable from the worker: a dual-stack host whose AAAA sorts first is unreachable on a pod with no IPv6 route, where plain requests would have tried the A record. Walk the validated addresses in resolution order, advancing only on connection errors, so every candidate stays pre-validated while the fallback behavior is preserved. Also patch os.makedirs in the file() tests, which were creating a real job_files/ directory in the working tree. * fix(serverless): fail file() on HTTP error responses file() saved whatever body came back, so a 4xx/5xx error page was written as the downloaded file and, when the URL ended in .zip, handed to the extractor. download_files_from_urls already called raise_for_status(); file() never did. Behavior change: file() now raises requests.RequestException on an error status instead of returning a dict pointing at the error page. It has no in-tree callers and is not re-exported from serverless.utils, so this is limited to direct importers. * fix(serverless): bracket IPv6 literals in the pinned Host header urlparse strips the brackets from an IPv6-literal URL host, so the adapter built a malformed Host header ("2606::1:443" instead of "[2606::1]:443"), which RFC 7230 requires and some servers reject. Extracted the authority construction into _host_header_for() and covered every host form. * fix(serverless): refuse proxied fetches that defeat IP pinning Pinning only holds when this process opens the socket. Through a proxy the request carries the hostname and the proxy resolves it, so the validated IP is not what gets dialed and the DNS-rebind protection silently stops applying while still appearing to be in force. Refuse such a URL with SSRFError instead. NO_PROXY exclusions are honored, since those hosts are fetched directly, and RUNPOD_ALLOW_PRIVATE_DOWNLOAD_URLS allows the proxied fetch for deployments that accept the trade-off. Chosen over trust_env=False, which would silently ignore a needed proxy and also drop REQUESTS_CA_BUNDLE.
1 parent bc9f6e1 commit b510d2a

4 files changed

Lines changed: 995 additions & 100 deletions

File tree

runpod/serverless/utils/rp_download.py

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616
import backoff
1717
from requests import RequestException
1818

19-
from runpod.http_client import SyncClientSession
19+
from runpod.serverless.utils.rp_ssrf import (
20+
iter_content_capped,
21+
max_download_bytes,
22+
safe_get,
23+
)
2024

2125
HEADERS = {"User-Agent": "runpod-python/0.0.0 (https://runpod.io; support@runpod.io)"}
2226

@@ -33,6 +37,20 @@ def calculate_chunk_size(file_size: int) -> int:
3337
return 1024 * 1024 * 10 # 10 MB
3438

3539

40+
def parse_content_length(headers) -> int:
41+
"""
42+
Reads Content-Length for chunk sizing, tolerating a missing or malformed value.
43+
44+
The header is remote-controlled, so a non-integer value must not abort an
45+
otherwise valid download; 0 simply yields the smallest chunk size. The real
46+
size limit is enforced while streaming by iter_content_capped().
47+
"""
48+
try:
49+
return int(headers.get("Content-Length", 0))
50+
except (TypeError, ValueError):
51+
return 0
52+
53+
3654
def extract_disposition_params(content_disposition: str) -> Dict[str, str]:
3755
parts = (p.strip() for p in content_disposition.split(";"))
3856

@@ -57,7 +75,7 @@ def download_files_from_urls(job_id: str, urls: Union[str, List[str]]) -> List[s
5775

5876
@backoff.on_exception(backoff.expo, RequestException, max_tries=3)
5977
def download_file(url: str, path_to_save: str) -> str:
60-
with SyncClientSession().get(url, headers=HEADERS, stream=True, timeout=5) as response:
78+
with safe_get(url, stream=True, timeout=5, headers=HEADERS) as response:
6179
response.raise_for_status()
6280
content_disposition = response.headers.get("Content-Disposition")
6381
file_extension = ""
@@ -69,14 +87,12 @@ def download_file(url: str, path_to_save: str) -> str:
6987
if not file_extension:
7088
file_extension = os.path.splitext(urlparse(url).path)[1]
7189

72-
file_size = int(response.headers.get("Content-Length", 0))
73-
chunk_size = calculate_chunk_size(file_size)
90+
chunk_size = calculate_chunk_size(parse_content_length(response.headers))
7491

75-
# write the content in chunks to the file
92+
# write the content in chunks to the file, aborting past the size cap
7693
with open(path_to_save + file_extension, "wb") as file_path:
77-
for chunk in response.iter_content(chunk_size=chunk_size):
78-
if chunk: # filter out keep-alive chunks
79-
file_path.write(chunk)
94+
for chunk in iter_content_capped(response, chunk_size, max_download_bytes()):
95+
file_path.write(chunk)
8096

8197
return file_extension
8298

@@ -117,27 +133,35 @@ def file(file_url: str) -> dict:
117133
"""
118134
os.makedirs("job_files", exist_ok=True)
119135

120-
download_response = SyncClientSession().get(file_url, headers=HEADERS, timeout=30)
136+
with safe_get(file_url, stream=True, timeout=30, headers=HEADERS) as download_response:
137+
# Fail on an error response rather than saving the error page as the
138+
# file; for a .zip that body would go straight to the extractor.
139+
download_response.raise_for_status()
140+
141+
content_disposition = download_response.headers.get("Content-Disposition")
121142

122-
content_disposition = download_response.headers.get("Content-Disposition")
143+
original_file_name = ""
144+
if content_disposition:
145+
params = extract_disposition_params(content_disposition)
123146

124-
original_file_name = ""
125-
if content_disposition:
126-
params = extract_disposition_params(content_disposition)
147+
original_file_name = params.get("filename", "")
127148

128-
original_file_name = params.get("filename", "")
149+
if not original_file_name:
150+
download_path = urlparse(file_url).path
151+
original_file_name = os.path.basename(download_path)
129152

130-
if not original_file_name:
131-
download_path = urlparse(file_url).path
132-
original_file_name = os.path.basename(download_path)
153+
file_type = os.path.splitext(original_file_name)[1].replace(".", "")
133154

134-
file_type = os.path.splitext(original_file_name)[1].replace(".", "")
155+
file_name = f"{uuid.uuid4()}"
135156

136-
file_name = f"{uuid.uuid4()}"
157+
output_file_path = os.path.join("job_files", f"{file_name}.{file_type}")
137158

138-
output_file_path = os.path.join("job_files", f"{file_name}.{file_type}")
139-
with open(output_file_path, "wb") as output_file:
140-
output_file.write(download_response.content)
159+
# Stream to disk in chunks (aborting past the size cap) instead of
160+
# buffering the entire untrusted body in memory.
161+
chunk_size = calculate_chunk_size(parse_content_length(download_response.headers))
162+
with open(output_file_path, "wb") as output_file:
163+
for chunk in iter_content_capped(download_response, chunk_size, max_download_bytes()):
164+
output_file.write(chunk)
141165

142166
if file_type == "zip":
143167
unzipped_directory = os.path.join("job_files", file_name)

0 commit comments

Comments
 (0)