Skip to content

Add Psalm taint analysis annotations - #295

Open
danog wants to merge 2 commits into
libvips:masterfrom
danog:psalm-taint-annotations
Open

danog wants to merge 2 commits into
libvips:masterfrom
danog:psalm-taint-annotations

Conversation

@danog

@danog danog commented Oct 23, 2026

Copy link
Copy Markdown

Adds Psalm taint analysis annotations, so that Psalm follows untrusted data through php-vips in the projects that use it:

  • The files Image::newFromFile() reads and Image::writeToFile() writes are file sinks.

These are docblock-only changes, with no effect at runtime or on other tools. Psalm currently ships them as a stub for this library (vimeo/psalm#12466); having them here keeps them in step with the code.

Psalm's taint analysis can only follow untrusted data through this library if its docblocks say which
parameters are sinks and which return values are sources. These annotations are docblock-only and have no
effect at runtime.
@jcupitt

jcupitt commented Oct 23, 2026

Copy link
Copy Markdown
Member

Hi @danog,

This sounds useful.

There are other sources and sinks, for example .newFromBuffer() (used on eg. a JPEG formatted image that has been loaded into memory) or .avg() (an image sink that computes the pixel average).

Does Psalm consider objects other than files?

@danog

danog commented Oct 23, 2026

Copy link
Copy Markdown
Author

Thanks! Psalm's taint analysis follows untrusted values (request parameters, uploaded data, …) into places where they could be injected: file paths, shell commands, SQL, HTML, URLs requested, and so on. A sink there is a parameter whose value is interpreted as one of those.

  • newFromBuffer() takes the image bytes themselves, not a path or a command, so there's nothing to inject: decoding untrusted image data is libvips' job, not a taint issue Psalm models.
  • avg() returns a number, which Psalm treats as unable to carry those taints, so it's neither a source nor a sink.

There were a few more methods taking a path that I'd missed, though, so I've added file sinks to Image::findLoad(), Source::newFromFile() and Target::newToFile().

The generated loaders and savers (jpegload(), csvsave(), …) also take filenames, but they are @method magic methods, which Psalm can't attach taint annotations to.

@danog

danog commented Oct 23, 2026

Copy link
Copy Markdown
Author

Unless you'd be okay with adding a custom taint type for image decoding, but I feel that's a bit overkill

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.

2 participants