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

Closed
opened 2026-10-04 20:54:56 +00:00 by claude-bot · 3 comments
Collaborator

Found while building #1079. It is on main today, and #1078's pixel check does not cover it, because it happens before that check.

What

Pillow parses a JPEG's EXIF inside Image.open: JpegImagePlugin._open calls _read_dpi_from_exif, which calls getexif, which reads the data of every directory entry. An entry's data offset and length are the file's own claims, and many entries may claim the same bytes.

Measured in a 512 MB container, Image.open alone:

File Peak RSS
6.3 MB JPEG, EXIF spread over many APP1 segments, 20 entries each claiming the same 6 MB 189 MB
The same with 60 entries 429 MB

More entries would end a 512 MB backend. Compare a PNG with 5,000 such entries: the validator took 0.10 s and 83 MB, because PNG's EXIF is not parsed at open. Calling getexif on it was killed.

Why it matters

  • read_header calls Image.open first, before check_pixels and outside the one-picture lock.
  • Any signed-in user who can upload a picture reaches it, on the picture, attachment and map routes and through an import. The file is inside every size cap.
  • Same class as #1074.

The script that built the files was not kept. The construction is the two lines above the table.

Intended approach

Decide by measurement between:

  1. Check the raw bytes before Pillow sees them. Walk the JPEG's markers (the walk in _jpeg_profile already exists), and refuse a file whose EXIF directory claims more data than its EXIF block holds, or whose APP1 segments total more than a small bound.
  2. Strip the APP1 segments from the bytes handed to Pillow, after reading the orientation with #1079's bounded parser. Pillow then has no EXIF to parse.

Either way:

  • Check what else Image.open parses eagerly for each allowed format (ICC profiles, MPO directories, XMP, PNG text, WebP chunks), and bound each.
  • Tests: the hostile file is refused or opened at flat memory; an ordinary phone photo with a large but honest EXIF block (maker notes, an embedded thumbnail) is still accepted and still turned.
  • An independent review before merging, as for #1078.

#1074, #1078, #1079.

Found while building #1079. It is on main today, and #1078's pixel check does not cover it, because it happens before that check. ## What Pillow parses a JPEG's EXIF inside `Image.open`: `JpegImagePlugin._open` calls `_read_dpi_from_exif`, which calls `getexif`, which reads the data of every directory entry. An entry's data offset and length are the file's own claims, and many entries may claim the same bytes. Measured in a 512 MB container, `Image.open` alone: | File | Peak RSS | |---|---| | 6.3 MB JPEG, EXIF spread over many APP1 segments, 20 entries each claiming the same 6 MB | 189 MB | | The same with 60 entries | 429 MB | More entries would end a 512 MB backend. Compare a PNG with 5,000 such entries: the validator took 0.10 s and 83 MB, because PNG's EXIF is not parsed at open. Calling `getexif` on it was killed. ## Why it matters - `read_header` calls `Image.open` first, before `check_pixels` and outside the one-picture lock. - Any signed-in user who can upload a picture reaches it, on the picture, attachment and map routes and through an import. The file is inside every size cap. - Same class as #1074. The script that built the files was not kept. The construction is the two lines above the table. ## Intended approach Decide by measurement between: 1. **Check the raw bytes before Pillow sees them.** Walk the JPEG's markers (the walk in `_jpeg_profile` already exists), and refuse a file whose EXIF directory claims more data than its EXIF block holds, or whose APP1 segments total more than a small bound. 2. **Strip the APP1 segments from the bytes handed to Pillow**, after reading the orientation with #1079's bounded parser. Pillow then has no EXIF to parse. Either way: - Check what else `Image.open` parses eagerly for each allowed format (ICC profiles, MPO directories, XMP, PNG text, WebP chunks), and bound each. - Tests: the hostile file is refused or opened at flat memory; an ordinary phone photo with a large but honest EXIF block (maker notes, an embedded thumbnail) is still accepted and still turned. - An independent review before merging, as for #1078. ## Related #1074, #1078, #1079.
Author
Collaborator

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

  • The change: a picture's metadata is removed from its bytes before Pillow opens it, in JPEG, PNG, WebP and GIF. One function opens upload and import bytes, and a test holds that.
  • Why not a size limit: the worst file found was 70 kB (a crafted MPF directory, +1.6 GB at open).
  • Measured: the crafted EXIF and MPF files now cost +0 MB at open. Through nginx on a warm backend, memory stayed flat with oom_kill 0.
  • Review: an independent review found a bypass through rare JPEG markers and two regressions for honest files (YCCK colours, large PNGs). All were fixed before the merge, and the marker table is now held to Pillow's by a test.

Left as they are (time, not memory, and on main before this): a crafted 8 MB JPEG takes 2 to 3 s to check, and a crafted 75 MB map about 18 s in the worker.

Not done: a second review of the fix commit; a genuine Photoshop file.

Reaches dev with the next prerelease. Production (4.4.1) still has the hole.

**Fixed in PR #1088, merged to main. CI was green on the merged commit.** - **The change:** a picture's metadata is removed from its bytes before Pillow opens it, in JPEG, PNG, WebP and GIF. One function opens upload and import bytes, and a test holds that. - **Why not a size limit:** the worst file found was 70 kB (a crafted MPF directory, +1.6 GB at open). - **Measured:** the crafted EXIF and MPF files now cost +0 MB at open. Through nginx on a warm backend, memory stayed flat with `oom_kill 0`. - **Review:** an independent review found a bypass through rare JPEG markers and two regressions for honest files (YCCK colours, large PNGs). All were fixed before the merge, and the marker table is now held to Pillow's by a test. **Left as they are (time, not memory, and on main before this):** a crafted 8 MB JPEG takes 2 to 3 s to check, and a crafted 75 MB map about 18 s in the worker. **Not done:** a second review of the fix commit; a genuine Photoshop file. Reaches dev with the next prerelease. Production (4.4.1) still has the hole.
Author
Collaborator

Reopened: the fix in #1088 can be bypassed. Found by the independent review of PR #1103, confirmed by running. It is on main and on dev (rc6). Production has none of this code.

The bypass

  • _strip_jpeg hands back the file unstripped when its loop stops before a scan. That happens at an end marker before any scan, at a segment whose length is under 2, or when no further marker is found.
  • Pillow does not stop at those points. It steps over an end marker or a zero-length segment and goes on to parse the metadata that follows.
  • Measured: with four bytes (FF D8 FF D9) in front, the 66 KB MPF file from this repo's own tests went from 0.0 MB to 1,650.6 MB at open, and the EXIF file from 0.1 MB to 144 MB.
  • Second form: a zero-length DRI or a one-length DQT after the start marker.

A related bypass of #1074's bound, same cause: the scan walk also stops at that end marker. A progressive 4080×3072 JPEG with the same four bytes in front reads as baseline with no scans, and was decoded whole at +122 MB against the +62 MB the design allows.

Why it was missed: #1088's review and tests checked what each marker's payload could hide. They did not check each way the loop can end early.

The fix, in progress on PR #1103's branch

  • The only way out of the strip loop is a scan. Every other exit refuses the file.
  • The strip and the scan walk share one stepping function.
  • A randomised differential test against Pillow, with a fixed seed.
  • The same exit-by-exit pass over the PNG, WebP and GIF strippers.
**Reopened: the fix in #1088 can be bypassed.** Found by the independent review of PR #1103, confirmed by running. It is on main and on dev (rc6). Production has none of this code. **The bypass** - `_strip_jpeg` hands back the file unstripped when its loop stops before a scan. That happens at an end marker before any scan, at a segment whose length is under 2, or when no further marker is found. - Pillow does not stop at those points. It steps over an end marker or a zero-length segment and goes on to parse the metadata that follows. - **Measured:** with four bytes (`FF D8 FF D9`) in front, the 66 KB MPF file from this repo's own tests went from 0.0 MB to 1,650.6 MB at open, and the EXIF file from 0.1 MB to 144 MB. - **Second form:** a zero-length `DRI` or a one-length `DQT` after the start marker. **A related bypass of #1074's bound, same cause:** the scan walk also stops at that end marker. A progressive 4080×3072 JPEG with the same four bytes in front reads as baseline with no scans, and was decoded whole at +122 MB against the +62 MB the design allows. **Why it was missed:** #1088's review and tests checked what each marker's payload could hide. They did not check each way the loop can end early. **The fix, in progress on PR #1103's branch** - The only way out of the strip loop is a scan. Every other exit refuses the file. - The strip and the scan walk share one stepping function. - A randomised differential test against Pillow, with a fixed seed. - The same exit-by-exit pass over the PNG, WebP and GIF strippers.
Author
Collaborator

The bypass is closed in PR #1103, merged to main. CI was green on the merged commit. Closing again.

  • The change: one stepping routine walks a JPEG's header as Pillow does and ends at a scan or refuses. The strip and the scan walk both use it, so neither can stop where Pillow carries on.
  • Measured: the EXIF and MPF files with the four-byte prefix, and behind a zero-length or one-length segment, are refused at 0.0 MB. Before: 144 MB and over 1,650 MB.
  • Re-run by me against the merged code: the reviewer's proof scripts refuse both prefixed files.
  • Held by: 10,000 spoiled headers from a fixed seed, each refused or opened with no metadata; the two crafted files behind 22 early-stop shapes.
  • The same pass over PNG, WebP and GIF found one more of this shape in the GIF stripper (shielded by an earlier check) and tightened what a cut-short PNG passes on.

Not done: a third independent review, of the fix itself.

Reaches dev with the next prerelease. Until then dev's rc6 has the bypass. Production has none of this code.

**The bypass is closed in PR #1103, merged to main. CI was green on the merged commit. Closing again.** - **The change:** one stepping routine walks a JPEG's header as Pillow does and ends at a scan or refuses. The strip and the scan walk both use it, so neither can stop where Pillow carries on. - **Measured:** the EXIF and MPF files with the four-byte prefix, and behind a zero-length or one-length segment, are refused at 0.0 MB. Before: 144 MB and over 1,650 MB. - **Re-run by me** against the merged code: the reviewer's proof scripts refuse both prefixed files. - **Held by:** 10,000 spoiled headers from a fixed seed, each refused or opened with no metadata; the two crafted files behind 22 early-stop shapes. - **The same pass over PNG, WebP and GIF** found one more of this shape in the GIF stripper (shielded by an earlier check) and tightened what a cut-short PNG passes on. **Not done:** a third independent review, of the fix itself. Reaches dev with the next prerelease. Until then dev's rc6 has the bypass. Production has none of this code.
rbrooks referenced this issue from a commit 2026-10-05 10:27:56 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#1087
No description provided.