Skip to content

fix(security): add URL safety check to image_ref fetch in openai image_gen (SSRF) - #127

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56035
Open

fix(security): add URL safety check to image_ref fetch in openai image_gen (SSRF)#127
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56035

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Problem (P1 Security — SSRF)

The _load_image_bytes() function in plugins/image_gen/openai/__init__.py fetches arbitrary URLs passed via the ref parameter from model tool calls without any URL safety validation:

resp = requests.get(ref, timeout=60)  # ref is model-supplied, no validation

A model-supplied URL can point to:

  • Cloud metadata endpoints (http://169.254.169.254/latest/meta-data/...) — leaks AWS/GCP/Azure credentials
  • Internal network addresses (http://10.0.0.1:8080/admin) — scans/exfiltrates internal services
  • Localhost services (http://127.0.0.1:6379/) — accesses Redis, databases, etc.

This is a server-side request forgery (SSRF) vulnerability through the gateway process.

Fix

Add is_safe_url() check from tools.url_safety before fetching, matching the pattern already used in:

  • tools/vision_tools.py:212 — image URL validation
  • tools/web_tools.py:100 — web search URL validation
  • tools/skills_hub.py:40 — skill URL validation
  • plugins/platforms/slack/adapter.py:1646 — Slack attachment validation
  • plugins/platforms/mattermost/adapter.py:500 — Mattermost attachment validation

Scope

Only the openai image_gen plugin fetches user-supplied URLs via ref. Other image_gen plugins (krea, xai, openrouter) only call their respective API endpoints with server-controlled URLs — no SSRF risk.

Testing

  • python3 -m py_compile plugins/image_gen/openai/__init__.py — OK
  • The is_safe_url function blocks private IPs (10.x, 172.16-31.x, 192.168.x), link-local (169.254.x), localhost, and metadata endpoints

Ref: NousResearch#54553 (previous SSRF fix attempt, not merged)


Mirror-of: NousResearch#56035
NousResearch#56035

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete

Files Reviewed: 1
Findings: 1

By Severity:

  • 🟡 Medium: 1

The PR adds a URL safety check to prevent SSRF attacks when fetching source images, but the fix is incomplete — HTTP redirect following (enabled by default in the requests library) allows attackers to bypass the pre-flight check by redirecting to internal addresses.

Files Reviewed (1 files)
plugins/image_gen/openai/__init__.py

@tenki-reviewer tenki-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: 🟠 High (65/100) — 1 medium finding · 6 LOC across 1 file


Overview

The PR modifies plugins/image_gen/openai/__init__.py to add an is_safe_url() pre-flight check before fetching source images via HTTP in the _load_image_bytes function.

Key Finding

SSRF redirect bypass (finding-001, medium): The new is_safe_url() check validates only the initial URL at line 137, but requests.get(ref, timeout=60) at line 143 follows HTTP redirects (up to 30 hops by default) without re-validating each redirect target. An attacker-controlled URL sending a 302 redirect to http://169.254.169.254/ or similar internal address will achieve SSRF despite the check.

Context

The codebase already documents this exact bypass vector in tools/url_safety.py:20-23 and has an established mitigation using httpx event hooks at gateway/platforms/base.py:541-554. The recommended fix is to either disable redirect following (allow_redirects=False) and manually re-validate each hop, or switch to httpx with the existing redirect-guard pattern.

Additional Notes

Two lower-confidence findings (confidence <80) were flagged but not included: (1) the blocked URL being leaked in the error message, and (2) URL-safety rejections being misclassified as io_error instead of a distinct security error type. These are defense-in-depth improvements worth considering.

)
import requests

resp = requests.get(ref, timeout=60)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 SSRF redirect bypass — is_safe_url pre-flight check defeated by HTTP redirect following (security)

The PR adds an is_safe_url() pre-flight SSRF check in _load_image_bytes() at plugins/image_gen/openai/__init__.py:137, but the subsequent requests.get(ref, timeout=60) at line 143 silently follows HTTP redirects (up to 30 hops by default) without re-validating each redirect target. An attacker-controlled URL that passes the initial pre-flight check can 302-redirect to http://169.254.169.254/ or any other internal address, achieving SSRF despite the new check. The codebase's own tools/url_safety.py:20-23 documents this as a known limitation and states that redirect-based bypass is mitigated via httpx event hooks — a pattern implemented at gateway/platforms/base.py:541-554 (_ssrf_redirect_guard) and used at lines 754 and 874. The new code uses the synchronous requests library which has no equivalent redirect event hook, leaving the redirect-bypass gap open.

💡 Suggestion: Either disable redirect following (allow_redirects=False) and manually handle redirects while re-validating each Location header with is_safe_url, or switch from requests to httpx (synchronous client available) with follow_redirects=True and an event hook that re-validates each redirect target — matching the established pattern at gateway/platforms/base.py:744-754. The simplest correct fix: use httpx.Client with the same redirect-guard pattern already in the codebase.

📋 Prompt for AI Agents

In plugins/image_gen/openai/__init__.py, in the _load_image_bytes function around line 143, replace the requests.get(ref, timeout=60) call with an httpx.Client call that uses follow_redirects=True and a response event hook that calls is_safe_url on every redirect target. Import httpx at the top of the function (or module). The redirect guard should raise ValueError when a redirect target fails is_safe_url. The established pattern is at gateway/platforms/base.py:541-554 (_ssrf_redirect_guard) and its usage at lines 744-754. Alternatively, set allow_redirects=False on the requests call and manually follow/re-validate each redirect target in a loop — this avoids the httpx dependency but requires more code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant