Skip to content

Raise SyntaxError for non-numeric XBM #define values - #10319

Merged
radarhere merged 5 commits into
python-pillow:mainfrom
LemonCondor:fix/xbm-non-numeric-define
Oct 28, 2026
Merged

radarhere merged 5 commits into
python-pillow:mainfrom
LemonCondor:fix/xbm-non-numeric-define

Conversation

@LemonCondor

Copy link
Copy Markdown
Contributor

Changes proposed in this pull request:

  • Since Read XBM header line by line, instead of through regex #10109, Image.open() on a file starting with #define FOO bar (e.g. a C header) raises ValueError from int() instead of UnidentifiedImageError, because ValueError is not one of the exceptions open() treats as "not this format".
  • Raise SyntaxError("not a XBM file") instead.
  • Add a test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@radarhere radarhere added the 🤖-assisted AI-assisted label Oct 26, 2026
@radarhere

Copy link
Copy Markdown
Member

Hi. I'm not convinced this is unequivocally a bug. Could you explain why you think it is? I don't expect you encountered a file starting with "#define FOO bar" that didn't intended to be an XBM file or similar, so I think we have identified the image.

@BastienGimbert

Copy link
Copy Markdown

I had the same problem too.

I agree that #define FOO bar alone may be considered an invalid XBM file. The stronger problem is a valid XBM file generated by Pillow.

This looks like a regression from the fix for CVE-2026-112901. Commit 2852a3e3d changed the XBM parser from a regular expression to a line-by-line parser. Since this change, Pillow 13.0.0 and main at 5e898683d fail when a valid XBM file is received in small parts.

I tested Pillow 12.3.0, Pillow 13.0.0, and main (13.1.0.dev0, commit 5e898683d). Here are all my A/B results:

1. 12-byte input: b"#define foo\n"

   Pillow 12.3.0:       survives, UnidentifiedImageError
   Pillow 13.0.0:       fails, unhandled ValueError for b"foo"
   Pillow main:         fails, unhandled ValueError for b"foo"

2. Same input, with a handler for Exception

   Pillow 12.3.0:       survives
   Pillow 13.0.0:       survives
   Pillow main:         survives

3. Same input, with formats=["PNG"]

   Pillow 12.3.0:       survives, rejected
   Pillow 13.0.0:       survives, rejected
   Pillow main:         survives, rejected

4. Complete valid XBM file

   Pillow 12.3.0:       accepted as XBM (8, 8)
   Pillow 13.0.0:       accepted as XBM (8, 8)
   Pillow main:         accepted as XBM (8, 8)

5. The same valid XBM sent in 8-byte blocks

   Pillow 12.3.0:       parsed as XBM (8, 8)
   Pillow 13.0.0:       fails with ValueError for b"im_width"
   Pillow main:         fails with ValueError for b"im_width"

The valid XBM was created with Pillow:

out = io.BytesIO()
Image.new("1", (8, 8), 0).save(out, "XBM")
data = out.getvalue()

The 8-byte parser test was:

parser = ImageFile.Parser()
for i in range(0, len(data), 8):
    parser.feed(data[i:i + 8])
parser.close()

With one partial block, the parser sees:

b"#define im_width"

The value has not arrived yet, but XbmImagePlugin.py runs:

value = int(line.split().pop())

This calls int(b"im_width") and raises ValueError.

Parser.feed() only catches OSError while waiting for more data:

try:
    with io.BytesIO(self.data) as fp:
        im = Image.open(fp)
except OSError:
    pass

Therefore, it cannot keep the partial data and wait for the next block.

The fix proposed in this PR #10319 should solve both cases:

try:
    value = int(line.split().pop())
except ValueError as e:
    raise SyntaxError("not a XBM file") from e

After applying the same fix locally, my test gave:

Complete valid XBM:       XBM (8, 8)
Valid XBM in 8-byte blocks: XBM (8, 8)
b"#define foo\n":          UnidentifiedImageError

So I think the non-numeric #define test is useful, but the streaming test is important too, I will add it rn

@BastienGimbert

BastienGimbert commented Oct 26, 2026 •

Copy link
Copy Markdown

@LemonCondor I opened a PR on your fork to add test_parser_streaming() to this branch (test pass). Feel free to review it and merge it into your PR if it looks good to you, so @radarhere can check everything together.
Have a nice day !

@LemonCondor

Copy link
Copy Markdown
Contributor Author

Thanks for looking at this. To be transparent: I found it while reading the parser after #10109, not from a real-world file. I agree that a lone #define FOO bar is a weak case on its own.

My reasoning was about the Image.open() contract: when a plugin's _open() decides the data isn't its format, it raises SyntaxError, and open() then moves on and finally raises UnidentifiedImageError. Here the int() call leaks a ValueError, which open() doesn't catch, so the caller gets a different exception than for any other unrecognised input.

@BastienGimbert's case is stronger: an XBM file written by Pillow itself fails when fed in small chunks through ImageFile.Parser. I've merged his test_parser_streaming() into this branch, thanks Bastien.

If you'd rather handle this differently, I'm happy to adjust or close.

@radarhere

Copy link
Copy Markdown
Member

I'm not entirely convinced by the Parser scenario, as I think the chunk size demonstrated is unrealistically small, and I don't know about the longevity of Parser in general - but I'll merge.

@radarhere
radarhere enabled auto-merge (squash) October 28, 2026 09:22
@radarhere
radarhere merged commit cb77e1b into python-pillow:main Oct 28, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-assisted AI-assisted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants