From fe43991e3dc6c46e2781d21369f5e268de7baef9 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 10 Oct 2017 18:11:31 +0200 Subject: Implement match on arguments position --- src/sp_config.h | 2 ++ src/sp_config_keywords.c | 7 +++++++ src/sp_disabled_functions.c | 14 ++++++++++---- src/tests/config/disabled_functions_pos.ini | 1 + src/tests/disabled_functions_param_pos.phpt | 12 ++++++++++++ 5 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 src/tests/config/disabled_functions_pos.ini create mode 100644 src/tests/disabled_functions_param_pos.phpt diff --git a/src/sp_config.h b/src/sp_config.h index 0877ebb..c005206 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -77,6 +77,7 @@ typedef struct { char *param; pcre *r_param; sp_php_type param_type; + int pos; char *ret; pcre *r_ret; @@ -183,6 +184,7 @@ typedef struct { #define SP_TOKEN_RET_TYPE ".ret_type(" #define SP_TOKEN_VALUE ".value(" #define SP_TOKEN_VALUE_REGEXP ".value_r(" +#define SP_TOKEN_VALUE_ARG_POS ".pos(" // cookies encryption #define SP_TOKEN_NAME ".cookie(" diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 29f1bfc..1f48852 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -144,7 +144,9 @@ int parse_cookie_encryption(char *line) { int parse_disabled_functions(char *line) { int ret = 0; bool enable = true, disable = false; + char *pos = NULL; sp_disabled_function *df = pecalloc(sizeof(*df), 1, 1); + df->pos = -1; sp_config_functions sp_config_funcs_disabled_functions[] = { {parse_empty, SP_TOKEN_ENABLE, &(enable)}, @@ -169,6 +171,7 @@ int parse_disabled_functions(char *line) { {parse_regexp, SP_TOKEN_RET_REGEXP, &(df->r_ret)}, {parse_php_type, SP_TOKEN_RET_TYPE, &(df->ret_type)}, {parse_str, SP_TOKEN_LOCAL_VAR, &(df->var)}, + {parse_str, SP_TOKEN_VALUE_ARG_POS, &(pos)}, {0}}; ret = parse_keywords(sp_config_funcs_disabled_functions, line); @@ -233,6 +236,10 @@ int parse_disabled_functions(char *line) { return -1; } + if (pos) { + df->pos = atoi(pos) > 128 ? 128: atoi(pos); // FIXME do the strtol dance + } + if (df->function) { df->functions_list = parse_functions_list(df->function); } diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index c8c723a..b05d949 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -178,11 +178,17 @@ bool should_disable(zend_execute_data* execute_data) { } /* Check if we filter on parameter value*/ - if (config_node->param || config_node->r_param) { - const unsigned int nb_param = execute_data->func->common.num_args; + if (config_node->param || config_node->r_param || (config_node->pos != -1)) { + unsigned int nb_param = execute_data->func->common.num_args; bool arg_matched = false; + int i = 0; - for (unsigned int i = 0; i < nb_param; i++) { + if (config_node->pos != -1) {//&& nb_param <= config_node->pos) { + i = config_node->pos; + nb_param = (config_node->pos) + 1; + } + + for (; i < nb_param; i++) { arg_matched = false; if (ZEND_USER_CODE(execute_data->func->type)) { // yay consistency arg_name = ZSTR_VAL(execute_data->func->common.arg_info[i].name); @@ -197,7 +203,7 @@ bool should_disable(zend_execute_data* execute_data) { (true == is_regexp_matching(config_node->r_param, arg_name)); /* This is the parameter name we're looking for. */ - if (true == arg_matching || true == pcre_matching) { + if (true == arg_matching || true == pcre_matching || (config_node->pos != -1)) { zval* arg_value = ZEND_CALL_VAR_NUM(execute_data, i); if (config_node->param_type) { // Are we matching on the `type`? diff --git a/src/tests/config/disabled_functions_pos.ini b/src/tests/config/disabled_functions_pos.ini new file mode 100644 index 0000000..81a524e --- /dev/null +++ b/src/tests/config/disabled_functions_pos.ini @@ -0,0 +1 @@ +sp.disable_functions.function("system").pos("0").value("id").drop(); diff --git a/src/tests/disabled_functions_param_pos.phpt b/src/tests/disabled_functions_param_pos.phpt new file mode 100644 index 0000000..de578b2 --- /dev/null +++ b/src/tests/disabled_functions_param_pos.phpt @@ -0,0 +1,12 @@ +--TEST-- +Disable functions - match on argument's position +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_pos.ini +--FILE-- + +--EXPECTF-- +[snuffleupagus][0.0.0.0][disabled_function][drop] The call to the function 'system' in %a/disabled_functions_param_pos.php:%d has been disabled, because its argument 'command' content (id) matched a rule. -- cgit v1.3 From 26fe379e4ed7b32084fd7b3026a713b8e26f2e29 Mon Sep 17 00:00:00 2001 From: bui Date: Tue, 17 Oct 2017 17:56:01 +0200 Subject: do the strtol dance to make jvoisin happy --- src/sp_config_keywords.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 1f48852..b2e83fe 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -237,7 +237,14 @@ int parse_disabled_functions(char *line) { } if (pos) { - df->pos = atoi(pos) > 128 ? 128: atoi(pos); // FIXME do the strtol dance + errno = 0; + df->pos = strtol(pos, NULL, 10) > 128 ? 128 : strtol(pos, NULL, 10); + if (errno != 0) { + sp_log_err("config", + "Failed to parse arg '%s' of `pos` on line %zu.", + pos, sp_line_no); + return -1; + } } if (df->function) { -- cgit v1.3 From dbfe7cc04adad832d5b37ff1c6ee45767c7c648f Mon Sep 17 00:00:00 2001 From: bui Date: Tue, 17 Oct 2017 18:28:36 +0200 Subject: strtol dance --- src/sp_config_keywords.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index b2e83fe..cbdd579 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -238,8 +238,9 @@ int parse_disabled_functions(char *line) { if (pos) { errno = 0; - df->pos = strtol(pos, NULL, 10) > 128 ? 128 : strtol(pos, NULL, 10); - if (errno != 0) { + char *endptr; + df->pos = strtol(pos, &endptr, 10) > 128 ? 128 : strtol(pos, NULL, 10); + if (errno != 0 || endptr == pos) { sp_log_err("config", "Failed to parse arg '%s' of `pos` on line %zu.", pos, sp_line_no); -- cgit v1.3 From 1e73d00ffaf4cadcaadcd7fae3b3be99305fb439 Mon Sep 17 00:00:00 2001 From: bui Date: Tue, 17 Oct 2017 18:30:39 +0200 Subject: extra tests --- src/tests/config/disabled_functions_invalid_pos.ini | 1 + src/tests/disabled_functions_param_invalid_pos.phpt | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 src/tests/config/disabled_functions_invalid_pos.ini create mode 100644 src/tests/disabled_functions_param_invalid_pos.phpt diff --git a/src/tests/config/disabled_functions_invalid_pos.ini b/src/tests/config/disabled_functions_invalid_pos.ini new file mode 100644 index 0000000..5ba3af3 --- /dev/null +++ b/src/tests/config/disabled_functions_invalid_pos.ini @@ -0,0 +1 @@ +sp.disable_functions.function("system").pos("qwe").value("id").drop(); diff --git a/src/tests/disabled_functions_param_invalid_pos.phpt b/src/tests/disabled_functions_param_invalid_pos.phpt new file mode 100644 index 0000000..91ddd93 --- /dev/null +++ b/src/tests/disabled_functions_param_invalid_pos.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - match on argument's position +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_invalid_pos.ini +--FILE-- + +--EXPECTF-- +[snuffleupagus][0.0.0.0][config][error] Failed to parse arg 'qwe' of `pos` on line 1. +1 + -- cgit v1.3 From 765b7f81a1d157e809889c7224581db70a0ebe53 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 18 Oct 2017 00:31:18 +0200 Subject: Improve the strtol dance --- src/sp_config_keywords.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index cbdd579..b03c28e 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -239,13 +239,17 @@ int parse_disabled_functions(char *line) { if (pos) { errno = 0; char *endptr; - df->pos = strtol(pos, &endptr, 10) > 128 ? 128 : strtol(pos, NULL, 10); + df->pos = strtol(pos, &endptr, 10); if (errno != 0 || endptr == pos) { - sp_log_err("config", - "Failed to parse arg '%s' of `pos` on line %zu.", - pos, sp_line_no); + sp_log_err("config", "Failed to parse arg '%s' of `pos` on line %zu.", + pos, sp_line_no); return -1; } + + // We'll never have a function with more than 128 params + if (df->pos > 128) { + df->pos = 128; + } } if (df->function) { -- cgit v1.3 From 63928badcf79c4f6abcac602a681e2f29ea37894 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 18 Oct 2017 13:19:27 +0200 Subject: Fix a possible mistake --- src/sp_disabled_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index b05d949..cca95ef 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -183,7 +183,7 @@ bool should_disable(zend_execute_data* execute_data) { bool arg_matched = false; int i = 0; - if (config_node->pos != -1) {//&& nb_param <= config_node->pos) { + if ((config_node->pos != -1) && (config_node->pos <= nb_param)) { i = config_node->pos; nb_param = (config_node->pos) + 1; } -- cgit v1.3 From 09a4af846c436398f44cc4057a6b48ef51a85c63 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 18 Oct 2017 13:42:35 +0200 Subject: Add some debug --- src/sp_disabled_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sp_disabled_functions.c b/src/sp_disabled_functions.c index cca95ef..54a1906 100644 --- a/src/sp_disabled_functions.c +++ b/src/sp_disabled_functions.c @@ -179,7 +179,7 @@ bool should_disable(zend_execute_data* execute_data) { /* Check if we filter on parameter value*/ if (config_node->param || config_node->r_param || (config_node->pos != -1)) { - unsigned int nb_param = execute_data->func->common.num_args; + int nb_param = execute_data->func->common.num_args; bool arg_matched = false; int i = 0; -- cgit v1.3 From 76dc2f2d18abadafcb76aa5cae37ec9a73081d32 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 18 Oct 2017 15:30:27 +0200 Subject: Fix the tests --- src/tests/config/disabled_functions_invalid_pos.ini | 2 +- src/tests/config/disabled_functions_pos.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/config/disabled_functions_invalid_pos.ini b/src/tests/config/disabled_functions_invalid_pos.ini index 5ba3af3..42988e4 100644 --- a/src/tests/config/disabled_functions_invalid_pos.ini +++ b/src/tests/config/disabled_functions_invalid_pos.ini @@ -1 +1 @@ -sp.disable_functions.function("system").pos("qwe").value("id").drop(); +sp.disable_function.function("system").pos("qwe").value("id").drop(); diff --git a/src/tests/config/disabled_functions_pos.ini b/src/tests/config/disabled_functions_pos.ini index 81a524e..f96cf3d 100644 --- a/src/tests/config/disabled_functions_pos.ini +++ b/src/tests/config/disabled_functions_pos.ini @@ -1 +1 @@ -sp.disable_functions.function("system").pos("0").value("id").drop(); +sp.disable_function.function("system").pos("0").value("id").drop(); -- cgit v1.3 From 066b79abe9943821240ccc3a458e8ba45b9ee9e8 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 18 Oct 2017 17:04:44 +0200 Subject: `.pos` is mutuaally exclusive with .param and .paran_r --- src/sp_config_keywords.c | 4 ++-- src/tests/broken_conf_mutually_exclusive4.phpt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index b03c28e..097d08b 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -204,10 +204,10 @@ int parse_disabled_functions(char *line) { "'.r_filename' and '.filename' are mutually exclusive on line %zu.", line, sp_line_no); return -1; - } else if (df->r_param && df->param) { + } else if (1 < ((df->r_param?1:0) + (df->param?1:0) + ((-1 != df->pos)?1:0))) { sp_log_err("config", "Invalid configuration line: 'sp.disabled_functions%s':" - "'.r_param' and '.param' are mutually exclusive on line %zu.", + "'.r_param', '.param' and '.pos' are mutually exclusive on line %zu.", line, sp_line_no); return -1; } else if (df->r_ret && df->ret) { diff --git a/src/tests/broken_conf_mutually_exclusive4.phpt b/src/tests/broken_conf_mutually_exclusive4.phpt index 4e888f5..a60f3fa 100644 --- a/src/tests/broken_conf_mutually_exclusive4.phpt +++ b/src/tests/broken_conf_mutually_exclusive4.phpt @@ -6,4 +6,4 @@ Broken configuration sp.configuration_file={PWD}/config/broken_conf_mutually_exclusive4.ini --FILE-- --EXPECT-- -[snuffleupagus][0.0.0.0][config][error] Invalid configuration line: 'sp.disabled_functions.function("system").param("id").value("42").param_r("^id$").drop();':'.r_param' and '.param' are mutually exclusive on line 1. \ No newline at end of file +[snuffleupagus][0.0.0.0][config][error] Invalid configuration line: 'sp.disabled_functions.function("system").param("id").value("42").param_r("^id$").drop();':'.r_param', '.param' and '.pos' are mutually exclusive on line 1. -- cgit v1.3