Skip to content

fix(thumbnails): serve saved thumbnails from disk, restrict image locations - #3828

Closed
Vondry wants to merge 16 commits into
bolt:6.1from
Vondry:fix/thumbnail-save-path
Closed

fix(thumbnails): serve saved thumbnails from disk, restrict image locations#3828
Vondry wants to merge 16 commits into
bolt:6.1from
Vondry:fix/thumbnail-save-path

Conversation

@Vondry

@Vondry Vondry commented Oct 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Since 6.0.1 (5161020) save_files writes thumbnails to thumbs/<w>_<h>_<q>_<fit>_<location>/, a path no thumbnail URL points to, so every /thumbs/… request is handled by PHP.

Changes

  • Save thumbnails under the requested parameter string again, so Apache / Nginx serve the next request from disk. Thumbnail URLs don't change. Nothing is saved for:

    • a dot segment (., .., .htaccess)
    • a path that changes when joined
    • a double-encoded filename (Glide decodes it again, so foo/%252e%252e/kitten.jpg would save a copy for every prefix)
    • a ?path= query
    • parameters that aren't understood (e.g. …×crop×files×<anything>, which would fill the disk)

    These are still served by PHP.

  • Only read images from locations inside the web root, and never from thumbs/. Before this, /thumbs/100×100×80×crop×site/<image> (or ×..) read any image in or above the project.

  • Keep a separate Glide cache per location. An image cached for one location was served (and saved) for another, even if it didn't exist there.

  • Deleting a file or folder in the file manager also removes its saved thumbnails and Glide cache entries. The web server serves saved thumbnails from disk, so they stayed public otherwise. Deleting a file also resolved its path against the working directory, which only worked because PHP runs from public/.

  • Limit generated images to thumbnails/max_image_size pixels (default 3840×2160), so /thumbs/20000×20000/… can't exhaust memory. The 10000 that ThumbnailHelper uses for a missing width or height is passed to Glide as missing, so it follows the aspect ratio. Before, thumbnail(400) with the crop fit was a 400×10000 strip, and that box counted towards the limit.

  • If a request for a saved thumbnail still reaches PHP, serve the file instead of generating and writing it again. If saving fails (e.g. thumbs/ isn't writable), serve the generated image instead of generating it again, and log an error.

  • Generate saved thumbnails without Glide's cache, so they're no longer stored twice.

  • Keep Glide's Cache-Control: max-age=31536000, public and add Last-Modified / 304. Thumbnails are excluded from the firewall: with a remember-me cookie it loaded the user, started a session and made every response private, max-age=0. Responses for a started session stay private. public/.htaccess and the Nginx config in docker/ send the same header for thumbnails served from disk.

Tests:

  • ImageControllerTest (73 cases) covers:
    • saving at the URL path, including every URL form ThumbnailHelper generates
    • the cases that aren't saved, including double-encoded filenames
    • traversal through the filename, ?path= and the location
    • the per-location cache, the size limit and missing dimensions
    • serving saved files, failed saves, the cache headers and remember-me cookies
  • ThumbnailCacheClearerTest and FileDeleteThumbnailsTest cover removing thumbnails of deleted files and folders.

Each fix's tests fail without it. I checked the .htaccess header manually with Apache 2.4, and the Nginx block with Nginx locally.

Upgrade:

  • The thumbs/<w>_<h>_…_files/ folders saved since 6.0.1 are no longer used. bin/console cache:thumbs removes them, along with saved 400×10000 strips.
  • bolt/project needs the same changes:
    • the rule in public/.htaccess
    • the thumbs firewall in config/packages/security.yaml

Refs #3714. That PR saves under normalised parameters (c → crop), so URLs using shorthand or the default fit still miss the saved file.

@Vondry
fix(thumbnails): save thumbnails under the path of their URL
69b0221 
Since 6.0.1 `save_files` writes thumbnails to
`thumbs/<w>_<h>_<q>_<fit>_<location>/`, a path no thumbnail URL points to,
so the web server never serves them and every /thumbs/ request is handled
by PHP.

Save them under the requested parameter string again, so Apache / Nginx
serve the next request straight from disk. The parameter string comes
from the URL, so the thumbnail is only saved if joining it into the path
didn't resolve any `.` or `..` segments, i.e. the file ends up exactly at
the path of its URL. Requests with a `?path=` query are not saved: it
changes the image but not the URL path.
@Vondry
Vondry force-pushed the fix/thumbnail-save-path branch from ffaf0e6 to 69b0221 Compare October 12, 2026 06:39
Vondry added 9 commits October 12, 2026 08:48
@Vondry
test(thumbnails): cover path traversal through the thumbnail endpoint
903704f 
Both the parameters and the filename of a /thumbs/ URL are user input,
but there were no tests that they can't be used to read or write files
outside the `files` and `thumbs` folders.

Request images in assets/static/images/ (outside the web root) through
`..` in the filename, in its encoded forms, with backslashes and through
`?path=`, and assert that the error image is returned and nothing is
written to public/ or public/thumbs/. The SVG cases fail without the
filename canonicalization, since SVGs are read directly instead of
through Glide.
@Vondry
fix(thumbnails): never save thumbnails under hidden folders
18d0f4b 
Thumbnails are saved under the parameter string of their URL, so a
request to /thumbs/.htaccess/kitten.jpg created a `thumbs/.htaccess/`
folder. Apache then refuses every request below `thumbs/` (403, ".htaccess
... not a regular file") until the folder is removed by hand.

Don't save a thumbnail if any segment of its path starts with a dot. It is
still served by PHP. The test now also sees hidden files and folders when
asserting that nothing was saved.
@Vondry
fix(thumbnails): only read images from locations inside the web root
f5cfd15 
The location in the parameters of a /thumbs/ URL is resolved like any
path, so `site`, `%site%`, `.` or `..` read images from anywhere in, or
above, the project, e.g. /thumbs/100×100×80×crop×site/<any image>. With
`save_files` those were copied into public/thumbs/ as well.

Only allow locations inside the web root, like `files` and `themes`,
which are public anyway, and never read images from the `thumbs` folder.

The filename was also checked against the base path of the `?location=`
query (or of the previous request, since the controller is a shared
service) because the parameters were parsed after that check. Parse them
first and drop the `?location=` query, which was otherwise never used.

The tests now also detect files saved into existing folders below
public/thumbs/.
@Vondry
fix(thumbnails): keep a separate Glide cache per location
9fc628c 
Glide caches an image by its path relative to the source folder and its
parameters, and the location isn't one of them. All locations share
var/cache/thumbnails/, so a thumbnail cached for `themes/foo.jpg` was
served for `files/foo.jpg`, and saved to public/thumbs/, even if
`files/foo.jpg` doesn't exist. This also kept serving images that were
cached through a location that is no longer allowed.

Prefix the cache path with the path of the location, and check that the
source image exists before saving a thumbnail, since Glide serves cached
images without checking that.
@Vondry
fix(thumbnails): only save thumbnails whose parameters are all unders…
2802815 
…tood

Each parameter string is saved as a separate file, but parseParameters()
ignores unknown or superfluous segments, so /thumbs/<w>×<h>×80×crop×files×
<anything>/kitten.jpg saved a new copy of the same thumbnail for every
value of <anything> and could fill the disk.

Only save a thumbnail if every segment of its parameter string is a
canonical number (no leading zeros or exponents), a quality between 0 and
100, a fit that Glide understands (in one form only) or a location.
Other URLs are still served, by PHP.

A test checks that the URLs generated by ThumbnailHelper are saved.
@Vondry
fix(thumbnails): limit the size of generated thumbnails
cbf8c9c 
The width and height of a thumbnail come from the URL and Glide had no
limit, so a single request for e.g. /thumbs/20000×20000×80×fill/kitten.jpg
made it allocate an image of 400 megapixels.

Set Glide's `max_image_size`, which scales larger thumbnails down while
keeping their aspect ratio. It can be configured with
`thumbnails/max_image_size` and defaults to 40 megapixels: ThumbnailHelper
uses 10000 for a missing width or height, so this still allows thumbnails
of 4000 pixels wide.
@Vondry
fix(thumbnails): serve a saved thumbnail instead of generating it again
8601a87 
The web server normally serves a saved thumbnail, but a request for it can
still reach PHP, e.g. when Nginx has no `try_files` for /thumbs/, or when a
parallel request saved it in the meantime. The thumbnail was then
generated (or read from Glide's cache) and written to public/thumbs/ again
on every request.

Serve the saved file instead, with a Content-Type based on its extension
(like the web server does), Last-Modified and support for
If-Modified-Since.
@Vondry
perf(thumbnails): don't keep saved thumbnails in Glide's cache as well
0f45d4c 
A thumbnail to save was generated into Glide's cache in var/cache/,
read from there into memory, written to public/thumbs/, and then read and
streamed from Glide's cache again for the response. Every saved
thumbnail was kept on disk twice.

Generate it with Glide's API directly, without the cache, and respond
with the saved file. Thumbnails that are not saved still use the cache.
@Vondry
perf(thumbnails): let browsers and proxies cache thumbnails
4da24b8 
Glide sets `Cache-Control: max-age=31536000, public` on thumbnails, but
Symfony makes a response private and uncacheable as soon as the session
is used, which the remember-me authenticator of the firewall does on
every request. Thumbnails served by PHP were therefore sent with
`max-age=0, must-revalidate, private`, and the ones Apache serves from
public/thumbs/ had no Cache-Control at all.

Keep the caching headers of thumbnail responses, unless the session was
started (e.g. for a logged-in user), since the response could then set
the session cookie. Pass the request to Glide's response factory, so it
sets Last-Modified and responds to If-Modified-Since with a 304.

Send the same Cache-Control from Apache for thumbnails it serves from
disk. Nginx is already configured with `expires max` for /thumbs in the
documentation.
@Vondry
Vondry force-pushed the fix/thumbnail-save-path branch from 5a30e70 to 4da24b8 Compare October 12, 2026 08:40
@Vondry Vondry changed the title fix(thumbnails): save thumbnails under the path of their URL fix(thumbnails): serve saved thumbnails from disk, restrict image locations Oct 12, 2026
Vondry added 6 commits October 12, 2026 10:58
@Vondry
fix(thumbnails): don't save thumbnails of double-encoded filenames
b27b666 
Glide decodes the filename a second time after the router, so
`foo/%252e%252e/kitten.jpg` passes the path checks as `%2e%2e` while Glide
reads `../kitten.jpg`. Saving it stored a separate copy of the same image
for every prefix, which bypassed the limit on saved thumbnails.
@Vondry
fix(thumbnails): remove the thumbnails of deleted files and folders
eef8854 
Saved thumbnails are served by the web server straight from disk, so the
thumbnails of a deleted image stayed public, and were cached by browsers
and proxies for a year. Deleting a file or folder in the file manager now
also removes its saved thumbnails and its images in Glide's cache.

Deleting a file resolved its path relative to the working directory, which
only worked because PHP runs from the web root. It is now resolved against
the path of its location.
@Vondry
fix(thumbnails): lower the default max_image_size to 3840×2160
06a735c 
40 megapixels let a single request make a thumbnail of e.g. 6300×6300,
which takes a lot of memory and CPU for a size no optimized website needs.

That limit was only that high because ThumbnailHelper uses 10000 for a
missing width or height, and Glide limits the requested box, not the
resulting thumbnail. A missing dimension is now passed to Glide as missing,
so it derives it from the aspect ratio of the image. This also fixes that
e.g. `thumbnail(400)` with the crop fit was cropped to a strip of 400×10000
pixels: such saved thumbnails are only regenerated after `cache:thumbs`.
@Vondry
perf(thumbnails): don't run the firewall for thumbnails
3a7b069 
Thumbnails are public and the same for every user, but the main firewall
ran for each one that PHP generated: with a remember-me cookie it loaded
the user from the database, started a session, refreshed the cookies and
made the response private and uncacheable.
@Vondry
perf(thumbnails): let Nginx serve saved thumbnails with caching headers
baf7efd 
Like public/.htaccess does for Apache, so browsers and proxies cache the
thumbnails that Nginx serves from disk just like the ones Bolt serves.
@Vondry
fix(thumbnails): log thumbnails that can't be saved, don't generate t…
e65f8e7 
…hem twice

When saving a thumbnail failed, e.g. because the thumbs folder is not
writable, the error was swallowed and Glide generated the same thumbnail
again for the response, on every request. The generated thumbnail is now
served, and the failure is logged.

Both this and a failure to remove the thumbnails of a deleted file are
logged as errors: the production log only writes warnings when an error
occurs in the same request.
@Vondry Vondry mentioned this pull request Oct 12, 2026
Open
@Vondry Vondry closed this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Vondry