From 5966fefb9a291bd0eecd0fff9396b2b6cea4a62e Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 7 Dec 2022 20:37:25 +0100 Subject: Minor refactor --- src/sp_unserialize.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/sp_unserialize.c b/src/sp_unserialize.c index 64cf1b5..e57ef9c 100644 --- a/src/sp_unserialize.c +++ b/src/sp_unserialize.c @@ -90,14 +90,15 @@ PHP_FUNCTION(sp_unserialize) { char *serialized_str = NULL; char *hmac = NULL; size_t buf_len = 0; - zval *opts = NULL; + HashTable *opts = NULL; const sp_config_unserialize *config_unserialize = &(SPCFG(unserialize)); - if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|a", &buf, &buf_len, &opts) == - FAILURE) { - RETURN_FALSE; - } + ZEND_PARSE_PARAMETERS_START(1, 2) + Z_PARAM_STRING(buf, buf_len) + Z_PARAM_OPTIONAL + Z_PARAM_ARRAY_HT(opts) + ZEND_PARSE_PARAMETERS_END(); /* 64 is the length of HMAC-256 */ if (buf_len < 64) { -- cgit v1.3 From ccfaf3e4713b1878241f1235a6fcb66ad0582d47 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 7 Dec 2022 21:02:22 +0100 Subject: Add unserialize_noclass --- doc/source/config.rst | 14 ++++++++ doc/source/features.rst | 2 ++ src/php_snuffleupagus.h | 1 + src/snuffleupagus.c | 8 ++--- src/sp_config.c | 1 + src/sp_config.h | 6 ++++ src/sp_config_keywords.c | 18 ++++++++++ src/sp_config_keywords.h | 1 + src/sp_unserialize.c | 30 ++++++++++++----- .../config/config_serialize_noclass.ini | 1 + .../unserialize/unserialize_noclass_forced.phpt | 21 ++++++++++++ src/tests/unserialize/unserialize_wrong_call.phpt | 2 +- .../config/config_serialize_noclass.ini | 1 + .../config/config_serialize_noclass_disabled.ini | 1 + .../unserialize_noclass_forced.phpt | 38 ++++++++++++++++++++++ .../unserialize_noclass_forced_disabled.phpt | 35 ++++++++++++++++++++ 16 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 src/tests/unserialize/config/config_serialize_noclass.ini create mode 100644 src/tests/unserialize/unserialize_noclass_forced.phpt create mode 100644 src/tests/unserialize_php8/config/config_serialize_noclass.ini create mode 100644 src/tests/unserialize_php8/config/config_serialize_noclass_disabled.ini create mode 100644 src/tests/unserialize_php8/unserialize_noclass_forced.phpt create mode 100644 src/tests/unserialize_php8/unserialize_noclass_forced_disabled.phpt diff --git a/doc/source/config.rst b/doc/source/config.rst index 9d2d0ed..bce4667 100644 --- a/doc/source/config.rst +++ b/doc/source/config.rst @@ -202,6 +202,20 @@ It can either be ``enabled`` or ``disabled``. sp.sloppy_comparison.enable(); sp.sloppy_comparison.disable(); +unserialize_noclass +^^^^^^^^^^^^^^^^^^^ + +:ref:`unserialize_noclass `, available only on PHP8+ and +disabled by default, will disable the deserialization of objects via +``unserialize``. It's equivalent to setting the ``options`` parameter of +``unserialize`` to ``false``, on every call. It can either be ``enabled`` or +``disabled``. + +:: + + sp.unserialize_noclass.enable(); + sp.unserialize_noclass.disable(); + unserialize_hmac ^^^^^^^^^^^^^^^^ diff --git a/doc/source/features.rst b/doc/source/features.rst index 25fd62d..60dbbef 100644 --- a/doc/source/features.rst +++ b/doc/source/features.rst @@ -166,6 +166,8 @@ CVE-2016-9138 `_, `2016-7124 `_, `CVE-2016-5771 and CVE-2016-5773 `_. +A less subtle mitigation can be used to simply prevent the deserialization of objects altogether. + Examples of related vulnerabilities """"""""""""""""""""""""""""""""""" diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h index 7c534a8..2117462 100644 --- a/src/php_snuffleupagus.h +++ b/src/php_snuffleupagus.h @@ -119,6 +119,7 @@ ZEND_BEGIN_MODULE_GLOBALS(snuffleupagus) sp_config_random config_random; sp_config_sloppy config_sloppy; sp_config_unserialize config_unserialize; +sp_config_unserialize_noclass config_unserialize_noclass; sp_config_readonly_exec config_readonly_exec; sp_config_upload_validation config_upload_validation; sp_config_cookie config_cookie; diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index be6240e..8454fc1 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -332,6 +332,8 @@ static void dump_config() { array_init(&arr); add_assoc_string(&arr, "version", PHP_SNUFFLEUPAGUS_VERSION); + add_assoc_bool(&arr, SP_TOKEN_UNSERIALIZE_NOCLASS "." SP_TOKEN_ENABLE, SPCFG(unserialize_noclass).enable); + add_assoc_bool(&arr, SP_TOKEN_UNSERIALIZE_HMAC "." SP_TOKEN_ENABLE, SPCFG(unserialize).enable); add_assoc_bool(&arr, SP_TOKEN_UNSERIALIZE_HMAC "." SP_TOKEN_SIM, SPCFG(unserialize).simulation); ADD_ASSOC_ZSTR(&arr, SP_TOKEN_UNSERIALIZE_HMAC "." SP_TOKEN_DUMP, SPCFG(unserialize).dump); @@ -562,10 +564,8 @@ static PHP_INI_MH(OnUpdateConfiguration) { hook_session(); } - if (NULL != SPCFG(encryption_key)) { - if (SPCFG(unserialize).enable) { - hook_serialize(); - } + if ((NULL != SPCFG(encryption_key) && SPCFG(unserialize).enable) || SPCFG(unserialize_noclass).enable) { + hook_serialize(); } hook_execute(); diff --git a/src/sp_config.c b/src/sp_config.c index dbccb1a..8bd238a 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -8,6 +8,7 @@ static zend_result sp_process_config_root(sp_parsed_keyword *parsed_rule) { sp_config_keyword sp_func[] = { {parse_unserialize, SP_TOKEN_UNSERIALIZE_HMAC, &(SPCFG(unserialize))}, + {parse_unserialize_noclass, SP_TOKEN_UNSERIALIZE_NOCLASS, &(SPCFG(unserialize_noclass))}, {parse_enable, SP_TOKEN_HARDEN_RANDOM, &(SPCFG(random).enable)}, {parse_log_media, SP_TOKEN_LOG_MEDIA, &(SPCFG(log_media))}, {parse_disabled_functions, SP_TOKEN_DISABLE_FUNC, NULL}, diff --git a/src/sp_config.h b/src/sp_config.h index ec73310..cddf816 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -81,6 +81,11 @@ typedef struct { u_long sid_max_length; } sp_config_session; +typedef struct { + bool enable; + zend_string *textual_representation; +} sp_config_unserialize_noclass; + typedef struct { bool enable; bool simulation; @@ -202,6 +207,7 @@ typedef struct { #define SP_TOKEN_HARDEN_RANDOM "harden_random" #define SP_TOKEN_READONLY_EXEC "readonly_exec" #define SP_TOKEN_UNSERIALIZE_HMAC "unserialize_hmac" +#define SP_TOKEN_UNSERIALIZE_NOCLASS "unserialize_noclass" #define SP_TOKEN_UPLOAD_VALIDATION "upload_validation" #define SP_TOKEN_XXE_PROTECTION "xxe_protection" #define SP_TOKEN_EVAL_BLACKLIST "eval_blacklist" diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index fa26635..ff834dd 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -74,6 +74,24 @@ SP_PARSEKW_FN(parse_log_media) { return SP_PARSER_ERROR; } +SP_PARSE_FN(parse_unserialize_noclass) { + bool enable = false, disable = false; + sp_config_unserialize_noclass *cfg = (sp_config_unserialize_noclass*)retval; + + sp_config_keyword config_keywords[] = { + {parse_empty, SP_TOKEN_ENABLE, &(enable)}, + {parse_empty, SP_TOKEN_DISABLE, &(disable)}, + {0, 0, 0}}; + + SP_PROCESS_CONFIG_KEYWORDS_ERR(); + + SP_SET_ENABLE_DISABLE(enable, disable, cfg->enable); + + cfg->textual_representation = sp_get_textual_representation(parsed_rule); + + return SP_PARSER_STOP; +} + SP_PARSE_FN(parse_unserialize) { bool enable = false, disable = false; sp_config_unserialize *cfg = (sp_config_unserialize*)retval; diff --git a/src/sp_config_keywords.h b/src/sp_config_keywords.h index 01eb0d1..27050e3 100644 --- a/src/sp_config_keywords.h +++ b/src/sp_config_keywords.h @@ -6,6 +6,7 @@ SP_PARSE_FN(parse_enable); SP_PARSE_FN(parse_global); SP_PARSE_FN(parse_cookie); SP_PARSE_FN(parse_unserialize); +SP_PARSE_FN(parse_unserialize_noclass); SP_PARSE_FN(parse_readonly_exec); SP_PARSE_FN(parse_disabled_functions); SP_PARSE_FN(parse_upload_validation); diff --git a/src/sp_unserialize.c b/src/sp_unserialize.c index e57ef9c..641d989 100644 --- a/src/sp_unserialize.c +++ b/src/sp_unserialize.c @@ -61,6 +61,10 @@ PHP_FUNCTION(sp_serialize) { orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } + if (!SPCFG(unserialize).enable) { + return; + } + /* Compute the HMAC of the textual representation of the serialized data*/ zend_string *hmac = sp_do_hash_hmac_sha256(Z_STRVAL_P(return_value), Z_STRLEN_P(return_value), ZSTR_VAL(SPCFG(encryption_key)), ZSTR_LEN(SPCFG(encryption_key))); @@ -84,29 +88,37 @@ PHP_FUNCTION(sp_serialize) { } PHP_FUNCTION(sp_unserialize) { - zif_handler orig_handler; - char *buf = NULL; - char *serialized_str = NULL; - char *hmac = NULL; size_t buf_len = 0; HashTable *opts = NULL; - const sp_config_unserialize *config_unserialize = &(SPCFG(unserialize)); - ZEND_PARSE_PARAMETERS_START(1, 2) Z_PARAM_STRING(buf, buf_len) Z_PARAM_OPTIONAL Z_PARAM_ARRAY_HT(opts) ZEND_PARSE_PARAMETERS_END(); + if (SPCFG(unserialize_noclass).enable) { +#if PHP_VERSION_ID > 80000 + HashTable ht; + zend_hash_init(&ht, 1, NULL, NULL, 0); + zval zv; + ZVAL_FALSE(&zv); + zend_hash_str_add(&ht, "allowed_classes", sizeof("allowed_classes")-1, &zv); + php_unserialize_with_options(return_value, buf, buf_len, &ht, "unserialize"); + return; +#else + sp_log_drop("unserialize_noclass", "unserialize_noclass is only supported on PHP8+"); +#endif + } + /* 64 is the length of HMAC-256 */ if (buf_len < 64) { sp_log_drop("unserialize", "The serialized object is too small."); } - hmac = buf + buf_len - 64; - serialized_str = ecalloc(buf_len - 64 + 1, 1); + char* hmac = buf + buf_len - 64; + char* serialized_str = ecalloc(buf_len - 64 + 1, 1); memcpy(serialized_str, buf, buf_len - 64); zend_string *expected_hmac = sp_do_hash_hmac_sha256(serialized_str, strlen(serialized_str), ZSTR_VAL(SPCFG(encryption_key)), ZSTR_LEN(SPCFG(encryption_key))); @@ -118,11 +130,13 @@ PHP_FUNCTION(sp_unserialize) { } } else { status = 1; } + zif_handler orig_handler; if (0 == status) { if ((orig_handler = zend_hash_str_find_ptr(SPG(sp_internal_functions_hook), ZEND_STRL("unserialize")))) { orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } } else { + const sp_config_unserialize *config_unserialize = &(SPCFG(unserialize)); if (config_unserialize->dump) { sp_log_request(config_unserialize->dump, config_unserialize->textual_representation); diff --git a/src/tests/unserialize/config/config_serialize_noclass.ini b/src/tests/unserialize/config/config_serialize_noclass.ini new file mode 100644 index 0000000..b84de51 --- /dev/null +++ b/src/tests/unserialize/config/config_serialize_noclass.ini @@ -0,0 +1 @@ +sp.unserialize_noclass.enable(); diff --git a/src/tests/unserialize/unserialize_noclass_forced.phpt b/src/tests/unserialize/unserialize_noclass_forced.phpt new file mode 100644 index 0000000..3b1e8d3 --- /dev/null +++ b/src/tests/unserialize/unserialize_noclass_forced.phpt @@ -0,0 +1,21 @@ +--TEST-- +Unserialize with noclass forced +--SKIPIF-- += 80000) print "skip"; ?> +--INI-- +sp.configuration_file={PWD}/config/config_serialize_noclass.ini +--FILE-- +name = "test"; + +$a = serialize($c); +var_dump(unserialize($a, ['allowed_classes' => false])); +var_dump(unserialize($a, ['allowed_classes' => true ])); +var_dump(unserialize($a)); +?> +--EXPECTF-- +Fatal error: [snuffleupagus][0.0.0.0][unserialize_noclass][drop] unserialize_noclass is only supported on PHP8+ in %s/tests/unserialize/unserialize_noclass_forced.php on line 9 diff --git a/src/tests/unserialize/unserialize_wrong_call.phpt b/src/tests/unserialize/unserialize_wrong_call.phpt index a6fe140..afa42f6 100644 --- a/src/tests/unserialize/unserialize_wrong_call.phpt +++ b/src/tests/unserialize/unserialize_wrong_call.phpt @@ -12,4 +12,4 @@ var_dump(unserialize($a, "too", "many", "aaaaaaaarguments!")); ?> --EXPECTF-- Warning: unserialize() expects at most 2 parameters, 4 given in %a/unserialize_wrong_call.php on line %d -bool(false) +NULL diff --git a/src/tests/unserialize_php8/config/config_serialize_noclass.ini b/src/tests/unserialize_php8/config/config_serialize_noclass.ini new file mode 100644 index 0000000..b84de51 --- /dev/null +++ b/src/tests/unserialize_php8/config/config_serialize_noclass.ini @@ -0,0 +1 @@ +sp.unserialize_noclass.enable(); diff --git a/src/tests/unserialize_php8/config/config_serialize_noclass_disabled.ini b/src/tests/unserialize_php8/config/config_serialize_noclass_disabled.ini new file mode 100644 index 0000000..0238772 --- /dev/null +++ b/src/tests/unserialize_php8/config/config_serialize_noclass_disabled.ini @@ -0,0 +1 @@ +sp.unserialize_noclass.disable(); diff --git a/src/tests/unserialize_php8/unserialize_noclass_forced.phpt b/src/tests/unserialize_php8/unserialize_noclass_forced.phpt new file mode 100644 index 0000000..9f276c5 --- /dev/null +++ b/src/tests/unserialize_php8/unserialize_noclass_forced.phpt @@ -0,0 +1,38 @@ +--TEST-- +Unserialize with noclass forced +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/config_serialize_noclass.ini +--FILE-- +name = "test"; + +$a = serialize($c); +var_dump(unserialize($a, ['allowed_classes' => false])); +var_dump(unserialize($a, ['allowed_classes' => true ])); +var_dump(unserialize($a)); +?> +--EXPECT-- +object(__PHP_Incomplete_Class)#2 (2) { + ["__PHP_Incomplete_Class_Name"]=> + string(1) "C" + ["name"]=> + string(4) "test" +} +object(__PHP_Incomplete_Class)#2 (2) { + ["__PHP_Incomplete_Class_Name"]=> + string(1) "C" + ["name"]=> + string(4) "test" +} +object(__PHP_Incomplete_Class)#2 (2) { + ["__PHP_Incomplete_Class_Name"]=> + string(1) "C" + ["name"]=> + string(4) "test" +} diff --git a/src/tests/unserialize_php8/unserialize_noclass_forced_disabled.phpt b/src/tests/unserialize_php8/unserialize_noclass_forced_disabled.phpt new file mode 100644 index 0000000..2c4223a --- /dev/null +++ b/src/tests/unserialize_php8/unserialize_noclass_forced_disabled.phpt @@ -0,0 +1,35 @@ +--TEST-- +Unserialize with noclass forced disabled +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/config_serialize_noclass_disabled.ini +--FILE-- +name = "test"; + +$a = serialize($c); +var_dump(unserialize($a, ['allowed_classes' => false])); +var_dump(unserialize($a, ['allowed_classes' => true ])); +var_dump(unserialize($a)); +?> +--EXPECT-- +object(__PHP_Incomplete_Class)#2 (2) { + ["__PHP_Incomplete_Class_Name"]=> + string(1) "C" + ["name"]=> + string(4) "test" +} +object(C)#2 (1) { + ["name"]=> + string(4) "test" +} +object(C)#2 (1) { + ["name"]=> + string(4) "test" +} + -- cgit v1.3