Reuse safe allocators for aligned strings

Apply ndossche's suggestion and check realloc alignment overflow.

php#23869 (comment)

Author: realFlowControl

Zend/zend_alloc.c

@@ -1825,6 +1825,10 @@ static zend_never_inline void *zend_mm_realloc_huge(zend_mm_heap *heap, void *pt
 #if ZEND_DEBUG
 	real_size = size;
 	size = ZEND_MM_ALIGNED_SIZE(size) + ZEND_MM_ALIGNED_SIZE(sizeof(zend_mm_debug_info));
+	if (UNEXPECTED(size < real_size)) {
+		zend_mm_delayed_error_or_panic(heap->fallible, "Possible integer overflow in memory allocation (%zu + %zu)", ZEND_MM_ALIGNED_SIZE(real_size), ZEND_MM_ALIGNED_SIZE(sizeof(zend_mm_debug_info)));
+		return NULL;
+	}
 #endif
 	if (size > ZEND_MM_MAX_LARGE_SIZE) {
 #if ZEND_DEBUG
@@ -1835,10 +1839,15 @@ static zend_never_inline void *zend_mm_realloc_huge(zend_mm_heap *heap, void *pt
 		 * We allocate them with 2MB size granularity, to avoid many
 		 * reallocations when they are extended by small pieces
 		 */
-		new_size = ZEND_MM_ALIGNED_SIZE_EX(size, MAX(REAL_PAGE_SIZE, ZEND_MM_CHUNK_SIZE));
+		size_t alignment = MAX(REAL_PAGE_SIZE, ZEND_MM_CHUNK_SIZE);
 #else
-		new_size = ZEND_MM_ALIGNED_SIZE_EX(size, REAL_PAGE_SIZE);
+		size_t alignment = REAL_PAGE_SIZE;
 #endif
+		new_size = ZEND_MM_ALIGNED_SIZE_EX(size, alignment);
+		if (UNEXPECTED(new_size < size)) {
+			zend_mm_delayed_error_or_panic(heap->fallible, "Possible integer overflow in memory allocation (%zu + %zu)", size, alignment);
+			return NULL;
+		}
 		if (new_size == old_size) {
 #if ZEND_DEBUG
 			zend_mm_change_huge_block_size(heap, ptr, new_size, real_size ZEND_FILE_LINE_RELAY_CC ZEND_FILE_LINE_ORIG_RELAY_CC);
@@ -1920,6 +1929,10 @@ static zend_always_inline void *zend_mm_realloc_heap(zend_mm_heap *heap, void *p
 		size_t real_size = size;
 
 		size = ZEND_MM_ALIGNED_SIZE(size) + ZEND_MM_ALIGNED_SIZE(sizeof(zend_mm_debug_info));
+		if (UNEXPECTED(size < real_size)) {
+			zend_mm_delayed_error_or_panic(heap->fallible, "Possible integer overflow in memory allocation (%zu + %zu)", ZEND_MM_ALIGNED_SIZE(real_size), ZEND_MM_ALIGNED_SIZE(sizeof(zend_mm_debug_info)));
+			return NULL;
+		}
 #endif
 
 		ZEND_MM_CHECK(chunk->heap == heap, "zend_mm_heap corrupted");

Zend/zend_string.h

@@ -131,24 +131,14 @@ static zend_always_inline zend_string *ZSTR_KNOWN(size_t idx) {
 #define ZSTR_MAX_OVERHEAD (ZEND_MM_ALIGNED_SIZE(_ZSTR_HEADER_SIZE + 1))
 #define ZSTR_MAX_LEN (SIZE_MAX - ZSTR_MAX_OVERHEAD)
 
-/* Returns n * m + l, or SIZE_MAX on fallible overflow. Rejects lengths above
- * ZSTR_MAX_LEN so the aligned allocation size of the string can not wrap. */
-static zend_always_inline size_t zend_string_safe_len(size_t n, size_t m, size_t l, bool fallible)
-{
-	bool overflow;
-	size_t len = zend_safe_address(n, m, l, &overflow);
-
-	if (UNEXPECTED(overflow || len > ZSTR_MAX_LEN)) {
-		zend_mm_delayed_error_or_panic(fallible, "Possible integer overflow in memory allocation (%zu * %zu + %zu)", n, m, l);
-		if (!fallible) {
-			/* zend_mm_delayed_error_or_panic() aborts for !fallible, but its body
-			 * is in zend_alloc.c and is not visible when compiling other source files.
-			 * Its declaration cannot use ZEND_NORETURN because the fallible path returns. */
-			ZEND_UNREACHABLE();
-		}
-		return SIZE_MAX;
+/* Offset for safe_*alloc(n, m, offset) to size the string like zend_string_alloc(n * m + l). */
+static zend_always_inline size_t zend_string_safe_offset(size_t n, size_t m, size_t l)
+{
+	if (UNEXPECTED(l > ZSTR_MAX_LEN)) {
+		return SIZE_MAX; /* Rejected by the allocator. */
 	}
-	return len;
+	/* Works even if n * m wraps, allocator detects that overflow. */
+	return ZEND_MM_ALIGNED_SIZE(_ZSTR_STRUCT_SIZE(n * m + l)) - n * m;
 }
 
 #define ZSTR_ALLOCA_ALLOC(str, _len, use_heap) do { \
@@ -229,11 +219,16 @@ static zend_always_inline zend_string *zend_string_alloc(size_t len, bool persis
 /* Fallible variant of zend_string_safe_alloc */
 static zend_always_inline zend_string *zend_string_safe_falloc(size_t n, size_t m, size_t l, bool persistent, bool fallible)
 {
-	size_t len = zend_string_safe_len(n, m, l, fallible && !persistent);
-	if (UNEXPECTED(len == SIZE_MAX)) {
-		return NULL;
+	zend_string *ret = (zend_string *)safe_fpemalloc(n, m, zend_string_safe_offset(n, m, l), persistent, fallible);
+	if (ZEND_CONST_COND(fallible, true) && UNEXPECTED(ret == NULL)) {
+		return ret;
 	}
-	return zend_string_falloc(len, persistent, fallible);
+
+	GC_SET_REFCOUNT(ret, 1);
+	GC_TYPE_INFO(ret) = GC_STRING | ((persistent ? IS_STR_PERSISTENT : 0) << GC_FLAGS_SHIFT);
+	ZSTR_H(ret) = 0;
+	ZSTR_LEN(ret) = (n * m) + l;
+	return ret;
 }
 
 static zend_always_inline zend_string *zend_string_safe_alloc(size_t n, size_t m, size_t l, bool persistent)
@@ -406,27 +401,23 @@ static zend_always_inline zend_string *zend_string_truncate(zend_string *s, size
 static zend_always_inline zend_string *zend_string_safe_frealloc(zend_string *s, size_t n, size_t m, size_t l, bool persistent, bool fallible)
 {
 	zend_string *ret;
-	size_t len = zend_string_safe_len(n, m, l, fallible && !persistent);
-	if (UNEXPECTED(len == SIZE_MAX)) {
-		return NULL;
-	}
 
 	if (!ZSTR_IS_INTERNED(s)) {
 		if (GC_REFCOUNT(s) == 1) {
-			ret = (zend_string *)fperealloc(s, ZEND_MM_ALIGNED_SIZE(_ZSTR_STRUCT_SIZE(len)), persistent, fallible);
+			ret = (zend_string *)safe_fperealloc(s, n, m, zend_string_safe_offset(n, m, l), persistent, fallible);
 			if (ZEND_CONST_COND(fallible, true) && !ret) {
 				return NULL;
 			}
-			ZSTR_LEN(ret) = len;
+			ZSTR_LEN(ret) = (n * m) + l;
 			zend_string_forget_hash_val(ret);
 			return ret;
 		}
 	}
-	ret = zend_string_falloc(len, persistent, fallible);
+	ret = zend_string_safe_falloc(n, m, l, persistent, fallible);
 	if (ZEND_CONST_COND(fallible, true) && !ret) {
 		return NULL;
 	}
-	memcpy(ZSTR_VAL(ret), ZSTR_VAL(s), MIN(len, ZSTR_LEN(s)) + 1);
+	memcpy(ZSTR_VAL(ret), ZSTR_VAL(s), MIN((n * m) + l, ZSTR_LEN(s)) + 1);
 	if (!ZSTR_IS_INTERNED(s)) {
 		GC_DELREF(s);
 	}

ext/mbstring/libmbfl/mbfl/mbfl_encoding.h

@@ -167,19 +167,16 @@ static inline zend_result mb_convert_buf_ensure(mb_convert_buf *buf, unsigned ch
 	ZEND_ASSERT(*out <= *limit);
 	if ((size_t)(*limit - *out) < needed) {
 		size_t oldsize = *limit - (unsigned char*)ZSTR_VAL((buf)->str);
-		size_t newsize = zend_string_safe_len(1, oldsize, MAX(oldsize >> 1, needed), /* fallible */ true);
-		if (UNEXPECTED(newsize == SIZE_MAX)) {
-			return FAILURE;
-		}
-		zend_string *newstr = ferealloc((buf)->str, ZEND_MM_ALIGNED_SIZE(_ZSTR_STRUCT_SIZE(newsize)), /* fallible */ true);
+		size_t growth = MAX(oldsize >> 1, needed);
+		zend_string *newstr = safe_ferealloc((buf)->str, 1, oldsize, zend_string_safe_offset(1, oldsize, growth), /* fallible */ true);
 		if (!newstr) {
 			/* An earlier growth may have moved the buffer. */
 			buf->out = *out;
 			buf->limit = *limit;
 			return FAILURE;
 		}
 		*out = (unsigned char*)ZSTR_VAL(newstr) + (*out - (unsigned char*)ZSTR_VAL((buf)->str));
-		*limit = (unsigned char*)ZSTR_VAL(newstr) + newsize;
+		*limit = (unsigned char*)ZSTR_VAL(newstr) + oldsize + growth;
 		(buf)->str = newstr;
 	}
 	return SUCCESS;

ext/standard/tests/strings/chunk_split_variation2_32bit.phpt

@@ -16,4 +16,4 @@ var_dump(chunk_split($a,$b,$c));
 --EXPECTF--
 *** Testing chunk_split() : unexpected large 'end' string argument variation 2 ***
 
-Fatal error: Possible integer overflow in memory allocation (65537 * 65537 + 65537) in %s on line %d
+Fatal error: Possible integer overflow in memory allocation (65537 * 65537 + %d) in %s on line %d