From 716274f09076e75ebfcc6660a946ef68990b03bb Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 12 Jul 2021 13:19:39 +0200 Subject: [PATCH 1/4] Fix #81243: Too much memory is allocated for preg_replace() If `new_len > alloc_len`, we had a serious issue anyway. The comparison only makes some sense if `new_len == alloc_len`, but in that case we don't need to *re*-allocate. Only if `new_len == 0` we need to allocate an empty zend_string. However, trimming a potentially over- allocated string appears to be reasonable, so we drop the condition altogether. --- ext/pcre/php_pcre.c | 14 ++++++-------- ext/pcre/tests/bug81243.phpt | 13 +++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 ext/pcre/tests/bug81243.phpt diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 64da43a05f8e7..68b8c47e9344f 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -1805,14 +1805,12 @@ PHPAPI zend_string *php_pcre_replace_impl(pcre_cache_entry *pce, zend_string *su result = zend_string_copy(subject_str); break; } - new_len = result_len + subject_len - last_end_offset; - if (new_len >= alloc_len) { - alloc_len = new_len; /* now we know exactly how long it is */ - if (NULL != result) { - result = zend_string_realloc(result, alloc_len, 0); - } else { - result = zend_string_alloc(alloc_len, 0); - } + /* now we know exactly how long it is */ + alloc_len = result_len + subject_len - last_end_offset; + if (NULL != result) { + result = zend_string_realloc(result, alloc_len, 0); + } else { + result = zend_string_alloc(alloc_len, 0); } /* stick that last bit of string on our output */ memcpy(ZSTR_VAL(result) + result_len, piece, subject_len - last_end_offset); diff --git a/ext/pcre/tests/bug81243.phpt b/ext/pcre/tests/bug81243.phpt new file mode 100644 index 0000000000000..6c2f966326fa8 --- /dev/null +++ b/ext/pcre/tests/bug81243.phpt @@ -0,0 +1,13 @@ +--TEST-- + +--FILE-- + +--EXPECT-- +bool(true) From c16fe09f8789c82c3e74b1cf78436de75d9806ae Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 12 Jul 2021 13:36:25 +0200 Subject: [PATCH 2/4] Fix php_pcre_replace_func_impl(), too --- ext/pcre/php_pcre.c | 14 ++++++-------- ext/pcre/tests/bug81243.phpt | 8 ++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 68b8c47e9344f..81c30b6a2f47f 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -2014,14 +2014,12 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce, zend_strin result = zend_string_copy(subject_str); break; } - new_len = result_len + subject_len - last_end_offset; - if (new_len >= alloc_len) { - alloc_len = new_len; /* now we know exactly how long it is */ - if (NULL != result) { - result = zend_string_realloc(result, alloc_len, 0); - } else { - result = zend_string_alloc(alloc_len, 0); - } + /* now we know exactly how long it is */ + alloc_len = result_len + subject_len - last_end_offset; + if (NULL != result) { + result = zend_string_realloc(result, alloc_len, 0); + } else { + result = zend_string_alloc(alloc_len, 0); } /* stick that last bit of string on our output */ memcpy(ZSTR_VAL(result) + result_len, piece, subject_len - last_end_offset); diff --git a/ext/pcre/tests/bug81243.phpt b/ext/pcre/tests/bug81243.phpt index 6c2f966326fa8..d340459e3ae61 100644 --- a/ext/pcre/tests/bug81243.phpt +++ b/ext/pcre/tests/bug81243.phpt @@ -3,11 +3,19 @@ --FILE-- --EXPECT-- bool(true) +bool(true) From 5b3564e402e4ece7a29275d8e4503d9943ee6737 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 12 Jul 2021 16:06:37 +0200 Subject: [PATCH 3/4] Allocate twice the size needed --- ext/pcre/php_pcre.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c index 81c30b6a2f47f..19ea92713875e 100644 --- a/ext/pcre/php_pcre.c +++ b/ext/pcre/php_pcre.c @@ -1719,7 +1719,7 @@ PHPAPI zend_string *php_pcre_replace_impl(pcre_cache_entry *pce, zend_string *su } if (new_len >= alloc_len) { - alloc_len = zend_safe_address_guarded(2, new_len, alloc_len); + alloc_len = zend_safe_address_guarded(2, new_len, 0); if (result == NULL) { result = zend_string_alloc(alloc_len, 0); } else { @@ -1957,7 +1957,7 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce, zend_strin ZEND_ASSERT(eval_result); new_len = zend_safe_address_guarded(1, ZSTR_LEN(eval_result), new_len); if (new_len >= alloc_len) { - alloc_len = zend_safe_address_guarded(2, new_len, alloc_len); + alloc_len = zend_safe_address_guarded(2, new_len, 0); if (result == NULL) { result = zend_string_alloc(alloc_len, 0); } else { From a417796d79dcaecd5aa9c8bb86f3bddc1ae8ca58 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 12 Jul 2021 16:35:35 +0200 Subject: [PATCH 4/4] Fill out test name --- ext/pcre/tests/bug81243.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pcre/tests/bug81243.phpt b/ext/pcre/tests/bug81243.phpt index d340459e3ae61..a413f33f6eea5 100644 --- a/ext/pcre/tests/bug81243.phpt +++ b/ext/pcre/tests/bug81243.phpt @@ -1,5 +1,5 @@ --TEST-- - +Bug #81243 (Too much memory is allocated for preg_replace()) --FILE--