[Security] An uploaded image can exhaust the backend's memory: the validator decodes it with no pixel bound

claude-bot commented

2026-10-04 08:50:04 +00:00

Collaborator

Found while measuring memory for #991 part 2 (PR B). It is in the ordinary upload routes on main today, not in the new code.

What

image_service.validate_and_reencode runs in the backend (512 MB, 340 to 425 MB when warm) for a map upload, a page picture and an image attachment. It decodes the whole image with no bound on its pixel count. Memory follows the pixels, not the file's size.

Measured with the validator alone in a bare interpreter (70 MB baseline), docker run -m 512m:

Picture	Process peak
10 MB JPEG, 4000×3000	173 MB
70 MB PNG, 5000×4900	403 MB
54 MB JPEG, 7000×7000	500 MB
450 KB PNG, 12000×12000	killed (exit 137); killed at 1 GB too

Why it matters

Any signed-in user who can upload a picture can kill a backend worker with a 450 KB file. It is well inside every size cap.
A legitimate large map can do the same. A 70 MB map is within map_max_upload_bytes and does not fit beside a warm backend.

Not measured: the same uploads against a running backend. The figures above are the validator alone; a warm backend has less room, not more.

Intended approach

A pixel bound in the validator, checked from the header before any pixel is decoded. The file is refused in words (422). Two bounds, by what the caller is:
- pictures and image attachments (8 MB cap): a bound low enough to fit beside a warm backend. About 12 MP costs roughly 100 MB by the table; the number needs measuring.
- maps: 8192×6144, the bound PR B of #991 uses for an imported map.
Maps are validated in the worker (1 GB), not the backend, as PR B already does for an imported map. An upload answers once the file is stored, and the map shows "checking" until the worker is done. This is the larger change and may want its own PR.
Tests: a small file with a huge declared size is refused before decoding, and the bound is checked against both dimensions and their product.

#1054: the same 512 MB limit, for an import's data.json.
#841: a backend worker died under load.
#991: PR B validates imported maps in the worker for this reason.

Found while measuring memory for #991 part 2 (PR B). It is in the ordinary upload routes on main today, not in the new code. ## What `image_service.validate_and_reencode` runs in the backend (512 MB, 340 to 425 MB when warm) for a map upload, a page picture and an image attachment. It decodes the whole image with no bound on its pixel count. Memory follows the pixels, not the file's size. Measured with the validator alone in a bare interpreter (70 MB baseline), `docker run -m 512m`: | Picture | Process peak | |---|---| | 10 MB JPEG, 4000×3000 | 173 MB | | 70 MB PNG, 5000×4900 | 403 MB | | 54 MB JPEG, 7000×7000 | 500 MB | | 450 KB PNG, 12000×12000 | killed (exit 137); killed at 1 GB too | ## Why it matters - **Any signed-in user who can upload a picture can kill a backend worker** with a 450 KB file. It is well inside every size cap. - **A legitimate large map can do the same.** A 70 MB map is within `map_max_upload_bytes` and does not fit beside a warm backend. Not measured: the same uploads against a running backend. The figures above are the validator alone; a warm backend has less room, not more. ## Intended approach 1. **A pixel bound in the validator, checked from the header before any pixel is decoded.** The file is refused in words (422). Two bounds, by what the caller is: - pictures and image attachments (8 MB cap): a bound low enough to fit beside a warm backend. About 12 MP costs roughly 100 MB by the table; the number needs measuring. - maps: 8192×6144, the bound PR B of #991 uses for an imported map. 2. **Maps are validated in the worker (1 GB), not the backend**, as PR B already does for an imported map. An upload answers once the file is stored, and the map shows "checking" until the worker is done. This is the larger change and may want its own PR. 3. Tests: a small file with a huge declared size is refused before decoding, and the bound is checked against both dimensions and their product. ## Related - #1054: the same 512 MB limit, for an import's `data.json`. - #841: a backend worker died under load. - #991: PR B validates imported maps in the worker for this reason.

claude-bot referenced this issue

2026-10-04 08:56:10 +00:00

Campaign export leaves out quest details, character sheets, maps and handout files #991

claude-bot referenced this issue

2026-10-04 08:56:18 +00:00

Media directories with no campaign are never swept #1075

rbrooks referenced this issue from a commit

2026-10-04 09:21:10 +00:00

An import of files is bounded, a file over its cap is left out, and a crash loses nothing (#991 part 2, review of schema 18)

claude-bot referenced this issue

2026-10-04 09:26:40 +00:00

A campaign's export carries handout files and maps (export schema 18) (#991) #1076

claude-bot referenced this issue

2026-10-04 09:26:56 +00:00

Map jobs share the general worker: a large import can hold up recordings #1077

claude-bot referenced this issue

2026-10-04 10:33:01 +00:00

Campaign export leaves out quest details, character sheets, maps and handout files #991

claude-bot commented

2026-10-04 10:33:50 +00:00

Author

Collaborator

Picking this up, on fix/1074-image-pixel-bounds, now that #1076 has landed (both touch the image checks).

Planned

Part 1: a pixel bound (pixels × frames, read from the header) in front of every caller of the validator, refused with a 422 in words.
Part 2, if it stays contained: a map upload is stored and checked in the worker, as an imported map already is.

A correction to the bound #1076 shipped for imports. It is 4000×3000, and a common phone photo is 4032×3024, which is just over. So today an import leaves such a page picture out (listed). The bound will be chosen by measurement so that it takes 4032×3024, and uploads and imports will share one constant.

**Picking this up**, on `fix/1074-image-pixel-bounds`, now that #1076 has landed (both touch the image checks). **Planned** - **Part 1:** a pixel bound (pixels × frames, read from the header) in front of every caller of the validator, refused with a 422 in words. - **Part 2, if it stays contained:** a map upload is stored and checked in the worker, as an imported map already is. **A correction to the bound #1076 shipped for imports.** It is 4000×3000, and a common phone photo is 4032×3024, which is just over. So today an import leaves such a page picture out (listed). The bound will be chosen by measurement so that it takes 4032×3024, and uploads and imports will share one constant.

rbrooks referenced this issue from a commit

2026-10-04 12:26:06 +00:00

A picture's pixels are counted before it is decoded, and a map's picture is checked in the worker (#1074)

rbrooks referenced this issue from a commit

2026-10-04 13:11:48 +00:00

An animated GIF is counted by what it costs: six frames and a quarter of them all (#1074)

claude-bot referenced this issue from a pull request that will close it,

2026-10-04 13:12:43 +00:00

A picture is checked by its pixels before it is opened, and a map upload is checked in the worker (#1074) #1078

claude-bot referenced this issue

2026-10-04 13:28:31 +00:00

A photo that relies on its EXIF orientation is stored sideways #1079

rbrooks referenced this issue from a commit

2026-10-04 14:12:09 +00:00

Close eight defects the PR 1078 review found, with a test each (#1074)

rbrooks referenced this issue from a commit

2026-10-04 14:57:55 +00:00

Every JPEG that libjpeg buffers whole counts as progressive (#1074)

rbrooks referenced this issue from a commit

2026-10-04 14:57:55 +00:00

Format the JPEG profile tests (#1074)

rbrooks referenced this issue from a commit

2026-10-04 15:27:17 +00:00

Re-run CI: Frontend shard 1 failed on AppFrame.test.jsx, which this branch does not touch (#1074)

claude-bot referenced this issue from a commit

2026-10-04 16:37:49 +00:00

Merge PR #1078: a picture is checked by its pixels before it is opened, and a map upload is checked in the worker (#1074)

claude-bot closed this issue

2026-10-04 16:37:49 +00:00

claude-bot commented

2026-10-04 16:37:57 +00:00

Author

Collaborator

Fixed in PR #1078, merged to main. CI was green on the merged commit.

What it does

Every picture upload and import is checked by its pixels and frames from the header before anything is decoded. The 450 KB file of 12000×12000 is refused with memory flat.
A map upload is stored and checked in the worker.
One picture is decoded at a time per container, off the event loop.

The bounds

A picture or image attachment: 4080×3072 (12.53 million pixels), which takes a 4032×3024 phone photo. Imports share it; the 4000×3000 that #1076 shipped is gone.
A map: 8192×6144.
WebP counts four times, a CMYK JPEG twice, a progressive JPEG twice in the backend.
An animated GIF: 6 × frame + total / 4 ≤ bound, at most 1,000 frames, and no frame larger than its screen.
A JPEG with more than 32 scans is refused.

Found on the way

An independent review found a bypass through a crafted GIF and seven smaller defects; all are fixed.
A second read found the progressive weight too narrow; fixed.
JPEG and GIF comments survived the re-encode (on main too). They are stripped now.

Follow-ups

#1079: EXIF orientation is never applied.
#1077: map jobs share the general worker.
#1075: orphaned media directories.
For the owner: whether large photos should be shrunk on upload instead of refused. A 24-megapixel photo is refused today.

Not measured: the cost of an arithmetic-coded or non-interleaved JPEG. Those are weighted as progressive from libjpeg's source.

Reaches dev with v4.5.0-rc4. Production (4.4.1) still has the hole.

**Fixed in PR #1078, merged to main. CI was green on the merged commit.** **What it does** - Every picture upload and import is checked by its pixels and frames from the header before anything is decoded. The 450 KB file of 12000×12000 is refused with memory flat. - A map upload is stored and checked in the worker. - One picture is decoded at a time per container, off the event loop. **The bounds** - A picture or image attachment: 4080×3072 (12.53 million pixels), which takes a 4032×3024 phone photo. Imports share it; the 4000×3000 that #1076 shipped is gone. - A map: 8192×6144. - WebP counts four times, a CMYK JPEG twice, a progressive JPEG twice in the backend. - An animated GIF: `6 × frame + total / 4 ≤ bound`, at most 1,000 frames, and no frame larger than its screen. - A JPEG with more than 32 scans is refused. **Found on the way** - An independent review found a bypass through a crafted GIF and seven smaller defects; all are fixed. - A second read found the progressive weight too narrow; fixed. - JPEG and GIF comments survived the re-encode (on main too). They are stripped now. **Follow-ups** - #1079: EXIF orientation is never applied. - #1077: map jobs share the general worker. - #1075: orphaned media directories. - **For the owner:** whether large photos should be shrunk on upload instead of refused. A 24-megapixel photo is refused today. **Not measured:** the cost of an arithmetic-coded or non-interleaved JPEG. Those are weighted as progressive from libjpeg's source. Reaches dev with `v4.5.0-rc4`. Production (4.4.1) still has the hole.

claude-bot referenced this issue

2026-10-04 20:54:56 +00:00

[Security] A JPEG's EXIF can exhaust the backend's memory when the file is opened #1087

rbrooks referenced this issue from a commit

2026-10-04 22:35:57 +00:00

Strip a picture's metadata before it is opened, not only before it is saved (#1087)

claude-bot referenced this issue

2026-10-05 05:45:41 +00:00

A backend worker died under load after a notifying POST (blocking calls in async handlers?) #841

claude-bot referenced this issue

2026-10-05 05:46:00 +00:00

A picture too large for the server is shrunk on the way in, not refused #1096