Ship hardened ImageMagick policy to mitigate image-bomb DoS#74
Conversation
The storage/avatars preview pipeline decodes untrusted images through Imagick with no bound on decoded dimensions. A crafted image (small on disk, enormous dimensions) decodes unbounded and spills ImageMagick's pixel cache to disk, filling the volume and killing MongoDB or the container. Install a hardened policy.xml capping width/height (hard reject before raster allocation), disk spill, and memory/map/area, and disabling the delegate coders never used for previews. Placed in ImageMagick's configure dir (discovered at build time); the build fails if it does not load. Added a tests.yaml assertion.
Greptile SummaryThis PR ships a hardened ImageMagick
Confidence Score: 5/5Safe to merge; the policy correctly targets the real attack vectors and the build-time verification ensures it cannot silently not load. The resource limits, coder denials, and installation mechanism are all sound. The two findings are minor quality notes — a no-op policy entry that could mislead future maintainers, and a CI assertion that could be slightly more precise about the disk value — neither affects runtime behavior or security coverage. No files require special attention beyond the minor notes on policy.xml and tests.yaml. Important Files Changed
Reviews (3): Last reviewed commit: "Raise dimension/disk limits so high-res ..." | Re-trigger Greptile |
- Dockerfile: strip trailing slash and take first entry of a colon-separated CONFIGURE_PATH so the policy is always written to a valid path. - policy.xml: deny the SVG/SVGZ/MSVG module. A coder deny alone leaves the rsvg delegate route open; the module deny closes SVG's SSRF/XXE vector entirely. Verified jpg/png/gif/heic/avif still round-trip.
16KP (16000px) rejected legitimate high-res phone photos - a 200MP sensor is ~16320px on its long side. Raise width/height to 50KP as a fast-fail backstop only, and make disk (4GiB) the primary DoS control. ImageMagick in the base image is Q16-HDRI (16 bytes/pixel), so a 150MP photo needs ~2.4GB of cache; 4GiB disk gives headroom while memory stays at 256MiB to bound per-worker RAM. Verified a 9180x16320 PNG and a realistic 150MP JPEG both preview-resize successfully, while a 60000px image is rejected on dimensions and a ~1.6GP image is bounded by the disk cap.
|
Re: the 16KP dimension cap being too low for high-res phone photos (e.g. 9180×16320) — fixed in 1e1370c. Reworked the sizing so dimension limits are only a fast-fail backstop and
Verified against your exact resolution:
So legitimate photos are no longer rejected, while a bomb is still capped to ~4GiB transient disk per operation (vs the unbounded ~32GB in the original report). |
What
Ship a hardened ImageMagick
policy.xmlin the final base image to mitigate an image-decompression-bomb DoS reported against the Appwrite storage file-preview endpoint (GET /v1/storage/buckets/:bucketId/files/:fileId/preview), which also affects the avatars pipeline.Why
The preview/avatars handlers decode untrusted images through Imagick. The utopia-php/image constructor calls
readImageBlob(), which fully decodes the raster before any dimension is inspected — the endpoint'swidth/heightparams (capped at 4000) only apply to the output resize afterward. Appwrite's only guard is a compressed-file-size check (_APP_STORAGE_PREVIEW_LIMIT, 20MB) plus amemoryresource limit (64MB).A crafted image — tiny on disk, enormous dimensions (a "zip-bomb"-style image) — sails past the size check. When the decoded raster exceeds the memory limit, ImageMagick doesn't fail; it spills its pixel cache to disk unbounded (default
disklimit is effectively unlimited, nopolicy.xmlships today). The reporter observed ~32GB of disk growth from repeated concurrent requests to one file, killing MongoDB on a 64GB volume and crashing the container on a 4GB one. Capping onlymemoryactually makes it worse by forcing the spill to disk sooner.The base image is the correct home: ImageMagick lives here, and every downstream image (appwrite, cloud, edge, tooling) inherits the fix — rather than patching each call site in each repo.
What the policy does
Only
width/height/diskraise an exception when exceeded (memory/map/area merely move the cache to disk), so they are the real guards:width/height= 16KP — hard reject oversized dimensions before the raster is allocated (well above the 4000px output cap)disk= 1GiB — caps the pixel-cache spill instead of letting it fill the volumememory256MiB,map512MiB,area128MP,thread2,time60s,max-memory-request256MiB@*).Installed into ImageMagick's configure dir, discovered at build time (
identify -list configure) so it survives package path changes, and the build fails loudly if the policy doesn't load (identify -list policy | grep 16KP). Note:MAGICK_CONFIGURE_PATHis not honored by the Alpine ImageMagick build — verified during development — hence the direct install into the discovered dir.Verification
Tested against
alpine + imagemagick(same package as the final stage):width or height exceeds limit (20000x20000) > (16000x16000)identify -list policyshows the width/height/disk limits activeAdded a
tests.yamlassertion so a regression (policy not loading) fails CI.Rollout
After this merges and a new
appwrite/basetag is cut, bumpFROM appwrite/base:1.4.3inappwrite/appwrite. Adiskresource-limit backstop inapp/config/storage/resource_limits.phpthere is worth adding too, so protection doesn't hinge solely on the base bump.🤖 Generated with Claude Code