From 74a7d0084629d6f2e092ce5823af03d868aba19a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 21 Jun 2019 17:08:24 +0200 Subject: [PATCH 1/9] run-tests: Don't die unnecessarily die/exit leak memory, don't use them if we don't need to. --- run-tests.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/run-tests.php b/run-tests.php index 9d20304c25f..c684a065760 100755 --- a/run-tests.php +++ b/run-tests.php @@ -63,7 +63,7 @@ function main() if (getenv("TEST_PHP_WORKER")) { $workerID = intval(getenv("TEST_PHP_WORKER")); run_worker(); - die; + return; } define('INIT_DIR', getcwd()); @@ -733,7 +733,7 @@ HELP; exit(1); } - exit(0); + return; } } @@ -1731,11 +1731,9 @@ function run_worker() { "type" => "error", "msg" => "Unrecognised message type: $command[type]" ]); - die; + break 2; } } - - die; } // From c7962207d7b4c1c0449c24338538a474c6f9dc13 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 17:35:22 +0200 Subject: [PATCH 2/9] Fix stream leak in phar cache_list --- Zend/zend_list.c | 2 +- Zend/zend_list.h | 2 +- ext/phar/phar.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Zend/zend_list.c b/Zend/zend_list.c index 293eeadd04a..21d013a5895 100644 --- a/Zend/zend_list.c +++ b/Zend/zend_list.c @@ -206,7 +206,7 @@ void plist_entry_destructor(zval *zv) free(res); } -int zend_init_rsrc_list(void) +ZEND_API int zend_init_rsrc_list(void) { zend_hash_init(&EG(regular_list), 8, NULL, list_entry_destructor, 0); return SUCCESS; diff --git a/Zend/zend_list.h b/Zend/zend_list.h index 0bec20a280e..b9a1d5e159a 100644 --- a/Zend/zend_list.h +++ b/Zend/zend_list.h @@ -45,7 +45,7 @@ void list_entry_destructor(zval *ptr); void plist_entry_destructor(zval *ptr); void zend_clean_module_rsrc_dtors(int module_number); -int zend_init_rsrc_list(void); +ZEND_API int zend_init_rsrc_list(void); /* Exported for phar hack */ int zend_init_rsrc_plist(void); void zend_close_rsrc_list(HashTable *ht); void zend_destroy_rsrc_list(HashTable *ht); diff --git a/ext/phar/phar.c b/ext/phar/phar.c index 109ec24afac..cc949a6b617 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -113,7 +113,7 @@ static void phar_split_cache_list(void) /* {{{ */ /* fake request startup */ PHAR_G(request_init) = 1; - zend_hash_init(&EG(regular_list), 0, NULL, NULL, 0); + zend_init_rsrc_list(); EG(regular_list).nNextFreeElement=1; /* we don't want resource id 0 */ PHAR_G(has_bz2) = zend_hash_str_exists(&module_registry, "bz2", sizeof("bz2")-1); From 77f7ec51521531bc84b4d9dc9bb6767c9f2aeb39 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 11:51:40 +0200 Subject: [PATCH 3/9] Fix TimeZone leak in intl MessageFormat I'm just giving each format a distinct owned object here ... sharing it looks complicated. --- ext/intl/msgformat/msgformat_helpers.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ext/intl/msgformat/msgformat_helpers.cpp b/ext/intl/msgformat/msgformat_helpers.cpp index dcc74283b22..ed75ae8bf68 100644 --- a/ext/intl/msgformat/msgformat_helpers.cpp +++ b/ext/intl/msgformat/msgformat_helpers.cpp @@ -343,20 +343,24 @@ static void umsg_set_timezone(MessageFormatter_object *mfo, } if (used_tz == NULL) { - zval nullzv, *zvptr = &nullzv; - ZVAL_NULL(zvptr); - used_tz = timezone_process_timezone_argument(zvptr, &err, "msgfmt_format"); + zval nullzv; + ZVAL_NULL(&nullzv); + used_tz = timezone_process_timezone_argument(&nullzv, &err, "msgfmt_format"); if (used_tz == NULL) { continue; } } - df->setTimeZone(*used_tz); + df->adoptTimeZone(used_tz->clone()); } if (U_SUCCESS(err.code)) { mfo->mf_data.tz_set = 1; } + + if (used_tz) { + delete used_tz; + } } U_CFUNC void umsg_format_helper(MessageFormatter_object *mfo, From b53bb3c15b1414e2f695988886b213dbe87bd223 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 12:02:01 +0200 Subject: [PATCH 4/9] Fix UConverter leak --- ext/intl/converter/converter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/intl/converter/converter.c b/ext/intl/converter/converter.c index 7b4109749e1..8c9377ecae8 100644 --- a/ext/intl/converter/converter.c +++ b/ext/intl/converter/converter.c @@ -828,7 +828,7 @@ static PHP_METHOD(UConverter, transcode) { if (U_SUCCESS(error) && (ret = php_converter_do_convert(dest_cnv, src_cnv, str, str_len, NULL)) != NULL) { - RETURN_NEW_STR(ret); + RETVAL_NEW_STR(ret); } if (U_FAILURE(error)) { From 6fcae63f614d1ed4aaeaff7b13a7a4627b1f1312 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 12:09:47 +0200 Subject: [PATCH 5/9] Fix SSL_CTX leak in ftp extension SSL_CTX is a refcounted structure, which will be held by the SSL handle, so we can free it here. --- ext/ftp/ftp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index b1343976b66..e5355b1bd4f 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -287,9 +287,10 @@ ftp_login(ftpbuf_t *ftp, const char *user, const size_t user_len, const char *pa SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH); ftp->ssl_handle = SSL_new(ctx); + SSL_CTX_free(ctx); + if (ftp->ssl_handle == NULL) { php_error_docref(NULL, E_WARNING, "failed to create the SSL handle"); - SSL_CTX_free(ctx); return 0; } From 8277acefbd62a5a4846a10d1d9b4f7daa36ee363 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 12:46:58 +0200 Subject: [PATCH 6/9] Fix leak on sqlite3 open error sqlite3_open creates the database object even if the operation fails. --- ext/sqlite3/sqlite3.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c index 61b032e900d..e3474e4acba 100644 --- a/ext/sqlite3/sqlite3.c +++ b/ext/sqlite3/sqlite3.c @@ -136,6 +136,7 @@ PHP_METHOD(sqlite3, open) rc = sqlite3_open_v2(fullpath, &(db_obj->db), flags, NULL); if (rc != SQLITE_OK) { + sqlite3_close(db_obj->db); zend_throw_exception_ex(zend_ce_exception, 0, "Unable to open database: %s", #ifdef HAVE_SQLITE3_ERRSTR db_obj->db ? sqlite3_errmsg(db_obj->db) : sqlite3_errstr(rc)); @@ -151,6 +152,7 @@ PHP_METHOD(sqlite3, open) #if SQLITE_HAS_CODEC if (encryption_key_len > 0) { if (sqlite3_key(db_obj->db, encryption_key, encryption_key_len) != SQLITE_OK) { + sqlite3_close(db_obj->db); zend_throw_exception_ex(zend_ce_exception, 0, "Unable to open database: %s", sqlite3_errmsg(db_obj->db)); return; } From 8757f30cc7f857f9b619b2f8b2f2731507653846 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 13:17:39 +0200 Subject: [PATCH 7/9] Fix CURLINFO_COOKIELIST leak --- ext/curl/interface.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 5db1be99e8b..bc3bf0db49e 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -3432,11 +3432,12 @@ PHP_FUNCTION(curl_getinfo) case CURLINFO_SLIST: { struct curl_slist *slist; - array_init(return_value); if (curl_easy_getinfo(ch->cp, option, &slist) == CURLE_OK) { - while (slist) { - add_next_index_string(return_value, slist->data); - slist = slist->next; + struct curl_slist *current = slist; + array_init(return_value); + while (current) { + add_next_index_string(return_value, current->data); + current = current->next; } curl_slist_free_all(slist); } else { From 42b22d3a9418e948fddf896dbe37536c6cc12f43 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 13:08:28 +0200 Subject: [PATCH 8/9] Fix out of bounds write in phpdbg It seems that this code has a peculiar interpretation of "len", where it actually points to the last character, not one past it. So we need +1 here for that extra char and another +1 for the terminating null byte. --- sapi/phpdbg/phpdbg_prompt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index 248391b1883..f2f20ae75b0 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -838,7 +838,7 @@ PHPDBG_COMMAND(run) /* {{{ */ while (*p == ' ') p++; while (*p) { char sep = ' '; - char *buf = emalloc(end - p + 1), *q = buf; + char *buf = emalloc(end - p + 2), *q = buf; if (*p == '<') { /* use as STDIN */ From 0f3ca15bb787fd977a5875c456c2133e86bdd143 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 11:32:18 +0200 Subject: [PATCH 9/9] FFI: Perform bitfield operations byte-wise Otherwise we may perform reads/writes outside the allocation, as already happens in 032.phpt. --- ext/ffi/ffi.c | 116 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 35 deletions(-) diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index 20a215a42aa..c058cb9721b 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -531,32 +531,60 @@ again: } /* }}} */ +static uint64_t zend_ffi_bit_field_read(void *ptr, zend_ffi_field *field) /* {{{ */ +{ + size_t bit = field->first_bit; + size_t last_bit = bit + field->bits - 1; + uint8_t *p = (uint8_t *) ptr + bit / 8; + uint8_t *last_p = (uint8_t *) ptr + last_bit / 8; + size_t pos = bit % 8; + size_t insert_pos = 0; + uint8_t mask; + uint64_t val = 0; + + /* Bitfield fits into a single byte */ + if (p == last_p) { + mask = (1U << field->bits) - 1U; + return (*p >> pos) & mask; + } + + /* Read partial prefix byte */ + if (pos != 0) { + size_t num_bits = 8 - pos; + mask = ((1U << num_bits) - 1U) << pos; + val = (*p++ >> pos) & mask; + insert_pos += num_bits; + } + + /* Read full bytes */ + while (p < last_p) { + val |= *p++ << insert_pos; + insert_pos += 8; + } + + /* Read partial suffix byte */ + if (p == last_p) { + size_t num_bits = last_bit % 8 + 1; + mask = (1U << num_bits) - 1U; + val |= (*p & mask) << insert_pos; + } + + return val; +} + static void zend_ffi_bit_field_to_zval(void *ptr, zend_ffi_field *field, zval *rv) /* {{{ */ { - uint64_t *p1 = (uint64_t *)((char*)ptr + (field->first_bit / 64) * 8); - uint64_t *p2 = (uint64_t *)((char*)ptr + ((field->first_bit + field->bits - 1) / 64) * 8); - uint64_t pos = field->first_bit % 64; - uint64_t shift = 64 - (field->bits % 64); - uint64_t val; - - if (p1 == p2) { - if (field->bits == 64) { - val = *p1; - shift = 0; - } else { - val = *p1 << (shift - pos); - } - } else { - val = (*p1 >> pos) | (*p2 << (64 - pos)); - } + uint64_t val = zend_ffi_bit_field_read(ptr, field); if (ZEND_FFI_TYPE(field->type)->kind == ZEND_FFI_TYPE_CHAR || ZEND_FFI_TYPE(field->type)->kind == ZEND_FFI_TYPE_SINT8 || ZEND_FFI_TYPE(field->type)->kind == ZEND_FFI_TYPE_SINT16 || ZEND_FFI_TYPE(field->type)->kind == ZEND_FFI_TYPE_SINT32 || ZEND_FFI_TYPE(field->type)->kind == ZEND_FFI_TYPE_SINT64) { - val = (int64_t)val >> shift; - } else { - val = val >> shift; + /* Sign extend */ + uint64_t shift = 64 - (field->bits % 64); + if (shift != 0) { + val = (int64_t)(val << shift) >> shift; + } } ZVAL_LONG(rv, val); } @@ -564,25 +592,43 @@ static void zend_ffi_bit_field_to_zval(void *ptr, zend_ffi_field *field, zval *r static int zend_ffi_zval_to_bit_field(void *ptr, zend_ffi_field *field, zval *value) /* {{{ */ { - uint64_t *p1 = (uint64_t *)((char*)ptr + (field->first_bit / 64) * 8); - uint64_t *p2 = (uint64_t *)((char*)ptr + ((field->first_bit + field->bits - 1) / (8 * 8)) * 8); - uint64_t pos = field->first_bit % 64; - uint64_t mask; uint64_t val = zval_get_long(value); + size_t bit = field->first_bit; + size_t last_bit = bit + field->bits - 1; + uint8_t *p = (uint8_t *) ptr + bit / 8; + uint8_t *last_p = (uint8_t *) ptr + last_bit / 8; + size_t pos = bit % 8; + uint8_t mask; - if (p1 == p2) { - if (field->bits == 64) { - *p1 = val; - } else { - mask = ((1ULL << field->bits) - 1ULL) << pos; - *p1 = (*p1 & ~mask) | ((val << pos) & mask); - } - } else { - mask = ((1ULL << (64 - pos)) - 1ULL) << pos; - *p1 = (*p1 & ~mask) | ((val << pos) & mask); - mask = (1ULL << pos) - 1ULL; - *p2 = (*p2 & ~mask) | ((val >> (64 - pos)) & mask); + /* Bitfield fits into a single byte */ + if (p == last_p) { + mask = ((1U << field->bits) - 1U) << pos; + *p = (*p & ~mask) | ((val << pos) & mask); + return SUCCESS; } + + /* Write partial prefix byte */ + if (pos != 0) { + size_t num_bits = 8 - pos; + mask = ((1U << num_bits) - 1U) << pos; + *p = (*p & ~mask) | ((val << pos) & mask); + p++; + val >>= num_bits; + } + + /* Write full bytes */ + while (p < last_p) { + *p++ = val; + val >>= 8; + } + + /* Write partial suffix byte */ + if (p == last_p) { + size_t num_bits = last_bit % 8 + 1; + mask = (1U << num_bits) - 1U; + *p = (*p & ~mask) | (val & mask); + } + return SUCCESS; } /* }}} */