Fix arbitrary file read and SSRF in Hunyuan3D integration#205
Conversation
Add input validation to prevent two security vulnerabilities in the Hunyuan3D tools: 1. Arbitrary file read via generate_hunyuan3d_model (fixes ahujasid#202): Local file paths passed as input_image_url were read without validation, allowing any file accessible by Blender to be read and exfiltrated via the API. Now validates that local paths have image file extensions and resolves symlinks before reading. 2. SSRF via import_generated_asset_hunyuan (fixes ahujasid#203): URLs were only checked for http/https prefix, allowing requests to internal services and cloud metadata endpoints. Now resolves hostnames and blocks requests to private, loopback, and link-local addresses. Validation is applied at both the MCP server layer and the Blender addon layer for defense in depth.
📝 WalkthroughWalkthroughAdded image-path and URL validation to prevent local file reads and SSRF. New constants and helper functions are integrated into image handling and Hunyuan asset import flows; URL resolution can pin downloads to validated IPs and local image reads are size/extension-checked. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server
participant Blender_Addon
participant DNS as "DNS Resolver"
participant External as "External Host / IP"
Client->>MCP_Server: request import/generated model (url or local path)
MCP_Server->>Blender_Addon: send_command(..., image/url)
Blender_Addon->>DNS: resolve(url)
DNS-->>Blender_Addon: IPs
Blender_Addon->>Blender_Addon: validate_url_not_internal(url, IPs)
alt URL is internal/unsafe
Blender_Addon-->>MCP_Server: return error (blocked)
MCP_Server-->>Client: error response
else URL is safe
Blender_Addon->>External: HTTP request to pinned IP (Host header set)
External-->>Blender_Addon: content / zip
Blender_Addon-->>MCP_Server: import result
MCP_Server-->>Client: success response
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoAdd security validations for Hunyuan3D file and URL handling
WalkthroughsDescription• Adds input validation to prevent arbitrary file read vulnerability in generate_hunyuan3d_model - Validates local image paths against allowed extensions - Resolves symlinks before file access • Adds SSRF protection to import_generated_asset_hunyuan by blocking private/internal addresses - Resolves hostnames and rejects private, loopback, link-local, and reserved IP ranges • Implements defense-in-depth with validation at both MCP server and Blender addon layers Diagramflowchart LR
A["User Input"] --> B["Addon Layer"]
B --> C["validate_image_path"]
B --> D["validate_url_not_internal"]
C --> E["Check Extension & Resolve Symlinks"]
D --> F["Resolve Hostname & Check IP Range"]
E --> G["Server Layer"]
F --> G
G --> H["_validate_image_path"]
G --> I["_validate_url_not_internal"]
H --> J["Safe File Access"]
I --> K["Safe URL Request"]
File Changes1. addon.py
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| with open(os.path.realpath(image), "rb") as f: | ||
| image_base64 = base64.b64encode(f.read()).decode("ascii") | ||
| data["image"] = image_base64 |
There was a problem hiding this comment.
2. Local base64 posted to url 📎 Requirement gap ⛨ Security
In LOCAL_API mode, the addon base64-encodes local file contents and sends them to
requests.post(f"{base_url}/generate", ...) where base_url is user-configurable. This still
allows exfiltration of locally read file contents to arbitrary endpoints.
Agent Prompt
## Issue description
The addon still base64-encodes local file contents and transmits them via HTTP to a user-configurable `base_url`, enabling data exfiltration.
## Issue Context
Compliance requires constraining outbound requests so attacker-controlled endpoints cannot be used for exfiltration, or preventing local-file-derived data from being transmitted.
## Fix Focus Areas
- addon.py[2172-2221]
- addon.py[2207-2214]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
addon.py (2)
2327-2329:⚠️ Potential issue | 🔴 CriticalReject traversal entries before calling
extractall().
zip_file_urlis still attacker-influenced input. After the SSRF filter, an attacker can host a ZIP containing../...or absolute paths, andextractall(temp_dir)will write outside the temporary directory. That turns this import path into an arbitrary file write on the Blender host.🧩 Proposed fix
# Unzip the ZIP with zipfile.ZipFile(zip_file_path, "r") as zip_ref: - zip_ref.extractall(temp_dir) + abs_temp_dir = os.path.abspath(temp_dir) + for member in zip_ref.infolist(): + target_path = os.path.abspath( + os.path.join(temp_dir, os.path.normpath(member.filename)) + ) + if not target_path.startswith(abs_temp_dir + os.sep): + return {"succeed": False, "error": "ZIP contains path traversal entries"} + zip_ref.extractall(temp_dir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@addon.py` around lines 2327 - 2329, Reject path-traversal entries in the ZIP before calling extractall by replacing the direct zip_ref.extractall(temp_dir) call: open the archive with zipfile.ZipFile(zip_file_path, "r") and iterate over zip_ref.infolist() or zip_ref.namelist(), for each member compute a safe target path by joining temp_dir with the member name, reject any member whose name is absolute or contains '..' components or whose normalized target path does not start with the normalized temp_dir, and only then extract the allowed members (e.g., using zip_ref.open() and writing to the validated path); reference zipfile.ZipFile, zip_ref.extractall, zip_ref.infolist()/namelist(), zip_file_path, and temp_dir when making the changes.
2197-2205:⚠️ Potential issue | 🔴 CriticalRemote image inputs still SSRF the
LOCAL_APIpath.The new guard only covers local files. When
imageis an HTTP(S) URL, this branch still downloads it directly from attacker-controlled input, sogenerate_hunyuan3d_model(..., input_image_url="http://127.0.0.1:...")can still hit internal services or cloud metadata from the Blender host.🛡️ Proposed fix
if image: if re.match(r'^https?://', image, re.IGNORECASE) is not None: + ssrf_err = validate_url_not_internal(image) + if ssrf_err: + return {"error": ssrf_err} try: - resImg = requests.get(image) + resImg = requests.get(image, timeout=30) resImg.raise_for_status() image_base64 = base64.b64encode(resImg.content).decode("ascii") data["image"] = image_base64🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@addon.py` around lines 2197 - 2205, The HTTP(S) image download path currently calls requests.get(image) directly and allows SSRF to LOCAL_API/internal services; before performing requests.get (the branch handling the variable image and building data["image"]), validate and reject URLs that resolve to loopback, private, link-local, or cloud metadata ranges: parse the image URL (image), resolve its hostname to IP(s) (e.g., via socket.getaddrinfo), check each IP against allowed public ranges (reject 127.0.0.0/8, ::1, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, cloud metadata CIDRs, and IPv6 equivalents), and only then call requests.get with a short timeout and no redirects; also ensure generate_hunyuan3d_model(...) callers that pass input_image_url are protected by the same validation path (reference the image variable, requests.get call, and data["image"] assignment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@addon.py`:
- Around line 32-46: Add a file-size cap to prevent reading and base64-encoding
huge local images: define a MAX_IMAGE_BYTES constant (e.g., a few MB) and,
inside validate_image_path(path: str) (and any call sites that currently do
f.read() before encoding), check os.path.getsize(resolved) and return an error
string like "File too large: X bytes (max Y bytes)" if it exceeds the cap;
additionally, update the code paths that call f.read() for Hunyuan to perform
the same size check before reading and base64.b64encode to avoid buffering
multi-GB files. Ensure you reference ALLOWED_IMAGE_EXTENSIONS and
validate_image_path when adding the new check so callers can reuse the
validation.
---
Outside diff comments:
In `@addon.py`:
- Around line 2327-2329: Reject path-traversal entries in the ZIP before calling
extractall by replacing the direct zip_ref.extractall(temp_dir) call: open the
archive with zipfile.ZipFile(zip_file_path, "r") and iterate over
zip_ref.infolist() or zip_ref.namelist(), for each member compute a safe target
path by joining temp_dir with the member name, reject any member whose name is
absolute or contains '..' components or whose normalized target path does not
start with the normalized temp_dir, and only then extract the allowed members
(e.g., using zip_ref.open() and writing to the validated path); reference
zipfile.ZipFile, zip_ref.extractall, zip_ref.infolist()/namelist(),
zip_file_path, and temp_dir when making the changes.
- Around line 2197-2205: The HTTP(S) image download path currently calls
requests.get(image) directly and allows SSRF to LOCAL_API/internal services;
before performing requests.get (the branch handling the variable image and
building data["image"]), validate and reject URLs that resolve to loopback,
private, link-local, or cloud metadata ranges: parse the image URL (image),
resolve its hostname to IP(s) (e.g., via socket.getaddrinfo), check each IP
against allowed public ranges (reject 127.0.0.0/8, ::1, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, cloud metadata CIDRs, and IPv6
equivalents), and only then call requests.get with a short timeout and no
redirects; also ensure generate_hunyuan3d_model(...) callers that pass
input_image_url are protected by the same validation path (reference the image
variable, requests.get call, and data["image"] assignment).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9badf8c-cb82-4291-a70e-0bbacccd4ad4
📒 Files selected for processing (2)
addon.pysrc/blender_mcp/server.py
| def _validate_url_not_internal(url: str) -> str | None: | ||
| """Check that a URL does not target private/loopback/link-local addresses. | ||
|
|
||
| Returns None if the URL is safe, or an error message string otherwise. | ||
| """ | ||
| parsed = urlparse(url) | ||
| hostname = parsed.hostname | ||
| if not hostname: | ||
| return "URL has no hostname" | ||
| try: | ||
| addr_infos = socket.getaddrinfo(hostname, parsed.port or 443) | ||
| except socket.gaierror: | ||
| return f"Could not resolve hostname: {hostname}" | ||
| for family, _, _, _, sockaddr in addr_infos: | ||
| ip = ipaddress.ip_address(sockaddr[0]) | ||
| if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_reserved: | ||
| return f"URL resolves to a non-public address ({ip}), request blocked" | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the validation helpers and the later HTTP sinks that still use the original hostname.
rg -n -C2 '_validate_url_not_internal|validate_url_not_internal|requests\.get\(zip_file_url|requests\.get\(image' src/blender_mcp/server.py addon.pyRepository: ahujasid/blender-mcp
Length of output: 1844
🏁 Script executed:
# Get the complete validate_url_not_internal function in addon.py
sed -n '49,80p' addon.pyRepository: ahujasid/blender-mcp
Length of output: 1230
🏁 Script executed:
# Check if _validate_url_not_internal is used in download functions and if connection is pinned
rg -n -A 10 'def import_generated_asset_hunyuan|zip_response = requests.get' src/blender_mcp/server.py addon.py | head -60Repository: ahujasid/blender-mcp
Length of output: 2222
🏁 Script executed:
# Check if there's any session or connection pinning between validation and download
rg -n 'Session|session\.|adapter|mount|resolve' src/blender_mcp/server.py addon.py | grep -v '#'Repository: ahujasid/blender-mcp
Length of output: 785
DNS rebinding attack bypasses SSRF validation—pin IPs or use connection pooling.
The validation function resolves the hostname once via socket.getaddrinfo(), but the actual HTTP downloads (lines 2321 in addon.py and equivalent in server.py) call requests.get() independently, triggering a fresh DNS lookup. An attacker controlling the DNS can serve a public IP for validation, then return a private IP (127.0.0.1, 169.254.169.254, etc.) at download time. Extract and pin the validated IP address through the download, or restrict URLs to a trusted allowlist in both validation and request layers.
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 52-52: Loop control variable family not used within loop body
Rename unused family to _family
(B007)
- Add MAX_LOCAL_IMAGE_BYTES (10 MB) file size cap to validate_image_path() to prevent base64-encoding oversized files from exhausting memory - Return resolved IPs from validate_url_not_internal() and pin them for the actual HTTP request, closing the DNS rebinding TOCTOU window - Add missing SSRF validation before requests.get(image) in create_hunyuan_job_local_site() and create_hunyuan_job_main_site() Applied consistently to both addon.py and server.py.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/blender_mcp/server.py (1)
27-75: Consider extracting the validators into a shared module.
_validate_image_path()and_validate_url_not_internal()now exist in bothsrc/blender_mcp/server.pyandaddon.py. For security checks, duplicated logic tends to drift; a small stdlib-only helper would keep both layers aligned without losing defense-in-depth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/blender_mcp/server.py` around lines 27 - 75, The two validators _validate_image_path and _validate_url_not_internal are duplicated between server.py and addon.py; extract them into a single stdlib-only helper module (e.g., validators) and replace the copies with imports in both places so logic stays in one location; preserve the exact function signatures and return types (including use of ALLOWED_IMAGE_EXTENSIONS and MAX_LOCAL_IMAGE_BYTES), move any required imports (os, socket, ipaddress, urlparse, etc.) into the new module, and update server.py and addon.py to import and call validators._validate_image_path and validators._validate_url_not_internal so both layers use the same implementation while still allowing each layer to apply defense-in-depth if desired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@addon.py`:
- Around line 2224-2234: The URL repinning using str.replace on
f"{parsed_img.scheme}://{parsed_img.hostname}" fails for userinfo and IPv6
cases; instead reconstruct the netloc from parsed components so userinfo and
port are preserved and IPv6 addresses are bracketed, then rebuild the URL and
use that for requests.get (same fix for the ZIP URL handling). Concretely, use
parsed = urlparse(image) (and parsed_zip), build userinfo =
f"{parsed.username}:{parsed.password}" (omit empty parts), determine host =
validated_ips[0] and wrap in brackets if host contains ':' (IPv6), append
f":{parsed.port}" when present, set new_netloc = f"{userinfo + '@' if userinfo
else ''}{host}{port}", then create pinned_img_url =
parsed._replace(netloc=new_netloc).geturl() and call
requests.get(pinned_img_url, headers={"Host": parsed.hostname}) (and mirror for
the ZIP code paths).
- Around line 2231-2237: The requests.get calls (e.g., the image fetch using
pinned_img_url/parsed_img and resImg -> data["image"], and the subsequent ZIP
download/ZipFile handling) currently allow redirects and load unlimited bytes;
change these requests.get calls to use allow_redirects=False and a timeout
(e.g., timeout=30), and if you must follow redirects implement per-hop URL
revalidation; replace direct .content buffering with streaming reads that
enforce a maximum byte limit (read up to MAX_IMAGE_BYTES before base64-encoding
into data["image"], and up to MAX_ZIP_BYTES before passing to zipfile), and for
ZIP extraction validate each member (check ZipInfo.file_size/uncompressed size,
sanitize names to prevent path traversal, and enforce total extracted size
limits) before writing/extracting. Ensure you reference the existing
variables/functions (pinned_img_url, parsed_img, resImg, data["image"],
res_zip/zip_bytes/ZipFile) when applying these changes.
---
Nitpick comments:
In `@src/blender_mcp/server.py`:
- Around line 27-75: The two validators _validate_image_path and
_validate_url_not_internal are duplicated between server.py and addon.py;
extract them into a single stdlib-only helper module (e.g., validators) and
replace the copies with imports in both places so logic stays in one location;
preserve the exact function signatures and return types (including use of
ALLOWED_IMAGE_EXTENSIONS and MAX_LOCAL_IMAGE_BYTES), move any required imports
(os, socket, ipaddress, urlparse, etc.) into the new module, and update
server.py and addon.py to import and call validators._validate_image_path and
validators._validate_url_not_internal so both layers use the same implementation
while still allowing each layer to apply defense-in-depth if desired.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea568816-9c39-4ed2-8ce4-a2b2db3c2ccd
📒 Files selected for processing (2)
addon.pysrc/blender_mcp/server.py
| parsed_img = urlparse(image) | ||
| pinned_img_url = image.replace( | ||
| f"{parsed_img.scheme}://{parsed_img.hostname}", | ||
| f"{parsed_img.scheme}://{validated_ips[0]}", | ||
| 1, | ||
| ) | ||
| try: | ||
| resImg = requests.get(image) | ||
| resImg = requests.get( | ||
| pinned_img_url, | ||
| headers={"Host": parsed_img.hostname}, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from urllib.parse import urlparse
samples = [
"https://user:pass@example.com/model.zip",
"https://[2001:db8::1]/model.zip",
]
for url in samples:
parsed = urlparse(url)
pinned = url.replace(
f"{parsed.scheme}://{parsed.hostname}",
f"{parsed.scheme}://203.0.113.10",
1,
)
print(f"{url} -> {pinned}")
PYRepository: ahujasid/blender-mcp
Length of output: 213
🏁 Script executed:
cd /tmp/repo && sed -n '2224,2234p' addon.pyRepository: ahujasid/blender-mcp
Length of output: 123
🏁 Script executed:
cd /tmp/repo && sed -n '2348,2365p' addon.pyRepository: ahujasid/blender-mcp
Length of output: 123
🏁 Script executed:
rg "urlparse.*replace" --context 5Repository: ahujasid/blender-mcp
Length of output: 46
🏁 Script executed:
fd "addon\.py" --type fRepository: ahujasid/blender-mcp
Length of output: 72
🏁 Script executed:
rg "urlparse.*replace\|str.replace" -A 5 -B 2Repository: ahujasid/blender-mcp
Length of output: 46
🏁 Script executed:
rg "parsed.*hostname" -A 3 -B 2 | head -100Repository: ahujasid/blender-mcp
Length of output: 1738
🏁 Script executed:
rg "pinned.*url.*replace" -n addon.pyRepository: ahujasid/blender-mcp
Length of output: 168
🏁 Script executed:
sed -n '2200,2280p' addon.py | cat -nRepository: ahujasid/blender-mcp
Length of output: 4006
🏁 Script executed:
sed -n '2320,2380p' addon.py | cat -nRepository: ahujasid/blender-mcp
Length of output: 2987
🏁 Script executed:
rg "validate_url_not_internal|urlparse.*replace" -B 2 -A 5 addon.py | tail -40Repository: ahujasid/blender-mcp
Length of output: 1619
🏁 Script executed:
wc -l addon.pyRepository: ahujasid/blender-mcp
Length of output: 77
Fix DNS rebinding vulnerability in URL pinning logic.
The str.replace() approach fails to repin URLs containing userinfo (e.g., https://user:pass@example.com/...) or IPv6 literals (e.g., https://[2001:db8::1]/...). In both cases, the pattern scheme://hostname doesn't match the actual URL string:
- Userinfo URLs: The pattern
https://example.comis absent inhttps://user:pass@example.com - IPv6 URLs:
urlparse.hostnamereturns the address without brackets (e.g.,2001:db8::1), but the URL string contains brackets (e.g.,https://[2001:db8::1]), so the pattern fails to match
This leaves requests vulnerable to DNS rebinding even after validation and the Host header is set. Rebuild the netloc from parsed components or use a transport-level helper instead of mutating the raw string.
Affects lines 2225-2230 (image URL) and 2349-2354 (ZIP file URL).
🧰 Tools
🪛 Ruff (0.15.5)
[error] 2231-2231: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@addon.py` around lines 2224 - 2234, The URL repinning using str.replace on
f"{parsed_img.scheme}://{parsed_img.hostname}" fails for userinfo and IPv6
cases; instead reconstruct the netloc from parsed components so userinfo and
port are preserved and IPv6 addresses are bracketed, then rebuild the URL and
use that for requests.get (same fix for the ZIP URL handling). Concretely, use
parsed = urlparse(image) (and parsed_zip), build userinfo =
f"{parsed.username}:{parsed.password}" (omit empty parts), determine host =
validated_ips[0] and wrap in brackets if host contains ':' (IPv6), append
f":{parsed.port}" when present, set new_netloc = f"{userinfo + '@' if userinfo
else ''}{host}{port}", then create pinned_img_url =
parsed._replace(netloc=new_netloc).geturl() and call
requests.get(pinned_img_url, headers={"Host": parsed.hostname}) (and mirror for
the ZIP code paths).
| resImg = requests.get( | ||
| pinned_img_url, | ||
| headers={"Host": parsed_img.hostname}, | ||
| ) | ||
| resImg.raise_for_status() | ||
| image_base64 = base64.b64encode(resImg.content).decode("ascii") | ||
| data["image"] = image_base64 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Python requests.get follow redirects by default, and how can redirects be disabled for a single request?
💡 Result:
Yes. requests.get() follows HTTP redirects by default (allow_redirects=True for GET). [1]
To disable redirects for a single request, pass allow_redirects=False:
import requests
r = requests.get("http://github.com/", allow_redirects=False)
print(r.status_code) # likely 301/302
print(r.headers.get("Location"))This turns off redirection handling just for that call. [1]
Sources:
[1] (app.readthedocs.org)
Citations:
🏁 Script executed:
#!/bin/bash
# Check file existence and size
wc -l addon.pyRepository: ahujasid/blender-mcp
Length of output: 77
🏁 Script executed:
#!/bin/bash
# Read the first location (2231-2237)
echo "=== Lines 2231-2237 ==="
sed -n '2231,2237p' addon.py
echo ""
echo "=== Lines 2363-2372 ==="
# Read the second location (2363-2372)
sed -n '2363,2372p' addon.py
echo ""
echo "=== Context around line 2231 (2220-2245) ==="
# Get broader context around first location
sed -n '2220,2245p' addon.py
echo ""
echo "=== Context around line 2363 (2355-2380) ==="
# Get broader context around second location
sed -n '2355,2380p' addon.pyRepository: ahujasid/blender-mcp
Length of output: 3468
🏁 Script executed:
#!/bin/bash
# Search for any timeout configuration or size limits in these functions
echo "=== Searching for timeout parameters around these sections ==="
sed -n '2210,2250p' addon.py | grep -i "timeout"
echo ""
echo "=== Searching for size/limit checks ==="
sed -n '2210,2250p' addon.py | grep -i "limit\|size\|max"
echo ""
echo "=== Searching for timeout in ZIP section ==="
sed -n '2350,2380p' addon.py | grep -i "timeout"
echo ""
echo "=== Searching for size limits in ZIP section ==="
sed -n '2350,2380p' addon.py | grep -i "limit\|max.*size"Repository: ahujasid/blender-mcp
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Check for any global requests configuration or Session setup
echo "=== Searching for requests.Session or global timeout config ==="
rg "requests\.Session|Session\(\)|timeout\s*=" addon.py | head -20
echo ""
echo "=== Check validate_url_not_internal to understand what it validates ==="
sed -n '1,2300p' addon.py | grep -A 15 "def validate_url_not_internal"Repository: ahujasid/blender-mcp
Length of output: 1325
Both requests.get() calls allow redirect-based SSRF and lack resource limits.
The requests at lines 2231-2237 and 2363-2372 use default allow_redirects=True, which bypasses the URL validation. Although validate_url_not_internal() checks the initial hostname, a 30x redirect from the pinned IP to 169.254.169.254, loopback, or RFC1918 space is followed without revalidation. Additionally, there are no download size limits or timeouts on these specific calls. The image download (line 2231) buffers the full response into memory via .content, and the ZIP extraction (line 2363) has no bounds on extraction size.
Fix: Pass allow_redirects=False to disable redirect handling, add timeout=30 (or appropriate value), and enforce byte limits before buffering/extracting. If redirects are needed, revalidate every hop.
🧰 Tools
🪛 Ruff (0.15.5)
[error] 2231-2231: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@addon.py` around lines 2231 - 2237, The requests.get calls (e.g., the image
fetch using pinned_img_url/parsed_img and resImg -> data["image"], and the
subsequent ZIP download/ZipFile handling) currently allow redirects and load
unlimited bytes; change these requests.get calls to use allow_redirects=False
and a timeout (e.g., timeout=30), and if you must follow redirects implement
per-hop URL revalidation; replace direct .content buffering with streaming reads
that enforce a maximum byte limit (read up to MAX_IMAGE_BYTES before
base64-encoding into data["image"], and up to MAX_ZIP_BYTES before passing to
zipfile), and for ZIP extraction validate each member (check
ZipInfo.file_size/uncompressed size, sanitize names to prevent path traversal,
and enforce total extracted size limits) before writing/extracting. Ensure you
reference the existing variables/functions (pinned_img_url, parsed_img, resImg,
data["image"], res_zip/zip_bytes/ZipFile) when applying these changes.
|
@ahujasid hi, can u have a look at this? |
|
Thanks for pushing on this. Just flagging that the same ZIP extraction
with zipfile.ZipFile(zip_file_path, "r") as zip_ref:
zip_ref.extractall(temp_dir)There's no path-traversal validation, so a crafted zip returned from The good news is that the correct pattern is already implemented with zipfile.ZipFile(zip_file_path, "r") as zip_ref:
for file_info in zip_ref.infolist():
target_path = os.path.join(temp_dir, os.path.normpath(file_info.filename))
abs_temp_dir = os.path.abspath(temp_dir)
abs_target_path = os.path.abspath(target_path)
if not abs_target_path.startswith(abs_temp_dir + os.sep):
shutil.rmtree(temp_dir, ignore_errors=True)
return {"error": "Security issue: zip-slip attempt"}
if ".." in file_info.filename.split("/") or ".." in file_info.filename.split("\\"):
shutil.rmtree(temp_dir, ignore_errors=True)
return {"error": "Security issue: directory-traversal segment"}
zip_ref.extractall(temp_dir)Minor note on the existing Sketchfab block: Happy to open a small separate PR if that helps keep review scopes |
3 participants
Summary
Adds input validation to prevent two security vulnerabilities in the Hunyuan3D tools, reported in #202 and #203.
1. Arbitrary file read via
generate_hunyuan3d_model(fixes #202)Local file paths passed as
input_image_urlwere opened withopen()without any validation, allowing any file accessible by the Blender process to be read, base64-encoded, and sent to the configured API endpoint. The fix validates that local paths have a recognized image file extension and resolves symlinks viaos.path.realpath()before reading.2. SSRF via
import_generated_asset_hunyuan(fixes #203)The
zip_file_urlparameter was only checked for anhttp:///https://prefix before being passed torequests.get(), allowing requests to internal services, cloud metadata endpoints (169.254.169.254), and other private network resources. The fix resolves the URL's hostname to IP addresses and blocks requests targeting private, loopback, link-local, or reserved address ranges.Validation is applied at both the MCP server layer (
server.py) and the Blender addon layer (addon.py) for defense in depth.Changes
validate_image_path()— checks file extension against an allowlist of image formats and resolves symlinksvalidate_url_not_internal()— resolves hostname viasocket.getaddrinfo()and rejects private/loopback/link-local/reserved IPscreate_hunyuan_job_main_siteandcreate_hunyuan_job_local_siteimport_generated_asset_hunyuan_aiserver.pyTest plan
generate_hunyuan3d_modelwith a valid local image path (e.g.,/path/to/image.png) still worksgenerate_hunyuan3d_modelwith a non-image path (e.g.,/etc/passwd) returns an errorgenerate_hunyuan3d_modelwith a symlink to a non-image file returns an errorimport_generated_asset_hunyuanwith a valid external URL still worksimport_generated_asset_hunyuanwithhttp://127.0.0.1orhttp://169.254.169.254returns an errorSummary by CodeRabbit