fix(thumbnails): bound declared image dimensions before decoding - #3457
Open
dschmidt wants to merge 1 commit into
Open
fix(thumbnails): bound declared image dimensions before decoding#3457dschmidt wants to merge 1 commit into
dschmidt wants to merge 1 commit into
Conversation
dschmidt
force-pushed
the
fix/thumbnails-dimension-guard
branch
from
September 3, 2026 12:16
da512aa to
1a104b5
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 12 |
🟢 Coverage 83.58% diff coverage · +0.06% coverage variation
Metric Results Coverage variation ✅ +0.06% coverage variation (-1.00%) Diff coverage ✅ 83.58% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (61037cc) 88237 20687 23.44% Head commit (50c38bf) 88294 (+57) 20751 (+64) 23.50% (+0.06%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3457) 67 56 83.58% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
dschmidt
force-pushed
the
fix/thumbnails-dimension-guard
branch
from
September 3, 2026 12:19
1a104b5 to
bff00be
Compare
The imaging build decodes the full pixel buffer from the header-declared dimensions before the existing MaxInputWidth/MaxInputHeight guard runs, so a tiny crafted file whose header declares huge dimensions forces a multi-GB allocation and can OOM the worker. Read the header with DecodeConfig and reject oversized sources before the decode allocates, in both the imaging and vips builds, and thread the limit through the audio cover-art and geogebra decoders that decode a second attacker-controlled image.
dschmidt
force-pushed
the
fix/thumbnails-dimension-guard
branch
from
September 3, 2026 12:21
bff00be to
50c38bf
Compare
dschmidt
marked this pull request as ready for review
September 3, 2026 12:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The non-vips (imaging) build decodes the full pixel buffer from the header declared dimensions before the existing
MaxInputWidth/MaxInputHeightguard runs. That guard sits inSimpleManager.Generate, at which pointpreprocessor.Converthas already decoded the image. The Go stdlib backend allocatesimage.NewGray/image.NewYCbCrfor the declared width*height at the SOS marker, before it fails on the mismatched scan data, so a tiny crafted file forces a large allocation and can exhaust the memory of the thumbnails worker.This reads only the header with
DecodeConfig, which allocates no pixels, and rejects oversized sources before the decode happens. The limit is threaded through the decoders that recurse into a second attacker controlled image: audio cover art (mp3/flac/ogg) and the geogebra decoders. The vips build gets the same check on the header lazyvips.NewImageFromReader, beforeThumbnailWithSizematerializes anything.The service now passes the configured
MaxInputWidth/MaxInputHeightinto the preprocessor options, so a zero limit keeps the previous behaviour and an unconfigured decoder stays unrestricted.Impact is availability only, and the official container images are not affected because they build with
ENABLE_VIPS=true. Affected are the non-vips builds produced bymake release-linux/release-darwin, so development builds and the published standalone binaries. Reachable by anyone who can have a preview generated, which means authenticated upload or a public share.Related Issue
Reported privately to security@opencloud.eu on 2026-08-18 and cleared for public handling, no public issue exists.
Motivation and Context
A 375 byte grayscale JPEG whose SOF0 declares large dimensions while the pixel data stays 8x8 drives the allocation in
ImageDecoder.Convert. Peak liveHeapAlloc: 20000x20000 gives roughly 381 MB, 65535x65535 gives roughly 4 GB. With the guard in place the same files stay at roughly 17 KB.How Has This Been Tested?
go test ./services/thumbnails/..., plus a running container to check the vips pathErrImageTooLargebefore the decode, verified by the new specs and by peak heap measurementTypes of changes
Checklist: