[pull] master from php:master

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).

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;

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;

ext/exif/exif.c

@@ -4308,96 +4308,96 @@ 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)
 {
 	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)))) {

ext/exif/tests/heic_box_overflow.phpt

@@ -0,0 +1,27 @@
+--TEST--
+HEIC box overflow
+--EXTENSIONS--
+exif
+--FILE--
+<?php
+$bytearray = [
+    0, 0, 0, 12, 'f', 't', 'y', 'p', 'h', 'e', 'i', 'c',
+    0, 0, 0, 1, 'x', 'y', 'z', 'w', 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff-11, 0, 0, 0, 0, 0, 0, 0
+];
+
+function convert($x) {
+    if (is_string($x)) return $x;
+    return chr($x);
+}
+
+file_put_contents(__DIR__."/heic_box_overflow", implode('', array_map(convert(...), $bytearray)));
+
+var_dump(exif_read_data(__DIR__."/heic_box_overflow"));
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__."/heic_box_overflow");
+?>
+--EXPECTF--
+Warning: exif_read_data(heic_box_overflow): Invalid HEIF file in %s on line %d
+bool(false)

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--
+<?php
+exif_read_data(__DIR__."/input");
+?>
+--EXPECTF--
+Warning: exif_read_data(%s): Invalid HEIF file in %s on line %d

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);
 					}
 				}