From 8774e96bd4d977f893f0e5abc1920ee0968c3c94 Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Sun, 14 Sep 2025 21:03:06 +0800 Subject: [PATCH 01/10] Fix naming clash with libxml macro In the macOS 26 SDK, xmlFree is defined as a macro for free. This causes issues where a same-named variable is used. Renaming the variable to should_free resolves the issue. See: $ grep -B4 -A2 -n "#define xmlFree(" "Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.sdk/usr/include/libxml/globals.h" 261-#if defined(LIBXML_HAS_DEPRECATED_MEMORY_ALLOCATION_FUNCTIONS) 262-#define xmlMalloc(size) malloc(size) 263-#define xmlMallocAtomic(size) malloc(size) 264-#define xmlRealloc(ptr, size) realloc((ptr), (size)) 265:#define xmlFree(ptr) free(ptr) 266-#define xmlMemStrdup(str) strdup(str) 267-#endif Fixes: ``` In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX26.sdk/usr/include/libxml/xmlIO.h:117, from /Library/Developer/CommandLineTools/SDKs/MacOSX26.sdk/usr/include/libxml/parser.h:813, from /private/tmp/php-20250914-13349-uqsk5o/php-8.4.12/ext/dom/php_dom.h:29, from /private/tmp/php-20250914-13349-uqsk5o/php-8.4.12/ext/dom/attr.c:26: /private/tmp/php-20250914-13349-uqsk5o/php-8.4.12/ext/dom/attr.c: In function 'dom_compare_value': /private/tmp/php-20250914-13349-uqsk5o/php-8.4.12/ext/dom/attr.c:208:17: error: called object 'free' is not a function or function pointer 208 | xmlFree(attr_value); | ^~~~~~~ /private/tmp/php-20250914-13349-uqsk5o/php-8.4.12/ext/dom/attr.c:204:14: note: declared here 204 | bool free; | ^~~~ make: *** [ext/dom/attr.lo] Error 1 ``` Closes GH-19832. Signed-off-by: Ruoyu Zhong --- NEWS | 3 +++ ext/dom/attr.c | 6 +++--- ext/dom/php_dom.c | 6 +++--- ext/simplexml/simplexml.c | 6 +++--- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 08c46ee656106..e3eadb6e41b79 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,9 @@ PHP NEWS of the curl_copy_handle() function to clone a CurlHandle. (timwolla) . Fix curl build failure on macOS+curl 8.16. (nielsdos) +- DOM: + . Fix macro name clash on macOS. (Ruoyu Zhong) + - Opcache: . Fixed bug GH-19669 (assertion failure in zend_jit_trace_type_to_info_ex). (Arnaud) diff --git a/ext/dom/attr.c b/ext/dom/attr.c index dfe3abc1a885c..5a0900d657eea 100644 --- a/ext/dom/attr.c +++ b/ext/dom/attr.c @@ -201,10 +201,10 @@ PHP_METHOD(DOMAttr, isId) bool dom_compare_value(const xmlAttr *attr, const xmlChar *value) { - bool free; - xmlChar *attr_value = php_libxml_attr_value(attr, &free); + bool should_free; + xmlChar *attr_value = php_libxml_attr_value(attr, &should_free); bool result = xmlStrEqual(attr_value, value); - if (free) { + if (should_free) { xmlFree(attr_value); } return result; diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 3242529d8842c..2438a081351a0 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -2396,10 +2396,10 @@ void php_dom_get_content_into_zval(const xmlNode *nodep, zval *return_value, boo } case XML_ATTRIBUTE_NODE: { - bool free; - xmlChar *value = php_libxml_attr_value((const xmlAttr *) nodep, &free); + bool should_free; + xmlChar *value = php_libxml_attr_value((const xmlAttr *) nodep, &should_free); RETVAL_STRING_FAST((const char *) value); - if (free) { + if (should_free) { xmlFree(value); } return; diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 619f627e8532a..787c1292e0e0c 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -1529,10 +1529,10 @@ static void sxe_add_registered_namespaces(php_sxe_object *sxe, xmlNodePtr node, /* Attributes in the xmlns namespace should be treated as namespace declarations too. */ if (attr->ns && xmlStrEqual(attr->ns->href, (const xmlChar *) "http://www.w3.org/2000/xmlns/")) { const char *prefix = attr->ns->prefix ? (const char *) attr->name : ""; - bool free; - xmlChar *href = php_libxml_attr_value(attr, &free); + bool should_free; + xmlChar *href = php_libxml_attr_value(attr, &should_free); sxe_add_namespace_name_raw(return_value, prefix, (const char *) href); - if (free) { + if (should_free) { xmlFree(href); } } From 4123b8e108be9e94eeeec775d7f7e9c757185b21 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 7 Sep 2025 01:49:53 +0200 Subject: [PATCH 02/10] exif/heic: Avoid undefined behaviour in address calculations It is illegal to construct out-of-bound pointers, even if they are not dereferenced. The current bound checks rely on undefined behaviour. Fix this by introducing convenience macros that check the remaining length. --- ext/exif/exif.c | 74 +++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index a0744e40f6891..49b2ad6261d1b 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4311,70 +4311,63 @@ static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, iso unsigned char *box_offset, *p, *p2; int header_size, exif_id = -1, version, item_count, i; - for (box_offset = data + 4; box_offset + 16 < end; box_offset += box.size) { + size_t remain; +#define CHECK(n) do { \ + if (remain < (n)) { \ + return; \ + } \ +} while (0) +#define ADVANCE(n) do { \ + CHECK(n); \ + remain -= (n); \ + p += (n); \ +} while (0) + + for (box_offset = data + 4; box_offset < end - 16; box_offset += box.size) { header_size = exif_isobmff_parse_box(box_offset, &box); if (box.size < header_size) { return; } + p = box_offset; + remain = end - p; + if (box.type == FOURCC("iinf")) { - p = box_offset + header_size; - if (p >= end) { - return; - } - version = p[0]; - p += 4; + ADVANCE(header_size + 4); + version = p[-4]; if (version < 2) { - if (p + 2 >= end) { - return; - } - item_count = php_ifd_get16u(p, 1); - p += 2; + ADVANCE(2); + item_count = php_ifd_get16u(p - 2, 1); } else { - if (p + 4 >= end) { - return; - } - item_count = php_ifd_get32u(p, 1); - p += 4; + ADVANCE(4); + item_count = php_ifd_get32u(p - 4, 1); } - for (i = 0; i < item_count && p + 20 < end; i++) { + for (i = 0; i < item_count && p < end - 20; i++) { header_size = exif_isobmff_parse_box(p, &item); if (item.size < header_size) { return; } - if (p + header_size + 12 >= end) { - return; - } + CHECK(header_size + 12); if (!memcmp(p + header_size + 8, "Exif", 4)) { exif_id = php_ifd_get16u(p + header_size + 4, 1); break; } - p += item.size; + ADVANCE(item.size); } if (exif_id < 0) { break; } } else if (box.type == FOURCC("iloc")) { - p = box_offset + header_size; - if (p >= end) { - return; - } - version = p[0]; - p += 6; + ADVANCE(header_size + 6); + version = p[-6]; if (version < 2) { - if (p + 2 >= end) { - return; - } - item_count = php_ifd_get16u(p, 1); - p += 2; + ADVANCE(2); + item_count = php_ifd_get16u(p - 2, 1); } else { - if (p + 4 >= end) { - return; - } - item_count = php_ifd_get32u(p, 1); - p += 4; + ADVANCE(4); + item_count = php_ifd_get32u(p - 4, 1); } - for (i = 0, p2 = p; i < item_count && p + 16 < end; i++, p2 += 16) { + for (i = 0, p2 = p; i < item_count && p < end - 16; i++, p2 += 16) { if (php_ifd_get16u(p2, 1) == exif_id) { pos->offset = php_ifd_get32u(p2 + 8, 1); pos->size = php_ifd_get32u(p2 + 12, 1); @@ -4384,6 +4377,9 @@ static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, iso break; } } + +#undef ADVANCE +#undef CHECK } static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf) From 4e70d41698f476aab71ea1d7b97ba7dd123350fd Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 7 Sep 2025 01:52:35 +0200 Subject: [PATCH 03/10] exif/heic: Fix bound check in loop The loop checks against `p` but increases `p2`. I don't see the point of having 2 separate variables, so use `p` instead to correct the bounds check and simplify the code in the process. --- ext/exif/exif.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index 49b2ad6261d1b..b9d20fb7f9add 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4308,7 +4308,7 @@ static int exif_isobmff_parse_box(unsigned char *buf, isobmff_box_type *box) static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, isobmff_item_pos_type *pos) { isobmff_box_type box, item; - unsigned char *box_offset, *p, *p2; + unsigned char *box_offset, *p; int header_size, exif_id = -1, version, item_count, i; size_t remain; @@ -4367,10 +4367,10 @@ static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, iso ADVANCE(4); item_count = php_ifd_get32u(p - 4, 1); } - for (i = 0, p2 = p; i < item_count && p < end - 16; i++, p2 += 16) { - if (php_ifd_get16u(p2, 1) == exif_id) { - pos->offset = php_ifd_get32u(p2 + 8, 1); - pos->size = php_ifd_get32u(p2 + 12, 1); + for (i = 0; i < item_count && p < end - 16; i++, p += 16) { + if (php_ifd_get16u(p, 1) == exif_id) { + pos->offset = php_ifd_get32u(p + 8, 1); + pos->size = php_ifd_get32u(p + 12, 1); break; } } From 518c2a8c163a3d6277e92da0d1ee80130e5c8432 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 7 Sep 2025 02:12:01 +0200 Subject: [PATCH 04/10] exif/heic: Avoid overflow when adding box size and checking against file size We change the order of operations such that the file size check cannot overflow in the for loop. This prevents infinite loops. We also add an overflow check at the end of the loop body to prevent the addition of offset and box.size from overflowing. --- ext/exif/exif.c | 5 ++++- ext/exif/tests/heic_box_overflow.phpt | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 ext/exif/tests/heic_box_overflow.phpt diff --git a/ext/exif/exif.c b/ext/exif/exif.c index b9d20fb7f9add..3e044717b798b 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4393,7 +4393,7 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf bool ret = false; pos.size = 0; - for (offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize > offset + 16; offset += box.size) { + for (offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize - 16 > offset; offset += box.size) { if ((php_stream_seek(ImageInfo->infile, offset, SEEK_SET) < 0) || (exif_read_from_stream_file_looped(ImageInfo->infile, (char*)buf, 16) != 16)) { break; @@ -4433,6 +4433,9 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf efree(data); break; } + if (offset + box.size < offset) { + break; + } } return ret; diff --git a/ext/exif/tests/heic_box_overflow.phpt b/ext/exif/tests/heic_box_overflow.phpt new file mode 100644 index 0000000000000..b3a5e42df91fc --- /dev/null +++ b/ext/exif/tests/heic_box_overflow.phpt @@ -0,0 +1,27 @@ +--TEST-- +HEIC box overflow +--EXTENSIONS-- +exif +--FILE-- + +--CLEAN-- + +--EXPECTF-- +Warning: exif_read_data(heic_box_overflow): Invalid HEIF file in %s on line %d +bool(false) From 9c901145f75ff87bea1eb701761585093e73644d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 7 Sep 2025 10:56:10 +0200 Subject: [PATCH 05/10] exif/heic: Prevent overflow when computing meta bounds --- ext/exif/exif.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index 3e044717b798b..7a43975d69598 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4416,7 +4416,8 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf exif_isobmff_parse_meta(data, data + limit, &pos); } if ((pos.size) && - (ImageInfo->FileSize >= pos.offset + pos.size) && + (pos.size < ImageInfo->FileSize) && + (ImageInfo->FileSize - pos.size >= pos.offset) && (php_stream_seek(ImageInfo->infile, pos.offset + 2, SEEK_SET) >= 0)) { if (limit >= pos.size - 2) { limit = pos.size - 2; From eba4140bc49ffce6c49cfa3a34a093df54721042 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 7 Sep 2025 11:24:03 +0200 Subject: [PATCH 06/10] exif/heic: Ensure file is at least 16 bytes to prevent underflow --- ext/exif/exif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index 7a43975d69598..4063f07e9111a 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4507,7 +4507,7 @@ static bool exif_scan_FILE_header(image_info_type *ImageInfo) exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_WARNING, "Invalid TIFF file"); return false; } - } else if ((ImageInfo->FileSize > 12) && + } else if ((ImageInfo->FileSize > 16) && (!memcmp(file_header + 4, "ftyp", 4)) && (exif_read_from_stream_file_looped(ImageInfo->infile, (char*)(file_header + 8), 4) == 4) && ((!memcmp(file_header + 8, "heic", 4)) || (!memcmp(file_header + 8, "heix", 4)) || (!memcmp(file_header + 8, "mif1", 4)))) { From 7a6a763f579889b44a1ebe1020c0422db075bb94 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 13 Sep 2025 11:43:32 +0200 Subject: [PATCH 07/10] Fix OSS-Fuzz #442954659: Crash in exif_scan_HEIF_header --- ext/exif/exif.c | 10 ++++++++-- ext/exif/tests/oss_fuzz_444479893/input | Bin 0 -> 1023 bytes .../oss_fuzz_444479893/oss_fuzz_444479893.phpt | 10 ++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 ext/exif/tests/oss_fuzz_444479893/input create mode 100644 ext/exif/tests/oss_fuzz_444479893/oss_fuzz_444479893.phpt diff --git a/ext/exif/exif.c b/ext/exif/exif.c index 4063f07e9111a..b103ee0f39319 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4308,7 +4308,7 @@ static int exif_isobmff_parse_box(unsigned char *buf, isobmff_box_type *box) static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, isobmff_item_pos_type *pos) { isobmff_box_type box, item; - unsigned char *box_offset, *p; + unsigned char *p; int header_size, exif_id = -1, version, item_count, i; size_t remain; @@ -4323,7 +4323,8 @@ static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, iso p += (n); \ } while (0) - for (box_offset = data + 4; box_offset < end - 16; box_offset += box.size) { + unsigned char *box_offset = data + 4; + while (box_offset < end - 16) { header_size = exif_isobmff_parse_box(box_offset, &box); if (box.size < header_size) { return; @@ -4376,6 +4377,11 @@ static void exif_isobmff_parse_meta(unsigned char *data, unsigned char *end, iso } break; } + + if (end - 16 - box_offset <= box.size) { + break; + } + box_offset += box.size; } #undef ADVANCE diff --git a/ext/exif/tests/oss_fuzz_444479893/input b/ext/exif/tests/oss_fuzz_444479893/input new file mode 100644 index 0000000000000000000000000000000000000000..91e083c9c62717786415641477ec3fbd2f43b97e GIT binary patch literal 1023 zcmZQzU{FmfsVvCNOf$?#Wk15g!t#iP#pE}T-U9(^_v~R{U~tGyElC8@Ai((lKSRU+ z|No&1;MBoAS_~XOd1eML#_InU9axk>JVC~Ku!>WA_N)dn1QH184X6|_42S|CKVbt53^aG*8YFw+!HOgXW8!uqP{k~m zDlkpcDE>>g$U_bp6#t=daEA +--EXPECTF-- +Warning: exif_read_data(%s): Invalid HEIF file in %s on line %d From 6cc1a110453ba6cd1b95c1741e04f36b448835ba Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 13 Sep 2025 23:19:39 +0200 Subject: [PATCH 08/10] exif/heic: Make sure pos is always initialized --- ext/exif/exif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index b103ee0f39319..f201700eee5b9 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4398,7 +4398,6 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf int box_header_size, remain; bool ret = false; - pos.size = 0; for (offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize - 16 > offset; offset += box.size) { if ((php_stream_seek(ImageInfo->infile, offset, SEEK_SET) < 0) || (exif_read_from_stream_file_looped(ImageInfo->infile, (char*)buf, 16) != 16)) { @@ -4418,6 +4417,7 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf if (remain) { memcpy(data, buf + box_header_size, remain); } + memset(&pos, 0, sizeof(pos)); if (exif_read_from_stream_file_looped(ImageInfo->infile, (char*)(data + remain), limit - remain) == limit - remain) { exif_isobmff_parse_meta(data, data + limit, &pos); } From fb3d4becf51686708256a690ca6447ce570547c9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 13 Sep 2025 23:20:58 +0200 Subject: [PATCH 09/10] exif/heic: Make offset size_t --- ext/exif/exif.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/exif/exif.c b/ext/exif/exif.c index f201700eee5b9..d0ca7a6819bd9 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4393,12 +4393,11 @@ static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf isobmff_box_type box; isobmff_item_pos_type pos; unsigned char *data; - off_t offset; uint64_t limit; int box_header_size, remain; bool ret = false; - for (offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize - 16 > offset; offset += box.size) { + for (size_t offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize - 16 > offset; offset += box.size) { if ((php_stream_seek(ImageInfo->infile, offset, SEEK_SET) < 0) || (exif_read_from_stream_file_looped(ImageInfo->infile, (char*)buf, 16) != 16)) { break; From 02c13b67bcf1eb122543ca4ba3374994120ccd8e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 14 Sep 2025 17:52:46 +0200 Subject: [PATCH 10/10] Update NEWS for exif changes --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 7f52099e2e410..e3bd1ef9c2c41 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS - Exif: . Fix OSS-Fuzz #442954659 (zero-size box in HEIF file causes infinite loop). (nielsdos) + . Fix OSS-Fuzz #442954659 (Crash in exif_scan_HEIF_header). (nielsdos) + . Various hardening fixes to HEIF parsing. (nielsdos) - Opcache: . Fixed bug GH-19669 (assertion failure in zend_jit_trace_type_to_info_ex).