From 1b3b5c94e52d10eb7a3f69b486a51b3f4d214d4f Mon Sep 17 00:00:00 2001 From: Peter van Dommelen Date: Sun, 30 May 2021 19:30:24 +0200 Subject: [PATCH] Fixed bug #81070 When the memory limit is reduced using an `ini_set("memory_limit", ..)` below the currently allocated memory, the out-of-memory check overflowed. Instead of implementing additional checks during allocation, `zend_set_memory_limit()` now validates the new memory limit. When below the current memory usage the ini_set call will fail and throw a warning. This is part of GH-7040. --- NEWS | 2 ++ Zend/tests/bug81070.phpt | 9 +++++++++ Zend/zend_alloc.c | 8 +++++++- main/main.c | 12 +++++++++--- 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/bug81070.phpt diff --git a/NEWS b/NEWS index 5a46ee4664b..32256d2e93d 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ PHP NEWS . Fixed bug #76359 (open_basedir bypass through adding ".."). (cmb) . Fixed bug #81090 (Typed property performance degradation with .= operator). (Nikita) + . Fixed bug #81070 (Integer underflow in memory limit comparison). + (Peter van Dommelen) - OpenSSL: . Fixed bug #76694 (native Windows cert verification uses CN as sever name). diff --git a/Zend/tests/bug81070.phpt b/Zend/tests/bug81070.phpt new file mode 100644 index 00000000000..0dc45056e33 --- /dev/null +++ b/Zend/tests/bug81070.phpt @@ -0,0 +1,9 @@ +--TEST-- +Bug #81070 Setting memory limit to below current usage +--FILE-- + +--EXPECTF-- +Warning: Failed to set memory limit to 3145728 bytes (Current memory usage is %d bytes) in %s on line %d diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index f5fbee676db..3a33e5441c7 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -2660,7 +2660,13 @@ ZEND_API char* ZEND_FASTCALL zend_strndup(const char *s, size_t length) ZEND_API int zend_set_memory_limit(size_t memory_limit) { #if ZEND_MM_LIMIT - AG(mm_heap)->limit = (memory_limit >= ZEND_MM_CHUNK_SIZE) ? memory_limit : ZEND_MM_CHUNK_SIZE; + if (memory_limit < ZEND_MM_CHUNK_SIZE) { + memory_limit = ZEND_MM_CHUNK_SIZE; + } + if (UNEXPECTED(memory_limit < AG(mm_heap)->real_size)) { + return FAILURE; + } + AG(mm_heap)->limit = memory_limit; #endif return SUCCESS; } diff --git a/main/main.c b/main/main.c index b17bbd7fe7b..e675998575f 100644 --- a/main/main.c +++ b/main/main.c @@ -296,12 +296,18 @@ static PHP_INI_MH(OnSetSerializePrecision) */ static PHP_INI_MH(OnChangeMemoryLimit) { + size_t value; if (new_value) { - PG(memory_limit) = zend_atol(ZSTR_VAL(new_value), ZSTR_LEN(new_value)); + value = zend_atol(ZSTR_VAL(new_value), ZSTR_LEN(new_value)); } else { - PG(memory_limit) = Z_L(1)<<30; /* effectively, no limit */ + value = Z_L(1)<<30; /* effectively, no limit */ } - return zend_set_memory_limit(PG(memory_limit)); + if (zend_set_memory_limit(value) == FAILURE) { + zend_error(E_WARNING, "Failed to set memory limit to %zd bytes (Current memory usage is %zd bytes)", value, zend_memory_usage(true)); + return FAILURE; + } + PG(memory_limit) = value; + return SUCCESS; } /* }}} */