Skip to content

pclmtoraster: reject a page whose image stream is shorter than declared - #276

Open
ManasBagul23 wants to merge 1 commit into
OpenPrinting:masterfrom
ManasBagul23:fix-pclmtoraster-short-stream-oob
Open

ManasBagul23 wants to merge 1 commit into
OpenPrinting:masterfrom
ManasBagul23:fix-pclmtoraster-short-stream-oob

Conversation

@ManasBagul23

Copy link
Copy Markdown

process_image() grows data->bitmap by exactly the number of bytes it actually reads from each PDF image's decoded stream (data->pixel_count), independently of the Width/Height the same image object declares in its dictionary. out_page() then derives data->rowsize from the declared cupsWidth/colorspace and walks data->bitmap for cupsHeight rows of rowsize bytes each -- both in rotate_bitmap() and in the per-line convertline()/cupsRasterWritePixels() loop -- without ever checking that pixel_count actually covers rowsize * cupsHeight.

A syntactically valid PDF/PCLm page that declares large dimensions but supplies a short (or near-empty) image stream reaches a heap-buffer-overflow read in both paths, and a heap-buffer-overflow write in rotate_bitmap()'s destination once a rotation is applied (#185).

Validate pixel_count against the declared geometry (in size_t, so a large declared width/height can't wrap the comparison itself) right after the convert function -- and therefore rowsize -- is selected, and before any rotation or pixel conversion touches the buffer. Reject the page the same way the existing cupsRasterWriteHeader() failure a few lines above does: log and return 1. The check only applies when the page actually declares pixel rows (cupsHeight > 0) and a page actually decoded some data (rowsize > 0), so a legitimate empty/blank page still raster-writes just its header as before.

Verified by tracing data->pixel_count's only producer (process_image(), now already fixed upstream to read each image stream to completion) and rowsize's definition (header.cupsWidth and the selected colorspace, both attacker-declared) against both consumers this guard protects: rotate_bitmap() and the convertline() loop in out_page(). No C build toolchain available locally to compile-check.

Fixes #185

process_image() grows data->bitmap by exactly the number of bytes it
actually reads from each PDF image's decoded stream (data->pixel_count),
independently of the Width/Height the same image object declares in its
dictionary. out_page() then derives data->rowsize from the declared
cupsWidth/colorspace and walks data->bitmap for cupsHeight rows of
rowsize bytes each -- both in rotate_bitmap() and in the per-line
convertline()/cupsRasterWritePixels() loop -- without ever checking that
pixel_count actually covers rowsize * cupsHeight.

A syntactically valid PDF/PCLm page that declares large dimensions but
supplies a short (or near-empty) image stream reaches a
heap-buffer-overflow read in both paths, and a heap-buffer-overflow
write in rotate_bitmap()'s destination once a rotation is applied (OpenPrinting#185).

Validate pixel_count against the declared geometry (in size_t, so a
large declared width/height can't wrap the comparison itself) right
after the convert function -- and therefore rowsize -- is selected, and
before any rotation or pixel conversion touches the buffer. Reject the
page the same way the existing cupsRasterWriteHeader() failure a few
lines above does: log and return 1. The check only applies when the
page actually declares pixel rows (cupsHeight > 0) and a page actually
decoded some data (rowsize > 0), so a legitimate empty/blank page still
raster-writes just its header as before.

Verified by tracing data->pixel_count's only producer (process_image(),
now already fixed upstream to read each image stream to completion) and
rowsize's definition (header.cupsWidth and the selected colorspace, both
attacker-declared) against both consumers this guard protects:
rotate_bitmap() and the convertline() loop in out_page(). No C build
toolchain available locally to compile-check.

Fixes OpenPrinting#185
Copilot AI balanced review requested due to automatic review settings October 8, 2026 18:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

pclmtoraster: declared image dimensions exceed decoded stream storage

2 participants