Skip to content

Stop fetching media URLs implicitly; add a guarded fetch for untrusted URLs - #1045

Open
wishborn wants to merge 1 commit into
prism-php:mainfrom
Particle-Academy:security/no-implicit-media-fetch
Open

wishborn wants to merge 1 commit into
prism-php:mainfrom
Particle-Academy:security/no-implicit-media-fetch

Conversation

@wishborn

Copy link
Copy Markdown
Contributor

First of the smaller PRs out of #1028, and the one I'd take first: it closes a server-side request forgery.

The problem

Reading a URL-backed media's bytes fetched the URL with a bare Http::get(): no host allow-list, no private-address check, redirects followed. Prism reads media bytes on ordinary provider calls (rawContent(), base64(), mimeType() sniffing, and resource() for audio and image edits). So an app that builds Image::fromUrl($request->input('url')) and sends it to a provider that inlines media has the server fetch whatever URL was named, cloud metadata included:

Image::fromUrl('http://169.254.169.254/latest/meta-data/iam/')->base64(); // made the request

What changes

Reading never fetches. For a URL-only payload, rawContent() and base64() return null, hasRawContent() and hasBase64() return false, and resource() throws, naming fetchUrlContent(). A payload holding both a url and base64 no longer fetches the url and ignores its own bytes.

Providers that need bytes refuse a URL instead of sending null: Ollama's image mapper, Gemini's image, document and audio/video mappers (YouTube and File API URLs still go by reference, untouched), and Gemini's image-generation request map, which never ran media validation. The message says what changed and what to call.

Two explicit fetches:

  • fetchUrlContent() behaves as before (unguarded, for a URL you control). It now returns the media, so Image::fromUrl($url)->fetchUrlContent() reads as a step, and its error messages no longer print userinfo or query strings.
  • fetchPublicUrlContent() is new, for a URL you didn't choose. It refuses non-http schemes; private, loopback, reserved, CGNAT and link-local addresses, including NAT64/6to4 spellings of them; a host name that resolves to one; any redirect into one (each hop is checked before it's requested); too many redirects; and a body over $maxBytes (default 32 MiB, refused while it streams). A refusal throws PrismUrlRefused with a code() to branch on (private_address_refused, redirect_refused, media_too_large, ...). HostResolver is bound in the container, so an app with its own DNS (a service mesh, split-horizon) can supply it.

Breaking

Yes, for anyone relying on the implicit fetch: a URL-only payload sent to a provider that needs bytes now throws, with the fix in the message. That's deliberate; the silent fetch is the vulnerability. The docs examples that relied on it now fetch explicitly, and the audio guide says which fetch to use for an untrusted URL.

Tests

  • Every reader of a URL image pointed at the metadata address, asserting zero requests, with a positive control that the explicit fetch sends exactly one.
  • The guarded fetch: each refused range as a URL literal and as a DNS answer, public controls (including NAT64/6to4 of a public address) that must still fetch, redirect hops into private space, the redirect bound, and the body cap (one byte over, an oversized declared length, exactly at the cap).
  • Provider refusals for Gemini and Ollama, with nothing sent.

Four existing tests asserted the fetch and are inverted, not deleted. The Gemini URL tests keep their intent by fetching explicitly. With this PR's Media.php swapped back to main's, 88 of these tests fail.

pint, rector and phpstan are clean, and the suite passes (2278).

This landed in the fork across v0.105 to v0.132; this is the same code, without the fork's other changes to Media (stored form, provider options).

…usted URLs

Reading a URL-backed media's bytes fetched the URL with a bare Http::get():
no host allow-list, no private-address check, redirects followed. Prism
reads media bytes on ordinary provider calls (rawContent(), base64(),
mimeType() sniffing, resource() for audio and image edits), so an
application that built Media::fromUrl() from request input and sent it to
a provider that inlines media had the server fetch whatever URL it named,
a cloud metadata endpoint included.

Reading never fetches now:
- rawContent() and base64() return null for a URL-only payload;
  hasRawContent() and hasBase64() answer false; resource() throws, naming
  fetchUrlContent().
- Providers that need bytes refuse a URL-only payload instead of sending
  null: Ollama's image mapper, Gemini's image, document and audio/video
  mappers (YouTube and File API URLs still go by reference), and Gemini's
  image-generation request map. The refusal names fetchUrlContent().
- A payload holding both a url and base64 no longer fetches the url and
  ignores its own bytes.

Two explicit fetches:
- fetchUrlContent() is unchanged in behaviour (still unguarded, for a URL
  you control) and now returns the media, so it reads as a step. Its
  error messages no longer print userinfo or query strings.
- fetchPublicUrlContent() is for a URL you did not choose. It refuses
  non-http schemes; private, loopback, reserved, CGNAT and link-local
  addresses, including NAT64/6to4 spellings; a host name that resolves to
  one; any redirect into one (each hop checked before it is requested); a
  redirect chain past the bound; and a body over $maxBytes (default
  32 MiB, refused while it streams). Refusals throw PrismUrlRefused with
  a code() a caller can branch on. HostResolver is bound in the container
  so an application with its own resolution can supply it.

Docs: every example that relied on the implicit fetch now fetches
explicitly, and the audio guide says which fetch to use for an untrusted
URL.

This branch has not been deployed

No deployments
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