Skip to content

ROB: tolerate malformed /Decode array in image extraction#3899

Open
metsw24-max wants to merge 2 commits into
py-pdf:mainfrom
metsw24-max:image-decode-array-bounds
Open

ROB: tolerate malformed /Decode array in image extraction#3899
metsw24-max wants to merge 2 commits into
py-pdf:mainfrom
metsw24-max:image-decode-array-bounds

Conversation

@metsw24-max

Copy link
Copy Markdown
Contributor

When extracting an image, _apply_decode reads the image XObject's /Decode entry and walks it in [min, max] pairs to build the remap table, taking decode[i + 1] at each step. That value comes straight from the PDF and is only meaningful as an even-length array of numbers, but the length was never checked, so an object whose /Decode carries an odd number of entries reads past the end of the list and a non-array /Decode has no length at all. Both escape through reader.pages[i].images, which is the public path that reaches _xobj_to_image and then _apply_decode, so a crafted file aborts image extraction rather than degrading. I came across it on a fuzzed image whose /Decode held three numbers, where the decode[i + 1] access went out of range. The guard rejects a /Decode that is not a list or tuple of even length, warns, and falls back to rendering the image without the remap, which keeps a malformed array a best-effort image instead of an exception. I kept the change inside _apply_decode so valid even-length arrays are unaffected, and added a regression test alongside the existing decode cases.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.86%. Comparing base (75e339c) to head (ed84111).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3899      +/-   ##
==========================================
+ Coverage   97.85%   97.86%   +0.01%     
==========================================
  Files          57       57              
  Lines       10702    10741      +39     
  Branches     2001     2007       +6     
==========================================
+ Hits        10472    10512      +40     
  Misses        127      127              
+ Partials      103      102       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pypdf/generic/_image_xobject.py Outdated
):
decode = [1.0, 0.0] * len(img.getbands())
if decode is not None and (
not isinstance(decode, (list, tuple)) or len(decode) % 2 != 0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will this be a tuple?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never in practice. It only ever comes from the image dictionary, where it parses as an ArrayObject (a list subclass), or from the literal lists built above. Dropped the tuple branch and kept it to list.

Comment thread pypdf/generic/_image_xobject.py Outdated
# array would otherwise read past its end at decode[i + 1]; drop it and
# render the image without the remap.
logger_warning(
"Ignoring malformed /Decode array; expected an even number of values.",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Ignoring malformed /Decode array; expected an even number of values.",
"Ignoring malformed /Decode array %(decode)s; expected an even number of values.",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, applied and passing decode through so the value lands in the message.

@pytest.mark.parametrize(
"decode",
[
ArrayObject([NumberObject(1), NumberObject(0), NumberObject(1)]),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider setting meaningful IDs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ids: odd-length-array, single-value-array, not-an-array.

Comment thread tests/generic/test_image_xobject.py Outdated
x_object = StreamObject()
x_object[NameObject(ImageAttributes.WIDTH)] = NumberObject(2)
x_object[NameObject(ImageAttributes.HEIGHT)] = NumberObject(2)
x_object[NameObject("/BitsPerComponent")] = NumberObject(8)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
x_object[NameObject("/BitsPerComponent")] = NumberObject(8)
x_object[NameObject(ImageAttributes.BITS_PER_COMPONENT)] = NumberObject(8)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread tests/generic/test_image_xobject.py Outdated
x_object[NameObject(ImageAttributes.WIDTH)] = NumberObject(2)
x_object[NameObject(ImageAttributes.HEIGHT)] = NumberObject(2)
x_object[NameObject("/BitsPerComponent")] = NumberObject(8)
x_object[NameObject(ImageAttributes.COLOR_SPACE)] = NameObject("/DeviceGray")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
x_object[NameObject(ImageAttributes.COLOR_SPACE)] = NameObject("/DeviceGray")
x_object[NameObject(ImageAttributes.COLOR_SPACE)] = NameObject(ColorSpace.DEVICE_GRAY)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, using ColorSpaces.DEVICE_GRAY.

@metsw24-max

Copy link
Copy Markdown
Contributor Author

gentle ping

@stefan6419846

Copy link
Copy Markdown
Collaborator

See #3900.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants