From 8b679cd57608db4b703aece5b69f500a138656e7 Mon Sep 17 00:00:00 2001 From: Drew Frezell Date: Mon, 18 Mar 2024 20:17:34 -0400 Subject: [PATCH 1/2] Fix memory leak with passing in TLS context config from php application --- php_memcached.c | 127 +++++++++++++++++++++++++++--------------------- 1 file changed, 72 insertions(+), 55 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index 73b25f9..e375cdc 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -130,8 +130,7 @@ static int php_memc_list_entry(void) { Helper macros ****************************************/ #define RETURN_FROM_GET RETURN_FALSE -#define COPY_STR(value) strdup((char *)value) -#define COPY_PTR_INT(value) *((int *)value) + /**************************************** Structures and definitions ****************************************/ @@ -3023,49 +3022,90 @@ static PHP_METHOD(Memcached, getOption) /* }}} */ #ifdef HAVE_MEMCACHED_TLS -static zend_bool php_set_tls_context_configurations(memcached_ssl_context_config *config, const char *key, void *value){ +static char * zval_dup_string(zval *zvalue) { + char *dup = NULL; + zend_string *tmp_str; + zend_string *zstr = zval_try_get_tmp_string(zvalue, &tmp_str); + if (ZSTR_LEN(zstr) > 0) { + dup = estrdup(ZSTR_VAL(zstr)); + } + zend_tmp_string_release(tmp_str); + return dup; +} + +static zend_bool php_set_tls_context_configurations(memcached_ssl_context_config *config, zend_string *zkey, zval *zvalue){ zend_bool ok = 1; - if (!strcmp(key, "cert_file")) { - config->cert_file = value; + if (zend_string_equals_literal(zkey, "cert_file")) { + config->cert_file = zval_dup_string(zvalue); } - else if(!strcmp(key, "key_file")){ - config->key_file = value; + else if(zend_string_equals_literal(zkey, "key_file")){ + config->key_file = zval_dup_string(zvalue); } - else if(!strcmp(key, "key_file_pass")){ - config->key_file_pass = value; + else if(zend_string_equals_literal(zkey, "key_file_pass")){ + config->key_file_pass = zval_dup_string(zvalue); } - else if(!strcmp(key, "ca_cert_file")){ - config->ca_cert_file = value; + else if(zend_string_equals_literal(zkey, "ca_cert_file")){ + config->ca_cert_file = zval_dup_string(zvalue); } - else if(!strcmp(key, "ca_cert_dir")){ - config->ca_cert_dir = value; + else if(zend_string_equals_literal(zkey, "ca_cert_dir")){ + config->ca_cert_dir = zval_dup_string(zvalue); } - else if(!strcmp(key, "protocol")){ - config->protocol = value; + else if(zend_string_equals_literal(zkey, "protocol")){ + config->protocol = zval_dup_string(zvalue); } - else if(!strcmp(key, "hostname")){ - config->hostname = value; + else if(zend_string_equals_literal(zkey, "hostname")){ + config->hostname = zval_dup_string(zvalue); } - else if(!strcmp(key, "ciphers")){ - config->ciphers = value; + else if(zend_string_equals_literal(zkey, "ciphers")){ + config->ciphers = zval_dup_string(zvalue); } - else if(!strcmp(key, "ciphersuites")){ - config->ciphersuites = value; + else if(zend_string_equals_literal(zkey, "ciphersuites")){ + config->ciphersuites = zval_dup_string(zvalue); } - else if(!strcmp(key, "prefer_server_ciphers")){ - config->prefer_server_ciphers = value; + else if(zend_string_equals_literal(zkey, "prefer_server_ciphers")){ + config->prefer_server_ciphers = zend_is_true(zvalue); } - else if(!strcmp(key, "skip_cert_verify")){ - config->skip_cert_verify = value; + else if(zend_string_equals_literal(zkey, "skip_cert_verify")){ + config->skip_cert_verify = zend_is_true(zvalue); } - else if(!strcmp(key, "skip_hostname_verify")){ - config->skip_hostname_verify = value; + else if(zend_string_equals_literal(zkey, "skip_hostname_verify")){ + config->skip_hostname_verify = zend_is_true(zvalue); } else { - php_error_docref(NULL, E_WARNING, "Got invalid TLS context configuration key: %s", key); + php_error_docref(NULL, E_WARNING, "Got invalid TLS context configuration key: %s", ZSTR_VAL(zkey)); ok = 0; } return ok; } + +static void php_free_tls_context_configurations(memcached_ssl_context_config *config) { + if (config->cert_file) { + efree(config->cert_file); + } + if (config->key_file) { + efree(config->key_file); + } + if (config->key_file_pass) { + efree(config->key_file_pass); + } + if (config->ca_cert_file) { + efree(config->ca_cert_file); + } + if (config->ca_cert_dir) { + efree(config->ca_cert_dir); + } + if (config->protocol) { + efree(config->protocol); + } + if (config->hostname) { + efree(config->hostname); + } + if (config->ciphers) { + efree(config->ciphers); + } + if (config->ciphersuites) { + efree(config->ciphersuites); + } +} #endif static @@ -3422,7 +3462,6 @@ static PHP_METHOD(Memcached, createAndSetTLSContext) zval *context_config; zend_string *zkey; zval *zvalue; - char *key; memcached_ssl_context_config config = {}; memcached_return rc; memc_ssl_context_error error; @@ -3442,32 +3481,9 @@ static PHP_METHOD(Memcached, createAndSetTLSContext) php_error_docref(NULL, E_WARNING, "invalid TLS context configuration. Configuration keys must be strings"); RETURN_FALSE; } else { - void *value; - zend_string *str = NULL; - key = ZSTR_VAL(zkey); - switch (Z_TYPE_P(zvalue)) { - case IS_TRUE: - *(int *)value = 1; - break; - case IS_FALSE: - *(int *)value = 0; - break; - case IS_LONG: - *(int *)value = *(int *)zvalue; - break; - case IS_STRING: - str = zval_get_string(zvalue); - if (ZSTR_LEN(str) == 0) { - value = NULL; - } else { - value = ZSTR_VAL(str); - } - break; - default: - php_error_docref(NULL, E_ERROR, "Got invalid TLS context configuration value for key: %s", key); - RETURN_FALSE; - } - if (!php_set_tls_context_configurations(&config, key, value)) { + if (!php_set_tls_context_configurations(&config, zkey, zvalue)) { + // Error in setting config, free context and return + php_free_tls_context_configurations(&config); RETURN_FALSE; } } @@ -3475,6 +3491,7 @@ static PHP_METHOD(Memcached, createAndSetTLSContext) // Create and set SSL context to the memcached client error = memcached_create_and_set_ssl_context(intern->memc, &config); + php_free_tls_context_configurations(&config); if (error != MEMCACHED_SSL_CTX_SUCCESS) { php_error_docref(NULL, E_ERROR, "Failed to create/set TLS context: %s", memcached_ssl_context_get_error(error)); RETURN_FALSE; From 472331c0a1790218e7edbc6e9b2bf6dbe3450e5b Mon Sep 17 00:00:00 2001 From: Drew Frezell Date: Tue, 19 Mar 2024 23:05:17 -0400 Subject: [PATCH 2/2] Fix indentation to match style in file --- php_memcached.c | 140 ++++++++++++++++++++++++------------------------ 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index e375cdc..453330f 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -3034,77 +3034,77 @@ static char * zval_dup_string(zval *zvalue) { } static zend_bool php_set_tls_context_configurations(memcached_ssl_context_config *config, zend_string *zkey, zval *zvalue){ - zend_bool ok = 1; - if (zend_string_equals_literal(zkey, "cert_file")) { - config->cert_file = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "key_file")){ - config->key_file = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "key_file_pass")){ - config->key_file_pass = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "ca_cert_file")){ - config->ca_cert_file = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "ca_cert_dir")){ - config->ca_cert_dir = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "protocol")){ - config->protocol = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "hostname")){ - config->hostname = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "ciphers")){ - config->ciphers = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "ciphersuites")){ - config->ciphersuites = zval_dup_string(zvalue); - } - else if(zend_string_equals_literal(zkey, "prefer_server_ciphers")){ - config->prefer_server_ciphers = zend_is_true(zvalue); - } - else if(zend_string_equals_literal(zkey, "skip_cert_verify")){ - config->skip_cert_verify = zend_is_true(zvalue); - } - else if(zend_string_equals_literal(zkey, "skip_hostname_verify")){ - config->skip_hostname_verify = zend_is_true(zvalue); - } else { - php_error_docref(NULL, E_WARNING, "Got invalid TLS context configuration key: %s", ZSTR_VAL(zkey)); - ok = 0; - } - return ok; + zend_bool ok = 1; + if (zend_string_equals_literal(zkey, "cert_file")) { + config->cert_file = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "key_file")){ + config->key_file = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "key_file_pass")){ + config->key_file_pass = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "ca_cert_file")){ + config->ca_cert_file = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "ca_cert_dir")){ + config->ca_cert_dir = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "protocol")){ + config->protocol = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "hostname")){ + config->hostname = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "ciphers")){ + config->ciphers = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "ciphersuites")){ + config->ciphersuites = zval_dup_string(zvalue); + } + else if(zend_string_equals_literal(zkey, "prefer_server_ciphers")){ + config->prefer_server_ciphers = zend_is_true(zvalue); + } + else if(zend_string_equals_literal(zkey, "skip_cert_verify")){ + config->skip_cert_verify = zend_is_true(zvalue); + } + else if(zend_string_equals_literal(zkey, "skip_hostname_verify")){ + config->skip_hostname_verify = zend_is_true(zvalue); + } else { + php_error_docref(NULL, E_WARNING, "Got invalid TLS context configuration key: %s", ZSTR_VAL(zkey)); + ok = 0; + } + return ok; } static void php_free_tls_context_configurations(memcached_ssl_context_config *config) { if (config->cert_file) { efree(config->cert_file); } - if (config->key_file) { + if (config->key_file) { efree(config->key_file); - } - if (config->key_file_pass) { + } + if (config->key_file_pass) { efree(config->key_file_pass); - } - if (config->ca_cert_file) { + } + if (config->ca_cert_file) { efree(config->ca_cert_file); - } - if (config->ca_cert_dir) { + } + if (config->ca_cert_dir) { efree(config->ca_cert_dir); - } - if (config->protocol) { + } + if (config->protocol) { efree(config->protocol); - } - if (config->hostname) { + } + if (config->hostname) { efree(config->hostname); - } - if (config->ciphers) { + } + if (config->ciphers) { efree(config->ciphers); - } - if (config->ciphersuites) { + } + if (config->ciphersuites) { efree(config->ciphersuites); - } + } } #endif @@ -3474,33 +3474,33 @@ static PHP_METHOD(Memcached, createAndSetTLSContext) MEMC_METHOD_FETCH_OBJECT; - // Iterate through each configuration key cast its value type and set it to - // the TLS context configurations object (config) + // Iterate through each configuration key cast its value type and set it to + // the TLS context configurations object (config) ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(context_config), zkey, zvalue) { if (zkey == NULL) { php_error_docref(NULL, E_WARNING, "invalid TLS context configuration. Configuration keys must be strings"); RETURN_FALSE; } else { - if (!php_set_tls_context_configurations(&config, zkey, zvalue)) { + if (!php_set_tls_context_configurations(&config, zkey, zvalue)) { // Error in setting config, free context and return php_free_tls_context_configurations(&config); - RETURN_FALSE; - } + RETURN_FALSE; + } } } ZEND_HASH_FOREACH_END(); - // Create and set SSL context to the memcached client - error = memcached_create_and_set_ssl_context(intern->memc, &config); + // Create and set SSL context to the memcached client + error = memcached_create_and_set_ssl_context(intern->memc, &config); php_free_tls_context_configurations(&config); - if (error != MEMCACHED_SSL_CTX_SUCCESS) { - php_error_docref(NULL, E_ERROR, "Failed to create/set TLS context: %s", memcached_ssl_context_get_error(error)); - RETURN_FALSE; - } + if (error != MEMCACHED_SSL_CTX_SUCCESS) { + php_error_docref(NULL, E_ERROR, "Failed to create/set TLS context: %s", memcached_ssl_context_get_error(error)); + RETURN_FALSE; + } RETURN_TRUE; #else - php_error_docref(NULL, E_ERROR, "NOT SUPPORTED: build the client library with TLS enabled in order to use TLS methods."); - RETURN_FALSE; + php_error_docref(NULL, E_ERROR, "NOT SUPPORTED: build the client library with TLS enabled in order to use TLS methods."); + RETURN_FALSE; #endif /* HAVE_MEMCACHED_TLS */ } /* }}} */