Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak with passing in TLS context config from php application #50

Merged
merged 2 commits into from
Mar 20, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 72 additions & 55 deletions php_memcached.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
****************************************/
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: indentation is off

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
Expand Down Expand Up @@ -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;
Expand All @@ -3442,39 +3481,17 @@ 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;
}
}
} ZEND_HASH_FOREACH_END();

// 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;
Expand Down
Loading