Skip to content

fix BitBuffer overflow in rastertohp OutputLine deinterleave#1650

Open
aizu-m wants to merge 1 commit into
OpenPrinting:masterfrom
aizu-m:rastertohp-bitbuffer-bounds
Open

fix BitBuffer overflow in rastertohp OutputLine deinterleave#1650
aizu-m wants to merge 1 commit into
OpenPrinting:masterfrom
aizu-m:rastertohp-bitbuffer-bounds

Conversation

@aizu-m

@aizu-m aizu-m commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

ASan, feeding a crafted application/vnd.cups-raster page to a queue that uses an HP PCL PPD:

ERROR: AddressSanitizer: heap-buffer-overflow WRITE of size 1
  #0 OutputLine rastertohp.c:558
  #1 main rastertohp.c:727
0x... is located 0 bytes after 800-byte region
  #1 StartPage rastertohp.c:338

I was tracing the deep-colour (cupsBitsPerColor > 1) branch of OutputLine. StartPage sizes BitBuffer as ColorBits * (cupsWidth + 7) / 8, but the deinterleave loop runs count = cupsBytesPerLine / NumPlanes times and writes bit_ptr[0] and bit_ptr[bytes]. NumPlanes is 1 for any colourspace that is not CMY or KCMY, so an RGB or CMYK header (its cupsNumColors already folded into cupsBytesPerLine) makes count several times larger than the two bytes-wide sub-planes the buffer holds, and bit_ptr[bytes] runs past the allocation. The page above is 800px, 8-bit, RGB.

Same loop, second way in: count is unsigned and the loop only stops on count > 0 with count -= 2, so an odd cupsBytesPerLine / NumPlanes (e.g. a 12px 2-bit black line where cupsBytesPerLine is 3) underflows to ~2^31 and it keeps writing off the heap.

Both reduce to the loop overrunning the two sub-planes, so I bounded bit_ptr to BitBuffer + bytes. Valid input where count is exactly 2 * bytes reaches the bound just as count hits 0, so nothing changes there; I diffed a 2-bit page before and after and the output is byte-identical.

@michaelrsweet michaelrsweet self-assigned this Jul 16, 2026
@michaelrsweet michaelrsweet added the investigating Investigating the issue label Jul 16, 2026
@michaelrsweet

Copy link
Copy Markdown
Member

I'm not sure we need to worry about this with the normal users of this driver, but will investigate.

Would it be possible for you to sign your commits?

Signed-off-by: Aizal Khan <aizumusheer2@gmail.com>
@aizu-m
aizu-m force-pushed the rastertohp-bitbuffer-bounds branch from 5c38711 to b189834 Compare July 17, 2026 10:09
@aizu-m

aizu-m commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Signed off and force-pushed, so the branch now carries the DCO line.

On reachability: the only way in is an untrusted application/vnd.cups-raster job to an HP PCL queue, so it's header fields a job controls rather than anything a normal user would set. An RGB or CMYK header trips it because cupsNumColors is folded into cupsBytesPerLine while NumPlanes stays 1, so count outruns the two sub-planes. Happy to attach the crafted raster if that's useful for the investigation.

@michaelrsweet

Copy link
Copy Markdown
Member

On reachability: the only way in is an untrusted application/vnd.cups-raster job to an HP PCL queue, so it's header fields a job controls rather than anything a normal user would set. An RGB or CMYK header trips it because cupsNumColors is folded into cupsBytesPerLine while NumPlanes stays 1, so count outruns the two sub-planes. Happy to attach the crafted raster if that's useful for the investigation.

No, not needed but thanks. I meant more that nobody uses this part (HP_DESKJET2 or model 2) of the driver anymore since it was dropped in 2007...

@aizu-m

aizu-m commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

That lines up with what I traced. The overrun only sits in the multi-bit deinterleave, which is the model 2 (HP_DESKJET2) branch, so in practice it's a dead path. I'd treat the patch as cheap hardening rather than anything urgent, so fine to take it or leave it, whichever suits.

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

Labels

investigating Investigating the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants