Decide on SVG: it is accepted and rendered by libvips, and format=orig silently returns JPEG #68

Open
opened 2026-08-09 03:44:54 +02:00 by clawbot · 0 comments
Collaborator

Verified against main at 61f42e6.

image/svg+xml is allow-listed by the fetcher (internal/httpfetcher/httpfetcher.go:100) and validated as acceptable by internal/magic/magic.go:114-116,180, so SVG bytes reach vips.NewImageFromBuffer (internal/imageprocessor/imageprocessor.go:154).

Two problems:

1. Security surface. SVG rendering in libvips goes through librsvg, and SVG is an XML format: external entity references, remote resource loading, and billion-laughs style expansion are the standard attack surface, on input fetched from an arbitrary upstream. Notably, SupportedInputFormats() (imageprocessor.go:221-229) deliberately does not list SVG — so the fetcher accepts a format the processor does not claim to support.

2. Silent format substitution, which is a repo-rule violation. detectFormat (imageprocessor.go:261-277) has no SVG case, so it returns "unknown", and formatFromString("unknown") returns FormatJPEG (:396). A request for orig on an SVG source therefore silently returns a JPEG. Repo rules are explicit: never silently fall back to a different setting when a parameter specifies a value — format=orig producing JPEG is exactly that defect.

Uncertain and worth checking first: whether the libvips build produced by script/bootstrap's environment actually has librsvg support compiled in. That determines whether problem 1 is live or latent, but problem 2 is real either way.

Definition of done

Pick one direction and implement it fully:

  • (a) Drop SVG — remove image/svg+xml from the fetcher's allowed content types and from magic, and return a clear 415 for SVG sources. Simplest and safest; SVG does not need rasterizing to be served by a CDN.
  • (b) Support SVG properly — add it to SupportedInputFormats(), add an SVG case to detectFormat, confirm librsvg is present and configured with external entity/resource loading disabled, and bound rasterization dimensions.

Either way:

  1. format=orig on an SVG source must never silently return a different format — it either works and returns SVG, or returns an explicit error. No JPEG fallback.
  2. Audit formatFromString's default-to-JPEG behavior (imageprocessor.go:396) for any other input that can reach it as "unknown"; a silent default there is the same class of bug.
  3. Failing tests first: an SVG source with format=orig does not return image/jpeg; and whichever of (a)/(b) is chosen is asserted directly.
  4. make check green.

Recommendation: (a). Rasterizing untrusted XML is a large attack surface for a feature nothing in the README promises.

Verified against `main` at `61f42e6`. `image/svg+xml` is allow-listed by the fetcher (`internal/httpfetcher/httpfetcher.go:100`) and validated as acceptable by `internal/magic/magic.go:114-116,180`, so SVG bytes reach `vips.NewImageFromBuffer` (`internal/imageprocessor/imageprocessor.go:154`). Two problems: **1. Security surface.** SVG rendering in libvips goes through librsvg, and SVG is an XML format: external entity references, remote resource loading, and billion-laughs style expansion are the standard attack surface, on input fetched from an arbitrary upstream. Notably, `SupportedInputFormats()` (`imageprocessor.go:221-229`) deliberately does **not** list SVG — so the fetcher accepts a format the processor does not claim to support. **2. Silent format substitution, which is a repo-rule violation.** `detectFormat` (`imageprocessor.go:261-277`) has no SVG case, so it returns `"unknown"`, and `formatFromString("unknown")` returns `FormatJPEG` (`:396`). A request for `orig` on an SVG source therefore silently returns a JPEG. Repo rules are explicit: never silently fall back to a different setting when a parameter specifies a value — `format=orig` producing JPEG is exactly that defect. **Uncertain and worth checking first:** whether the libvips build produced by `script/bootstrap`'s environment actually has librsvg support compiled in. That determines whether problem 1 is live or latent, but problem 2 is real either way. ## Definition of done Pick one direction and implement it fully: - **(a) Drop SVG** — remove `image/svg+xml` from the fetcher's allowed content types and from `magic`, and return a clear 415 for SVG sources. Simplest and safest; SVG does not need rasterizing to be served by a CDN. - **(b) Support SVG properly** — add it to `SupportedInputFormats()`, add an SVG case to `detectFormat`, confirm librsvg is present and configured with external entity/resource loading disabled, and bound rasterization dimensions. Either way: 1. `format=orig` on an SVG source must never silently return a different format — it either works and returns SVG, or returns an explicit error. No JPEG fallback. 2. Audit `formatFromString`'s default-to-JPEG behavior (`imageprocessor.go:396`) for any other input that can reach it as `"unknown"`; a silent default there is the same class of bug. 3. Failing tests first: an SVG source with `format=orig` does not return `image/jpeg`; and whichever of (a)/(b) is chosen is asserted directly. 4. `make check` green. **Recommendation: (a).** Rasterizing untrusted XML is a large attack surface for a feature nothing in the README promises.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:44:54 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#68