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). 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 32eaaf8c2b606..ee99f97f35455 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -2333,10 +2333,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/exif/exif.c b/ext/exif/exif.c index a0744e40f6891..d0ca7a6819bd9 100644 --- a/ext/exif/exif.c +++ b/ext/exif/exif.c @@ -4308,82 +4308,84 @@ 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 *p; 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) + + 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; } + 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) { - 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; } } break; } + + if (end - 16 - box_offset <= box.size) { + break; + } + box_offset += box.size; } + +#undef ADVANCE +#undef CHECK } static bool exif_scan_HEIF_header(image_info_type *ImageInfo, unsigned char *buf) @@ -4391,13 +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; - pos.size = 0; - for (offset = php_ifd_get32u(buf, 1); ImageInfo->FileSize > offset + 16; 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; @@ -4416,11 +4416,13 @@ 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); } 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; @@ -4437,6 +4439,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; @@ -4507,7 +4512,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)))) { 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) diff --git a/ext/exif/tests/oss_fuzz_444479893/input b/ext/exif/tests/oss_fuzz_444479893/input new file mode 100644 index 0000000000000..91e083c9c6271 Binary files /dev/null and b/ext/exif/tests/oss_fuzz_444479893/input differ diff --git a/ext/exif/tests/oss_fuzz_444479893/oss_fuzz_444479893.phpt b/ext/exif/tests/oss_fuzz_444479893/oss_fuzz_444479893.phpt new file mode 100644 index 0000000000000..b03635400f0a6 --- /dev/null +++ b/ext/exif/tests/oss_fuzz_444479893/oss_fuzz_444479893.phpt @@ -0,0 +1,10 @@ +--TEST-- +OSS-Fuzz #442954659 (Crash in exif_scan_HEIF_header) +--EXTENSIONS-- +exif +--FILE-- + +--EXPECTF-- +Warning: exif_read_data(%s): Invalid HEIF file in %s on line %d diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index db1d002b66979..7abaee78666e7 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -1528,10 +1528,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); } }