From 02c97fa6c15577e14be2b92c2dad2832f5942fd3 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 17 Mar 2023 19:35:58 +0100 Subject: [PATCH 1/2] Fix NUL byte in exception string terminating Exception::__toString() Fixes GH-10810 --- Zend/tests/gh10810.phpt | 10 ++++++++++ Zend/zend_exceptions.c | 22 +++++++++++----------- Zend/zend_string.h | 2 ++ main/snprintf.c | 9 +++++++++ main/spprintf.c | 9 +++++++++ 5 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 Zend/tests/gh10810.phpt diff --git a/Zend/tests/gh10810.phpt b/Zend/tests/gh10810.phpt new file mode 100644 index 0000000000000..2539d5c7b40d6 --- /dev/null +++ b/Zend/tests/gh10810.phpt @@ -0,0 +1,10 @@ +--TEST-- +GH-10810: Fix NUL byte terminating Exception::__toString() +--FILE-- + +--EXPECTF-- +Exception: Hello%0World in %s:%d +Stack trace: +#0 {main} diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index 7ccdaeea44311..b111be572dd27 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -680,24 +680,24 @@ ZEND_METHOD(Exception, __toString) } if ((Z_OBJCE_P(exception) == zend_ce_type_error || Z_OBJCE_P(exception) == zend_ce_argument_count_error) && strstr(ZSTR_VAL(message), ", called in ")) { - zend_string *real_message = zend_strpprintf(0, "%s and defined", ZSTR_VAL(message)); + zend_string *real_message = zend_strpprintf_unchecked(0, "%S and defined", message); zend_string_release_ex(message, 0); message = real_message; } + zend_string *tmp_trace = (Z_TYPE(trace) == IS_STRING && Z_STRLEN(trace)) + ? zend_string_copy(Z_STR(trace)) + : ZSTR_INIT_LITERAL("#0 {main}\n", false); if (ZSTR_LEN(message) > 0) { - str = zend_strpprintf(0, "%s: %s in %s:" ZEND_LONG_FMT - "\nStack trace:\n%s%s%s", - ZSTR_VAL(Z_OBJCE_P(exception)->name), ZSTR_VAL(message), ZSTR_VAL(file), line, - (Z_TYPE(trace) == IS_STRING && Z_STRLEN(trace)) ? Z_STRVAL(trace) : "#0 {main}\n", - ZSTR_LEN(prev_str) ? "\n\nNext " : "", ZSTR_VAL(prev_str)); + str = zend_strpprintf_unchecked(0, "%S: %S in %S:" ZEND_LONG_FMT "\nStack trace:\n%S%s%S", + Z_OBJCE_P(exception)->name, message, file, line, + tmp_trace, ZSTR_LEN(prev_str) ? "\n\nNext " : "", prev_str); } else { - str = zend_strpprintf(0, "%s in %s:" ZEND_LONG_FMT - "\nStack trace:\n%s%s%s", - ZSTR_VAL(Z_OBJCE_P(exception)->name), ZSTR_VAL(file), line, - (Z_TYPE(trace) == IS_STRING && Z_STRLEN(trace)) ? Z_STRVAL(trace) : "#0 {main}\n", - ZSTR_LEN(prev_str) ? "\n\nNext " : "", ZSTR_VAL(prev_str)); + str = zend_strpprintf_unchecked(0, "%S in %S:" ZEND_LONG_FMT "\nStack trace:\n%S%s%S", + Z_OBJCE_P(exception)->name, file, line, + tmp_trace, ZSTR_LEN(prev_str) ? "\n\nNext " : "", prev_str); } + zend_string_release_ex(tmp_trace, false); zend_string_release_ex(prev_str, 0); zend_string_release_ex(message, 0); diff --git a/Zend/zend_string.h b/Zend/zend_string.h index 472bf6bbfde63..32acba9f21ce0 100644 --- a/Zend/zend_string.h +++ b/Zend/zend_string.h @@ -108,6 +108,8 @@ END_EXTERN_C() #define ZSTR_ALLOCA_FREE(str, use_heap) free_alloca(str, use_heap) +#define ZSTR_INIT_LITERAL(s, persistent) (zend_string_init((s), strlen(s), (persistent))) + /*---*/ static zend_always_inline zend_ulong zend_string_hash_val(zend_string *s) diff --git a/main/snprintf.c b/main/snprintf.c index f082115c6764d..66662f907acf6 100644 --- a/main/snprintf.c +++ b/main/snprintf.c @@ -669,6 +669,15 @@ static size_t format_converter(buffy * odp, const char *fmt, va_list ap) /* {{{ } break; } + case 'S': { + zend_string *str = va_arg(ap, zend_string*); + s_len = ZSTR_LEN(str); + s = ZSTR_VAL(str); + if (adjust_precision && (size_t)precision < s_len) { + s_len = precision; + } + break; + } case 'u': switch(modifier) { default: diff --git a/main/spprintf.c b/main/spprintf.c index 4c01347fcf494..74f717630ed1d 100644 --- a/main/spprintf.c +++ b/main/spprintf.c @@ -376,6 +376,15 @@ static void xbuf_format_converter(void *xbuf, bool is_char, const char *fmt, va_ } break; } + case 'S': { + zend_string *str = va_arg(ap, zend_string*); + s_len = ZSTR_LEN(str); + s = ZSTR_VAL(str); + if (adjust_precision && (size_t)precision < s_len) { + s_len = precision; + } + break; + } case 'u': switch(modifier) { default: From 8072d04696ec2a8a0fca51ca25ef020e7ac31f5c Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Sat, 18 Mar 2023 12:31:04 +0100 Subject: [PATCH 2/2] Use %Z printf modifier instead of introducing new one --- Zend/zend_exceptions.c | 26 +++++++++++++++++++------- main/snprintf.c | 9 --------- main/spprintf.c | 9 --------- 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index b111be572dd27..89d75627e3f23 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -680,7 +680,9 @@ ZEND_METHOD(Exception, __toString) } if ((Z_OBJCE_P(exception) == zend_ce_type_error || Z_OBJCE_P(exception) == zend_ce_argument_count_error) && strstr(ZSTR_VAL(message), ", called in ")) { - zend_string *real_message = zend_strpprintf_unchecked(0, "%S and defined", message); + zval message_zv; + ZVAL_STR(&message_zv, message); + zend_string *real_message = zend_strpprintf_unchecked(0, "%Z and defined", &message_zv); zend_string_release_ex(message, 0); message = real_message; } @@ -688,14 +690,24 @@ ZEND_METHOD(Exception, __toString) zend_string *tmp_trace = (Z_TYPE(trace) == IS_STRING && Z_STRLEN(trace)) ? zend_string_copy(Z_STR(trace)) : ZSTR_INIT_LITERAL("#0 {main}\n", false); + + zval name_zv, trace_zv, file_zv, prev_str_zv; + ZVAL_STR(&name_zv, Z_OBJCE_P(exception)->name); + ZVAL_STR(&trace_zv, tmp_trace); + ZVAL_STR(&file_zv, file); + ZVAL_STR(&prev_str_zv, prev_str); + if (ZSTR_LEN(message) > 0) { - str = zend_strpprintf_unchecked(0, "%S: %S in %S:" ZEND_LONG_FMT "\nStack trace:\n%S%s%S", - Z_OBJCE_P(exception)->name, message, file, line, - tmp_trace, ZSTR_LEN(prev_str) ? "\n\nNext " : "", prev_str); + zval message_zv; + ZVAL_STR(&message_zv, message); + + str = zend_strpprintf_unchecked(0, "%Z: %Z in %Z:" ZEND_LONG_FMT "\nStack trace:\n%Z%s%Z", + &name_zv, &message_zv, &file_zv, line, + &trace_zv, ZSTR_LEN(prev_str) ? "\n\nNext " : "", &prev_str_zv); } else { - str = zend_strpprintf_unchecked(0, "%S in %S:" ZEND_LONG_FMT "\nStack trace:\n%S%s%S", - Z_OBJCE_P(exception)->name, file, line, - tmp_trace, ZSTR_LEN(prev_str) ? "\n\nNext " : "", prev_str); + str = zend_strpprintf_unchecked(0, "%Z in %Z:" ZEND_LONG_FMT "\nStack trace:\n%Z%s%Z", + &name_zv, &file_zv, line, + &trace_zv, ZSTR_LEN(prev_str) ? "\n\nNext " : "", &prev_str_zv); } zend_string_release_ex(tmp_trace, false); diff --git a/main/snprintf.c b/main/snprintf.c index 66662f907acf6..f082115c6764d 100644 --- a/main/snprintf.c +++ b/main/snprintf.c @@ -669,15 +669,6 @@ static size_t format_converter(buffy * odp, const char *fmt, va_list ap) /* {{{ } break; } - case 'S': { - zend_string *str = va_arg(ap, zend_string*); - s_len = ZSTR_LEN(str); - s = ZSTR_VAL(str); - if (adjust_precision && (size_t)precision < s_len) { - s_len = precision; - } - break; - } case 'u': switch(modifier) { default: diff --git a/main/spprintf.c b/main/spprintf.c index 74f717630ed1d..4c01347fcf494 100644 --- a/main/spprintf.c +++ b/main/spprintf.c @@ -376,15 +376,6 @@ static void xbuf_format_converter(void *xbuf, bool is_char, const char *fmt, va_ } break; } - case 'S': { - zend_string *str = va_arg(ap, zend_string*); - s_len = ZSTR_LEN(str); - s = ZSTR_VAL(str); - if (adjust_precision && (size_t)precision < s_len) { - s_len = precision; - } - break; - } case 'u': switch(modifier) { default: