fix: prevent arbitrary file write via unsanitized SVG/MVG bitmap previews [10.16] (OC10-164)

[oc-tmueller]

Backport of #41827 to 10.16.

#41834 (the per-provider Imagick coder pin) was merged into #41827's branch, so master's squash commit carries both and so does this backport. Two master-only preview commits come with it:

master PR	why it is here
#41827 + #41834	the backport itself: media-type gate, Detection::detectString() contract fix, ImagickFactory hardening, getImagickFormat() on all 8 Bitmap subclasses, CoderPinningTest and its fixtures
#41838	required — without it the suite is red, because BitmapStreamTest fed a PNG through the Photoshop provider and the PSD pin correctly refuses that mismatch
#41855	folded in on request: !is_resource($stream) in both providers, plus fclose() in a finally in the SVG one, and SVGStreamTest

This PR previously held the pre-#41834 shape (mime gate only) and was deliberately held until the pinning approach was settled. It has been rebuilt from master's merged commits now that #41827 has landed.

What PHP 7.4 required

10.16 builds on PHP 7.4 only, and master justifies several guards by PHP 8 raising an \Error that escapes catch (\Exception). On 7.4 the same calls only warn, so those claims were re-derived on the target runtime instead of carried across. Measured, not inferred:

call	PHP 7.4	PHP 8
finfo_buffer(false, …)	warns, returns false	TypeError
stream_get_contents(false) / (null)	warns, returns false	TypeError
fclose(null)	warns, returns false	TypeError
fopen(false, 'wb')	warns, returns false	ValueError
null into a userland string parameter	TypeError	TypeError

Two consequences:

• The detectString() fix matters more here, not less. On 7.4 the unguarded finfo_buffer() returned false to Bitmap::isDangerousToDecode(), where it collapses to '' and matches no deny-list entry — so the missing guard admitted the content the list exists to reject. On PHP 8 it is a 500 instead. The comments and changelog say that.
• SVGStreamTest's unopenable-file case asserts on warnings, not on the return value. Master's assertFalse() passes with the guard reverted on this runtime, so it could never go red. BitmapStreamTest's equivalent already had the 7.4 form and gains fix: return no preview when a preview file cannot be opened or read #41855's false/null data provider.

Also corrected: master's note that an unstubbed getMimeType() mock yields a TypeError. getThumbnail() casts the value, so it yields '' — worse for a test, since seven of the eight providers answer '' with the same constant they answer anything with.

Verification

All in owncloudci/php:7.4, over tests/lib/Preview/ plus tests/lib/Files/Type/DetectionTest.php:

tree	result
pristine 10.16	41 tests / 123 assertions / 18 skipped / 0 failures
this branch	72 tests / 219 assertions / 12 skipped / 0 failures

Skips are environment only — Movie (4), Office (4), SVG (4). The four PDF cases run here for the first time: their guard used to probe the SVG coder, which this image does not register, so they had been skipping under a misleading "No PDF provider present".

Red before green, with production code reverted and the tests kept — 9 failures, each for the right reason:

• CoderPinningTest::testRejectsPostScriptContentFromAForeignProvider ×4 — PostScript rendered to a PNG through the SGI, Photoshop, TIFF and Heic providers. This is the residual path the pin closes.
• CoderPinningTest::testFontNeverInvokesADangerousCoderForForeignContent — the Font provider rendered the PostScript page.
• SVGStreamTest ×3 — two unopenable-handle rows warning from stream_get_contents() and fclose(), and the handle still open after a read that throws.
• BitmapStreamTest — the null handle row, warning twice.

SanitizeTest cannot discriminate on the CI image, which registers no SVG or MVG coder, so the pre-fix sanitize-and-fallback path also ends in "no preview" there. Proved separately on a build that has them (libmagickcore-6.q16-6-extra installed, policy.xml opened): 6 of 10 cases fail pre-fix — every SVG-shaped payload, through both the PDF and Font providers. On that same build this branch is green at 72 tests / 245 assertions / 8 skipped, with the SVG cases running too.

The coder-pin measurements were re-taken on owncloudci/php:7.4, which ships the same ImageMagick 6.9.11-60 and Ghostscript 9.55.0 as the 8.3 image; every figure reproduced.

Also clean: php -l under 7.4 on all changed files; php-cs-fixer 0 of 2412; phpcs over the CI scope; OCPSinceChecker. phpstan reports 311 pre-existing errors in this local environment where CI reports none — a known artefact of a locally installed lib/composer — and none of them names a file this branch touches.

Known and deliberate

• SVG::sanitizeSVGContent()'s ?string/null sentinel is dead here as on master: the pinned rhukster/dom-sanitizer 1.0.17 declares sanitize(): string, and the real sentinel for malformed input is ''. Kept unchanged so the two branches stay identical.
• Pinning costs previews for extension-mislabelled files — a JPEG saved as photo.tif now gets a media type icon. The changelog records this; it is the trade-off the pin buys.
• Font's PFB branch has no coverage, because no .pfb/.pfa fixture exists in the repository.

🤖 Generated with Claude Code