From 4c47ccc47927d5fd4ba4746246ee0304f8b3da83 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 17 Aug 2021 15:48:58 +0200 Subject: [PATCH 1/2] Fix #73122: Integer Overflow when concatenating strings We must avoid integer overflows in memory allocations, so we introduce an additional check in the VM, and bail out in the rare case of an overflow. Since the recent fix for bug #74960 still doesn't catch all possible overflows, we fix that right away. --- Zend/zend_operators.c | 2 +- Zend/zend_vm_def.h | 3 +++ Zend/zend_vm_execute.h | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 031894755c8eb..2d0adb1be2e14 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -1882,7 +1882,7 @@ ZEND_API int ZEND_FASTCALL concat_function(zval *result, zval *op1, zval *op2) / size_t result_len = op1_len + op2_len; zend_string *result_str; - if (UNEXPECTED(op1_len > ZSTR_MAX_LEN - op2_len)) { + if (UNEXPECTED(op1_len > ZSTR_MAX_LEN - op2_len || op2_len > ZSTR_MAX_LEN)) { zend_throw_error(NULL, "String size overflow"); zval_ptr_dtor_str(&op1_copy); zval_ptr_dtor_str(&op2_copy); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index b10eea480e919..c3e8327c865c0 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -416,6 +416,9 @@ ZEND_VM_HANDLER(8, ZEND_CONCAT, CONST|TMPVAR|CV, CONST|TMPVAR|CV, SPEC(NO_CONST_ !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 6e1edaba3f03e..b9220a646b057 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -7491,6 +7491,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CONST_TMPVAR_HANDL !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -9949,6 +9952,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CONST_CV_HANDLER(Z !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -13922,6 +13928,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_CONST_HANDL !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -15343,6 +15352,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_TMPVAR_HAND !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -17027,6 +17039,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_CV_HANDLER( !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -38332,6 +38347,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_CONST_HANDLER(Z !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -41828,6 +41846,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_TMPVAR_HANDLER( !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); @@ -46835,6 +46856,9 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_CV_HANDLER(ZEND !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); + if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); + } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); memcpy(ZSTR_VAL(str) + len, ZSTR_VAL(op2_str), ZSTR_LEN(op2_str)+1); ZVAL_NEW_STR(EX_VAR(opline->result.var), str); From 8cc7a73d326a84bda02bb87b8750624067500529 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 17 Aug 2021 16:56:52 +0200 Subject: [PATCH 2/2] Drop superfluous check --- Zend/zend_operators.c | 2 +- Zend/zend_vm_def.h | 2 +- Zend/zend_vm_execute.h | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 2d0adb1be2e14..031894755c8eb 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -1882,7 +1882,7 @@ ZEND_API int ZEND_FASTCALL concat_function(zval *result, zval *op1, zval *op2) / size_t result_len = op1_len + op2_len; zend_string *result_str; - if (UNEXPECTED(op1_len > ZSTR_MAX_LEN - op2_len || op2_len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(op1_len > ZSTR_MAX_LEN - op2_len)) { zend_throw_error(NULL, "String size overflow"); zval_ptr_dtor_str(&op1_copy); zval_ptr_dtor_str(&op2_copy); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index c3e8327c865c0..d362b01643214 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -416,7 +416,7 @@ ZEND_VM_HANDLER(8, ZEND_CONCAT, CONST|TMPVAR|CV, CONST|TMPVAR|CV, SPEC(NO_CONST_ !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index b9220a646b057..23798d6cf0cbc 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -7491,7 +7491,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CONST_TMPVAR_HANDL !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -9952,7 +9952,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CONST_CV_HANDLER(Z !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -13928,7 +13928,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_CONST_HANDL !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -15352,7 +15352,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_TMPVAR_HAND !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -17039,7 +17039,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_TMPVAR_CV_HANDLER( !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -38347,7 +38347,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_CONST_HANDLER(Z !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -41846,7 +41846,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_TMPVAR_HANDLER( !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0); @@ -46856,7 +46856,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_CONCAT_SPEC_CV_CV_HANDLER(ZEND !ZSTR_IS_INTERNED(op1_str) && GC_REFCOUNT(op1_str) == 1) { size_t len = ZSTR_LEN(op1_str); - if (UNEXPECTED(ZSTR_LEN(op2_str) > ZSTR_MAX_LEN - len || len > ZSTR_MAX_LEN)) { + if (UNEXPECTED(len > ZSTR_MAX_LEN - ZSTR_LEN(op2_str))) { zend_error_noreturn(E_ERROR, "Integer overflow in memory allocation"); } str = zend_string_extend(op1_str, len + ZSTR_LEN(op2_str), 0);