From 3cbe18490a93d1cde57cc475222c0449d69837fa Mon Sep 17 00:00:00 2001 From: Giovanni Giacobbi Date: Sun, 4 Aug 2024 13:59:29 +0200 Subject: [PATCH 1/3] imageloadfont: Fix font loading in opposite endianness format This function used to (partially) be able to load fonts of the opposite endianness until php 7.4.0. This change restores this ability and also adds the missing part when the first character offset is non zero. --- NEWS | 4 ++ ext/gd/gd.c | 24 ++++---- ext/gd/tests/imageloadfont_endianness.phpt | 66 ++++++++++++++++++++++ 3 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 ext/gd/tests/imageloadfont_endianness.phpt diff --git a/NEWS b/NEWS index 343a59f4bf2a3..591b6f7da21f0 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,10 @@ PHP NEWS . Fixed bug GH-15192 (Segmentation fault in dom extension (html5_serializer)). (nielsdos) +- GD: + . Fixed imageloadfont() when loading font files for the opposite endianness, + including those with non-zero offset field. (Giovanni Giacobbi) + - PHPDBG: . array out of bounds, stack overflow handled for segfault handler on windows. (David Carlier) diff --git a/ext/gd/gd.c b/ext/gd/gd.c index 5af2d4a5152bb..09bbd923705f1 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -575,7 +575,17 @@ PHP_FUNCTION(imageloadfont) body_size_check = php_stream_tell(stream) - hdr_size; php_stream_seek(stream, i, SEEK_SET); - if (overflow2(font->nchars, font->h) || overflow2(font->nchars * font->h, font->w )) { + /* Typically a font should only contain 256 characters. If the total + * characters exceeds a two bytes value, assume opposite endianness. */ + if ((font->nchars < 0) || (font->nchars > 0xffff)) { + font->nchars = FLIPWORD(font->nchars); + font->offset = FLIPWORD(font->offset); + font->w = FLIPWORD(font->w); + font->h = FLIPWORD(font->h); + } + + if (overflow2(font->nchars, font->h) || overflow2(font->nchars * font->h, font->w) || + (font->offset < 0) || (font->offset > 0xff)) { php_error_docref(NULL, E_WARNING, "Error reading font, invalid font header"); efree(font); php_stream_close(stream); @@ -583,18 +593,6 @@ PHP_FUNCTION(imageloadfont) } body_size = font->w * font->h * font->nchars; - if (body_size != body_size_check) { - font->w = FLIPWORD(font->w); - font->h = FLIPWORD(font->h); - font->nchars = FLIPWORD(font->nchars); - if (overflow2(font->nchars, font->h) || overflow2(font->nchars * font->h, font->w )) { - php_error_docref(NULL, E_WARNING, "Error reading font, invalid font header"); - efree(font); - php_stream_close(stream); - RETURN_FALSE; - } - body_size = font->w * font->h * font->nchars; - } if (body_size != body_size_check) { php_error_docref(NULL, E_WARNING, "Error reading font"); diff --git a/ext/gd/tests/imageloadfont_endianness.phpt b/ext/gd/tests/imageloadfont_endianness.phpt new file mode 100644 index 0000000000000..76223613b5eac --- /dev/null +++ b/ext/gd/tests/imageloadfont_endianness.phpt @@ -0,0 +1,66 @@ +--TEST-- +imageloadfont() detects endianness correctly +--EXTENSIONS-- +gd +--FILE-- + $file_LE, "BE" => $file_BE) as $key => $file) { + print "\n=== $key ===\n"; + $font = imageloadfont($file); + print "width = "; var_dump(imagefontwidth($font)); + print "height = "; var_dump(imagefontheight($font)); + $im = imagecreate(4, 1); + imagecolorallocate($im, 255, 255, 255); + imagechar($im, $font, 0, 0, ' ', imagecolorallocate($im, 0, 0, 0)); + print "pixels = "; var_export(array( + imagecolorat($im, 0, 0), + imagecolorat($im, 1, 0), + imagecolorat($im, 2, 0), + imagecolorat($im, 3, 0))); + print "\n"; + imagedestroy($im); +} +unlink($file_BE); +unlink($file_LE); +?> +--EXPECT-- + +=== LE === +width = int(4) +height = int(1) +pixels = array ( + 0 => 1, + 1 => 0, + 2 => 1, + 3 => 1, +) + +=== BE === +width = int(4) +height = int(1) +pixels = array ( + 0 => 1, + 1 => 0, + 2 => 1, + 3 => 1, +) From 2cb653681fcd35acf8e611a9b05ff0c52b218803 Mon Sep 17 00:00:00 2001 From: Giovanni Giacobbi Date: Sun, 4 Aug 2024 21:00:01 +0200 Subject: [PATCH 2/3] FIXUP related test output changed --- ext/gd/tests/bug81739.phpt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ext/gd/tests/bug81739.phpt b/ext/gd/tests/bug81739.phpt index b340aa7c07580..f81095d307091 100644 --- a/ext/gd/tests/bug81739.phpt +++ b/ext/gd/tests/bug81739.phpt @@ -15,8 +15,5 @@ var_dump(imageloadfont(__DIR__ . "/font.font")); @unlink(__DIR__ . "/font.font"); ?> --EXPECTF-- -Warning: imageloadfont(): %croduct of memory allocation multiplication would exceed INT_MAX, failing operation gracefully - in %s on line %d - -Warning: imageloadfont(): Error reading font, invalid font header in %s on line %d -bool(false) \ No newline at end of file +Warning: imageloadfont(): Error reading font in %s on line %d +bool(false) From c9112a0ec2026475dcd3daa89992753d15738b2f Mon Sep 17 00:00:00 2001 From: Giovanni Giacobbi Date: Mon, 5 Aug 2024 10:22:17 +0200 Subject: [PATCH 3/3] WIP use a more portable approach --- ext/gd/gd.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/ext/gd/gd.c b/ext/gd/gd.c index 09bbd923705f1..85ade74feb263 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -528,10 +528,10 @@ PHP_FUNCTION(gd_info) PHP_FUNCTION(imageloadfont) { zend_string *file; - int hdr_size = sizeof(gdFont) - sizeof(char *); int body_size, n = 0, b, i, body_size_check; gdFontPtr font; php_stream *stream; + uint32_t hdr_buf[4]; ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_PATH_STR(file) @@ -542,9 +542,7 @@ PHP_FUNCTION(imageloadfont) RETURN_FALSE; } - /* Only supports a architecture-dependent binary dump format - * at the moment. - * The file format is like this on machines with 32-byte integers: + /* The file format is like this on machines with 32-byte integers: * * byte 0-3: (int) number of characters in the font * byte 4-7: (int) value of first character in the font (often 32, space) @@ -554,14 +552,7 @@ PHP_FUNCTION(imageloadfont) * in each character, for a total of * (nchars*width*height) bytes. */ - font = (gdFontPtr) emalloc(sizeof(gdFont)); - b = 0; - while (b < hdr_size && (n = php_stream_read(stream, (char*)&font[b], hdr_size - b)) > 0) { - b += n; - } - - if (n <= 0) { - efree(font); + if (php_stream_read(stream, (char *) hdr_buf, sizeof(hdr_buf)) != sizeof(hdr_buf)) { if (php_stream_eof(stream)) { php_error_docref(NULL, E_WARNING, "End of file while reading header"); } else { @@ -570,18 +561,22 @@ PHP_FUNCTION(imageloadfont) php_stream_close(stream); RETURN_FALSE; } - i = php_stream_tell(stream); - php_stream_seek(stream, 0, SEEK_END); - body_size_check = php_stream_tell(stream) - hdr_size; - php_stream_seek(stream, i, SEEK_SET); + + font = (gdFontPtr) emalloc(sizeof(gdFont)); /* Typically a font should only contain 256 characters. If the total * characters exceeds a two bytes value, assume opposite endianness. */ - if ((font->nchars < 0) || (font->nchars > 0xffff)) { - font->nchars = FLIPWORD(font->nchars); - font->offset = FLIPWORD(font->offset); - font->w = FLIPWORD(font->w); - font->h = FLIPWORD(font->h); + if (hdr_buf[0] > 0xffff) { + font->nchars = (int) FLIPWORD(hdr_buf[0]); + font->offset = (int) FLIPWORD(hdr_buf[1]); + font->w = (int) FLIPWORD(hdr_buf[2]); + font->h = (int) FLIPWORD(hdr_buf[3]); + } + else { + font->nchars = (int) hdr_buf[0]; + font->offset = (int) hdr_buf[1]; + font->w = (int) hdr_buf[2]; + font->h = (int) hdr_buf[3]; } if (overflow2(font->nchars, font->h) || overflow2(font->nchars * font->h, font->w) || @@ -592,6 +587,11 @@ PHP_FUNCTION(imageloadfont) RETURN_FALSE; } + i = php_stream_tell(stream); + php_stream_seek(stream, 0, SEEK_END); + body_size_check = php_stream_tell(stream) - sizeof(hdr_buf); + php_stream_seek(stream, i, SEEK_SET); + body_size = font->w * font->h * font->nchars; if (body_size != body_size_check) {