From ad6b3e723fe26bf1a3a573aed776960916d35499 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 10 Jan 2018 14:56:33 +0100 Subject: Eval whitelist Implement whitelist in eval--- src/php_snuffleupagus.h | 4 +- src/snuffleupagus.c | 4 +- src/sp_config.c | 3 +- src/sp_config.h | 6 +- src/sp_config_keywords.c | 16 ++++- src/sp_config_keywords.h | 3 +- src/sp_disabled_functions.c | 12 ++-- src/sp_disabled_functions.h | 1 + src/sp_execute.c | 88 +++++++++++++++++++------ src/tests/config/eval_backlist.ini | 2 +- src/tests/config/eval_backlist_list.ini | 2 +- src/tests/config/eval_backlist_simulation.ini | 2 +- src/tests/config/eval_whitelist.ini | 1 + src/tests/eval_backlist_whitelist.phpt | 27 ++++++++ src/tests/eval_whitelist_builtin.phpt | 19 ++++++ src/tests/eval_whitelist_include_then_user.phpt | 29 ++++++++ src/tests/eval_whitelist_user_then_builtin.phpt | 23 +++++++ 17 files changed, 203 insertions(+), 39 deletions(-) create mode 100644 src/tests/config/eval_whitelist.ini create mode 100644 src/tests/eval_backlist_whitelist.phpt create mode 100644 src/tests/eval_whitelist_builtin.phpt create mode 100644 src/tests/eval_whitelist_include_then_user.phpt create mode 100644 src/tests/eval_whitelist_user_then_builtin.phpt (limited to 'src') diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h index ca39bb8..93ef472 100644 --- a/src/php_snuffleupagus.h +++ b/src/php_snuffleupagus.h @@ -63,7 +63,7 @@ sp_config config; bool is_config_valid; HashTable *disabled_functions_hook; HashTable *sp_internal_functions_hook; -HashTable *sp_eval_filter_functions_hook; +HashTable *sp_eval_blacklist_functions_hook; ZEND_END_MODULE_GLOBALS(snuffleupagus) #define SNUFFLEUPAGUS_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(snuffleupagus, v) @@ -85,7 +85,7 @@ ZEND_TSRMLS_CACHE_EXTERN() #endif PHP_FUNCTION(check_disabled_function); -PHP_FUNCTION(eval_filter_callback); +PHP_FUNCTION(eval_blacklist_callback); static inline void sp_terminate() { zend_bailout(); } diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 4f11e1e..cb79b23 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -62,7 +62,7 @@ PHP_GINIT_FUNCTION(snuffleupagus) { SP_INIT_HT(snuffleupagus_globals->disabled_functions_hook); SP_INIT_HT(snuffleupagus_globals->sp_internal_functions_hook); - SP_INIT_HT(snuffleupagus_globals->sp_eval_filter_functions_hook); + SP_INIT_HT(snuffleupagus_globals->sp_eval_blacklist_functions_hook); SP_INIT(snuffleupagus_globals->config.config_unserialize); SP_INIT(snuffleupagus_globals->config.config_random); @@ -106,7 +106,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { pefree(SNUFFLEUPAGUS_G(F), 1); FREE_HT(disabled_functions_hook); - FREE_HT(sp_eval_filter_functions_hook); + FREE_HT(sp_eval_blacklist_functions_hook); #undef FREE_HT diff --git a/src/sp_config.c b/src/sp_config.c index dc5aae9..4037e26 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -19,7 +19,8 @@ sp_config_tokens const sp_func[] = { {.func = parse_global, .token = SP_TOKEN_GLOBAL}, {.func = parse_auto_cookie_secure, .token = SP_TOKEN_AUTO_COOKIE_SECURE}, {.func = parse_disable_xxe, .token = SP_TOKEN_DISABLE_XXE}, - {.func = parse_eval_filter, .token = SP_TOKEN_EVAL}, + {.func = parse_eval_blacklist, .token = SP_TOKEN_EVAL_BLACKLIST}, + {.func = parse_eval_whitelist, .token = SP_TOKEN_EVAL_WHITELIST}, {NULL, NULL}}; /* Top level keyword parsing */ diff --git a/src/sp_config.h b/src/sp_config.h index a4a4f10..25963f1 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -177,7 +177,8 @@ typedef struct { #define SP_TOKEN_UNSERIALIZE_HMAC ".unserialize_hmac" #define SP_TOKEN_UPLOAD_VALIDATION ".upload_validation" #define SP_TOKEN_DISABLE_XXE ".disable_xxe" -#define SP_TOKEN_EVAL ".eval_filter" +#define SP_TOKEN_EVAL_BLACKLIST ".eval_blacklist" +#define SP_TOKEN_EVAL_WHITELIST ".eval_whitelist" // common tokens #define SP_TOKEN_ENABLE ".enable(" @@ -231,8 +232,7 @@ typedef struct { #define SP_TOKEN_UPLOAD_SCRIPT ".script(" // eval blacklist -#define SP_TOKEN_EVAL_BLACKLIST ".blacklist(" -#define SP_TOKEN_EVAL_WHITELIST ".whitelist(" +#define SP_TOKEN_EVAL_LIST ".list(" int sp_parse_config(const char *); int parse_array(sp_disabled_function *); diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 85e04ab..c5cc950 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -102,11 +102,11 @@ int parse_global(char *line) { return parse_keywords(sp_config_funcs_global, line); } -int parse_eval_filter(char *line) { +static int parse_eval_filter_conf(char *line, sp_list_node *list) { char *token; char *rest; sp_config_functions sp_config_funcs[] = { - {parse_str, SP_TOKEN_EVAL_BLACKLIST, &rest}, + {parse_str, SP_TOKEN_EVAL_LIST, &rest}, {parse_empty, SP_TOKEN_SIMULATION, &(SNUFFLEUPAGUS_G(config).config_eval->simulation)}, {0}}; @@ -116,11 +116,21 @@ int parse_eval_filter(char *line) { } while ((token = strtok_r(rest, ",", &rest))) { - sp_list_insert(SNUFFLEUPAGUS_G(config).config_eval->blacklist, token); + sp_list_insert(list, token); } return SUCCESS; } +int parse_eval_blacklist(char *line) { + return parse_eval_filter_conf(line, + SNUFFLEUPAGUS_G(config).config_eval->blacklist); +} + +int parse_eval_whitelist(char *line) { + return parse_eval_filter_conf(line, + SNUFFLEUPAGUS_G(config).config_eval->whitelist); +} + int parse_cookie(char *line) { int ret = 0; char *samesite = NULL; diff --git a/src/sp_config_keywords.h b/src/sp_config_keywords.h index d7ea36a..4205dac 100644 --- a/src/sp_config_keywords.h +++ b/src/sp_config_keywords.h @@ -12,6 +12,7 @@ int parse_unserialize(char *line); int parse_readonly_exec(char *line); int parse_disabled_functions(char *line); int parse_upload_validation(char *line); -int parse_eval_filter(char *line); +int parse_eval_blacklist(char *line); +int parse_eval_whitelist(char *line); #endif // __SP_CONFIG_KEYWORDS_H diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index 8e96085..fa9d625 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -5,7 +5,8 @@ ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus) -static char* get_complete_function_path( + +char* get_complete_function_path( zend_execute_data const* const execute_data) { if (zend_is_executing() && !EG(current_execute_data)->func) { return NULL; @@ -107,6 +108,7 @@ static const sp_list_node* get_config_node(const char* builtin_name) { return SNUFFLEUPAGUS_G(config) .config_disabled_constructs->construct_include; } + ZEND_ASSUME(0); return NULL; // This should never happen. } @@ -463,7 +465,7 @@ static int hook_functions(const sp_list_node* config) { return SUCCESS; } -ZEND_FUNCTION(eval_filter_callback) { +ZEND_FUNCTION(eval_blacklist_callback) { void (*orig_handler)(INTERNAL_FUNCTION_PARAMETERS); const char* current_function_name = get_active_function_name(TSRMLS_C); @@ -483,7 +485,7 @@ ZEND_FUNCTION(eval_filter_callback) { } orig_handler = zend_hash_str_find_ptr( - SNUFFLEUPAGUS_G(sp_eval_filter_functions_hook), current_function_name, + SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook), current_function_name, strlen(current_function_name)); orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -503,8 +505,8 @@ int hook_disabled_functions(void) { while (it) { hook_function((char*)it->data, - SNUFFLEUPAGUS_G(sp_eval_filter_functions_hook), - PHP_FN(eval_filter_callback), false); + SNUFFLEUPAGUS_G(sp_eval_blacklist_functions_hook), + PHP_FN(eval_blacklist_callback), false); it = it->next; } } diff --git a/src/sp_disabled_functions.h b/src/sp_disabled_functions.h index f80c9c2..9629308 100644 --- a/src/sp_disabled_functions.h +++ b/src/sp_disabled_functions.h @@ -5,5 +5,6 @@ int hook_disabled_functions(); bool should_disable(zend_execute_data *, const char *, const char *, const char *); bool should_drop_on_ret(zval *, const zend_execute_data *const); +char* get_complete_function_path(zend_execute_data const* const execute_data); #endif /* __SP_DISABLE_FUNCTIONS_H */ diff --git a/src/sp_execute.c b/src/sp_execute.c index 3ce6643..c1d68d7 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -6,6 +6,8 @@ ZEND_DECLARE_MODULE_GLOBALS(snuffleupagus) static void (*orig_execute_ex)(zend_execute_data *execute_data); +static void (*orig_zend_execute_internal)(zend_execute_data *execute_data, + zval *return_value); static int (*orig_zend_stream_open)(const char *filename, zend_file_handle *handle); @@ -29,9 +31,9 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { } static void is_builtin_matching(const char *restrict const filename, - char *restrict function_name, - char *restrict param_name, - sp_list_node *config) { + const char *restrict const function_name, + const char *restrict const param_name, + const sp_list_node *config) { if (!config || !config->data) { return; } @@ -42,6 +44,43 @@ static void is_builtin_matching(const char *restrict const filename, } } +static void is_in_eval_and_whitelisted(zend_execute_data *execute_data) { + if (EXPECTED(0 == SNUFFLEUPAGUS_G(in_eval))) { + return; + } + + if (EXPECTED(NULL == SNUFFLEUPAGUS_G(config).config_eval->whitelist->data)) { + return; + } + + if (zend_is_executing() && !EG(current_execute_data)->func) { + return; + } + + if (!(execute_data->func->common.function_name)) { + return; + } + + char const* const current_function = ZSTR_VAL(EX(func)->common.function_name); + + if (EXPECTED(current_function)) { + const sp_list_node *it = SNUFFLEUPAGUS_G(config).config_eval->whitelist; + while (it) { + if (0 == strcmp(current_function, (char *)(it->data))) { + /* We've got a match, the function is whiteslited. */ + return; + } + it = it->next; + } + + sp_log_msg( + "Eval_whitelist", SP_LOG_DROP, + "The function '%s' isn't in the eval whitelist, dropping its call.", + current_function); + sp_terminate(); + } +} + /* This function gets the filename in which `eval()` is called from, * since it looks like "foo.php(1) : eval()'d code", so we're starting * from the end of the string until the second closing parenthesis. */ @@ -67,28 +106,42 @@ static void sp_execute_ex(zend_execute_data *execute_data) { sp_terminate(); } - if (execute_data->func->op_array.type == ZEND_EVAL_CODE) { + if (EX(func)->op_array.type == ZEND_EVAL_CODE) { SNUFFLEUPAGUS_G(in_eval)++; - sp_list_node *config = + const sp_list_node *config = SNUFFLEUPAGUS_G(config).config_disabled_constructs->construct_eval; char *filename = get_eval_filename((char *)zend_get_executed_filename()); is_builtin_matching(filename, "eval", NULL, config); efree(filename); } - if (NULL != execute_data->func->op_array.filename) { + is_in_eval_and_whitelisted(execute_data); + + if (NULL != EX(func)->op_array.filename) { if (true == SNUFFLEUPAGUS_G(config).config_readonly_exec->enable) { - terminate_if_writable(ZSTR_VAL(execute_data->func->op_array.filename)); + terminate_if_writable(ZSTR_VAL(EX(func)->op_array.filename)); } } orig_execute_ex(execute_data); - if (true == should_drop_on_ret(execute_data->return_value, execute_data)) { + if (true == should_drop_on_ret(EX(return_value), execute_data)) { sp_terminate(); } - SNUFFLEUPAGUS_G(in_eval)--; + if (ZEND_EVAL_CODE == EX(func)->op_array.type) { + SNUFFLEUPAGUS_G(in_eval)--; + } +} + +static void sp_zend_execute_internal(INTERNAL_FUNCTION_PARAMETERS) { + is_in_eval_and_whitelisted(execute_data); + + EX(func)->internal_function.handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); + + if (UNEXPECTED(NULL != orig_zend_execute_internal)) { + orig_zend_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU); + } } static int sp_stream_open(const char *filename, zend_file_handle *handle) { @@ -121,11 +174,7 @@ static int sp_stream_open(const char *filename, zend_file_handle *handle) { is_builtin_matching(filename, "include_once", "inclusion path", config); break; - case ZEND_EVAL: - is_builtin_matching(filename, "eval", NULL, config); - break; - default: - break; + EMPTY_SWITCH_DEFAULT_CASE(); } } @@ -136,16 +185,17 @@ end: int hook_execute(void) { TSRMLS_FETCH(); - /* zend_execute_ex is used for "classic" function calls */ + /* zend_execute_ex is used for "user" function calls */ orig_execute_ex = zend_execute_ex; zend_execute_ex = sp_execute_ex; - /* zend_stream_open_function is used FIXME */ + /* zend_execute_internal is used for "builtin" functions calls */ + orig_zend_execute_internal = zend_execute_internal; + zend_execute_internal = sp_zend_execute_internal; + + /* zend_stream_open_function is used for include-related stuff */ orig_zend_stream_open = zend_stream_open_function; zend_stream_open_function = sp_stream_open; - /* zend_execute_internal is used for "indirect" functions call, - * like array_map or call_user_func. */ - return SUCCESS; } diff --git a/src/tests/config/eval_backlist.ini b/src/tests/config/eval_backlist.ini index 1e34b5b..b181598 100644 --- a/src/tests/config/eval_backlist.ini +++ b/src/tests/config/eval_backlist.ini @@ -1 +1 @@ -sp.eval_filter.blacklist("strlen"); +sp.eval_blacklist.list("strlen"); diff --git a/src/tests/config/eval_backlist_list.ini b/src/tests/config/eval_backlist_list.ini index da5650d..b395d03 100644 --- a/src/tests/config/eval_backlist_list.ini +++ b/src/tests/config/eval_backlist_list.ini @@ -1 +1 @@ -sp.eval_filter.blacklist("strcmp,strlen"); +sp.eval_blacklist.list("strcmp,strlen"); diff --git a/src/tests/config/eval_backlist_simulation.ini b/src/tests/config/eval_backlist_simulation.ini index fafebd5..2d8dc73 100644 --- a/src/tests/config/eval_backlist_simulation.ini +++ b/src/tests/config/eval_backlist_simulation.ini @@ -1 +1 @@ -sp.eval_filter.blacklist("strlen").simulation(); +sp.eval_blacklist.list("strlen").simulation(); diff --git a/src/tests/config/eval_whitelist.ini b/src/tests/config/eval_whitelist.ini new file mode 100644 index 0000000..7a8f6ef --- /dev/null +++ b/src/tests/config/eval_whitelist.ini @@ -0,0 +1 @@ +sp.eval_whitelist.list("my_fun,cos"); diff --git a/src/tests/eval_backlist_whitelist.phpt b/src/tests/eval_backlist_whitelist.phpt new file mode 100644 index 0000000..1611288 --- /dev/null +++ b/src/tests/eval_backlist_whitelist.phpt @@ -0,0 +1,27 @@ +--TEST-- +Eval whitelist +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/eval_whitelist.ini +--FILE-- + +--EXPECTF-- +Outside of eval: my_fun: 1337 1337 1337 +After allowed eval: my_fun: 1234 +[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'my_other_fun' isn't in the eval whitelist, dropping its call. diff --git a/src/tests/eval_whitelist_builtin.phpt b/src/tests/eval_whitelist_builtin.phpt new file mode 100644 index 0000000..bd7c2ac --- /dev/null +++ b/src/tests/eval_whitelist_builtin.phpt @@ -0,0 +1,19 @@ +--TEST-- +Eval whitelist - builtin function +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/eval_whitelist.ini +--FILE-- + +--EXPECTF-- +Outside of eval: 0.54030230586814 +After allowed eval: 0.28366218546323 +[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call. diff --git a/src/tests/eval_whitelist_include_then_user.phpt b/src/tests/eval_whitelist_include_then_user.phpt new file mode 100644 index 0000000..6d4e36a --- /dev/null +++ b/src/tests/eval_whitelist_include_then_user.phpt @@ -0,0 +1,29 @@ +--TEST-- +Eval whitelist - builtin function +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/eval_whitelist.ini +--FILE-- +'); + +$a = cos(1); +echo "Outside of eval: $a\n"; +eval('$a = cos(5);'); +echo "After allowed eval: $a\n"; +eval("include_once('$dir' . '/test.bla');"); +echo "After eval: $b\n"; +?> +--CLEAN-- + +--EXPECTF-- +Outside of eval: 0.54030230586814 +After allowed eval: 0.28366218546323 +[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call. diff --git a/src/tests/eval_whitelist_user_then_builtin.phpt b/src/tests/eval_whitelist_user_then_builtin.phpt new file mode 100644 index 0000000..8db36fc --- /dev/null +++ b/src/tests/eval_whitelist_user_then_builtin.phpt @@ -0,0 +1,23 @@ +--TEST-- +Eval whitelist - builtin function +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/eval_whitelist.ini +--FILE-- + +--EXPECTF-- +Outside of eval: -0.54402111088937 +[snuffleupagus][0.0.0.0][Eval_whitelist][drop] The function 'sin' isn't in the eval whitelist, dropping its call. -- cgit v1.3