Skip to content

ext/gd: fix out-of-bounds write reading font header on short reads#22380

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:gd-imageloadfont-oob
Closed

ext/gd: fix out-of-bounds write reading font header on short reads#22380
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:gd-imageloadfont-oob

Conversation

@iliaal

@iliaal iliaal commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

imageloadfont() indexes the header copy with (char*)&font[b], which scales the byte counter b by sizeof(gdFont) rather than advancing one byte, so a short php_stream_read() (deliverable by a user stream wrapper) writes hdr_size-b bytes past the emalloc(sizeof(gdFont)) buffer. Byte-index the destination to match the body read a few lines below. #15231 rewrites the same loop to read into a sized buffer and would also resolve this, but it has been idle since March, so this is the minimal targeted fix.

imageloadfont() read the font header with `(char*)&font[b]`, which scales
the byte counter b by sizeof(gdFont) rather than advancing one byte, so a
short php_stream_read() (deliverable by a user stream wrapper) makes the
loop write hdr_size-b bytes past the emalloc(sizeof(gdFont)) buffer. Index
the destination by bytes, matching the body read a few lines below.
@iliaal iliaal closed this in df1250d Jun 21, 2026
@iluuu1994

Copy link
Copy Markdown
Member

The new test fails in nightly on PPC64. Can you please have a look?

https://github.com/php/php-src/actions/runs/27996485653/job/82859805119

@iliaal

iliaal commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Test bug, not a code bug. The header was packed with pack('V4', ...) (little-endian), but imageloadfont() reads it as native-endian ints, so on PPC64 the byte-swapped values trip the INT_MAX overflow check before the FLIPWORD fallback runs and the font is rejected. #22416 repacks with pack('i4', ...) to keep it valid on both.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants