From cf312295b7cfa8de5a1e74f231eed72b7740bacb Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Thu, 19 Oct 2023 18:07:48 -0700 Subject: [PATCH 1/4] If client-side verify_key is not enabled, don't check it automatically --- php_memcached.c | 1 + 1 file changed, 1 insertion(+) diff --git a/php_memcached.c b/php_memcached.c index 00d5e3d4..f30cf2af 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -246,6 +246,7 @@ zend_bool s_memc_valid_key_ascii(zend_string *key) #define MEMC_CHECK_KEY(intern, key) \ if (UNEXPECTED(ZSTR_LEN(key) == 0 || \ ZSTR_LEN(key) > MEMC_OBJECT_KEY_MAX_LENGTH || \ + memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_VERIFY_KEY) && \ (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) \ ? !s_memc_valid_key_binary(key) \ : !s_memc_valid_key_ascii(key) \ From e429fc9ec2f13e9b3774f6b6696c548b2af90ec1 Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Thu, 19 Oct 2023 19:06:12 -0700 Subject: [PATCH 2/4] fix tests --- tests/cas_invalid_key.phpt | 5 ++++- tests/delete_bykey.phpt | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/cas_invalid_key.phpt b/tests/cas_invalid_key.phpt index b8a815dc..9cb7293c 100644 --- a/tests/cas_invalid_key.phpt +++ b/tests/cas_invalid_key.phpt @@ -5,7 +5,10 @@ Memcached::cas() with strange key --FILE-- false, + Memcached::OPT_VERIFY_KEY => true +)); error_reporting(0); var_dump($m->cas(0, '', true, 10)); diff --git a/tests/delete_bykey.phpt b/tests/delete_bykey.phpt index 929ffbc1..6aa589c5 100644 --- a/tests/delete_bykey.phpt +++ b/tests/delete_bykey.phpt @@ -5,7 +5,10 @@ Memcached::deleteByKey() --FILE-- false, + Memcached::OPT_VERIFY_KEY => true +)); $m->setByKey('keffe', 'eisaleeoo', "foo"); var_dump($m->getByKey('keffe', 'eisaleeoo')); From 78ba261e82b1cd37c132c960514bc0bf767014a9 Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Thu, 19 Oct 2023 21:42:43 -0700 Subject: [PATCH 3/4] Fix more tests with a slight refactor --- php_memcached.c | 3 +-- tests/get.phpt | 5 ++++- tests/keys_ascii.phpt | 5 +---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index f30cf2af..fd9e4f1c 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -246,10 +246,9 @@ zend_bool s_memc_valid_key_ascii(zend_string *key) #define MEMC_CHECK_KEY(intern, key) \ if (UNEXPECTED(ZSTR_LEN(key) == 0 || \ ZSTR_LEN(key) > MEMC_OBJECT_KEY_MAX_LENGTH || \ - memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_VERIFY_KEY) && \ (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) \ ? !s_memc_valid_key_binary(key) \ - : !s_memc_valid_key_ascii(key) \ + : (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_VERIFY_KEY) && !s_memc_valid_key_ascii(key)) \ ))) { \ intern->rescode = MEMCACHED_BAD_KEY_PROVIDED; \ RETURN_FALSE; \ diff --git a/tests/get.phpt b/tests/get.phpt index 71889064..722308f0 100644 --- a/tests/get.phpt +++ b/tests/get.phpt @@ -5,7 +5,10 @@ Memcached::get() --FILE-- false, + Memcached::OPT_VERIFY_KEY => true +)); $m->delete('foo'); diff --git a/tests/keys_ascii.phpt b/tests/keys_ascii.phpt index f7e98894..e7846e99 100644 --- a/tests/keys_ascii.phpt +++ b/tests/keys_ascii.phpt @@ -8,11 +8,8 @@ Test valid and invalid keys - ascii include dirname (__FILE__) . '/config.inc'; $ascii = memc_get_instance (array ( Memcached::OPT_BINARY_PROTOCOL => false, - Memcached::OPT_VERIFY_KEY => false + Memcached::OPT_VERIFY_KEY => true )); -// libmemcached can verify keys, but these are tests are for our own -// function s_memc_valid_key_ascii, so explicitly disable the checks -// that libmemcached can perform. echo 'ASCII: SPACES' . PHP_EOL; var_dump ($ascii->set ('ascii key with spaces', 'this is a test')); From 1527217e1488289f5f3ab545c12279b0083d2814 Mon Sep 17 00:00:00 2001 From: Rasmus Lerdorf Date: Fri, 20 Oct 2023 08:00:37 -0700 Subject: [PATCH 4/4] Check for spaces in keys when using the non-binary protocol even if key verification is disabled to avoid injection issues --- php_memcached.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index fd9e4f1c..8054b3eb 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -231,14 +231,21 @@ zend_bool s_memc_valid_key_binary(zend_string *key) } static -zend_bool s_memc_valid_key_ascii(zend_string *key) +zend_bool s_memc_valid_key_ascii(zend_string *key, uint64_t verify_key) { const char *str = ZSTR_VAL(key); size_t i, len = ZSTR_LEN(key); - for (i = 0; i < len; i++) { - if (!isgraph(str[i]) || isspace(str[i])) - return 0; + if (verify_key) { + for (i = 0; i < len; i++) { + if (!isgraph(str[i]) || isspace(str[i])) + return 0; + } + } else { /* if key verification is disabled, only check for spaces to avoid injection issues */ + for (i = 0; i < len; i++) { + if (isspace(str[i])) + return 0; + } } return 1; } @@ -248,7 +255,7 @@ zend_bool s_memc_valid_key_ascii(zend_string *key) ZSTR_LEN(key) > MEMC_OBJECT_KEY_MAX_LENGTH || \ (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) \ ? !s_memc_valid_key_binary(key) \ - : (memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_VERIFY_KEY) && !s_memc_valid_key_ascii(key)) \ + : !s_memc_valid_key_ascii(key, memcached_behavior_get(intern->memc, MEMCACHED_BEHAVIOR_VERIFY_KEY)) \ ))) { \ intern->rescode = MEMCACHED_BAD_KEY_PROVIDED; \ RETURN_FALSE; \ @@ -342,7 +349,7 @@ PHP_INI_MH(OnUpdateSessionPrefixString) php_error_docref(NULL, E_WARNING, "memcached.sess_prefix too long (max: %d)", MEMCACHED_MAX_KEY - 1); return FAILURE; } - if (!s_memc_valid_key_ascii(new_value)) { + if (!s_memc_valid_key_ascii(new_value, 1)) { php_error_docref(NULL, E_WARNING, "memcached.sess_prefix cannot contain whitespace or control characters"); return FAILURE; }