Skip to content

Commit

Permalink
rewrote parameter matching logic. breaks compatibility with previous …
Browse files Browse the repository at this point in the history
…versions.
  • Loading branch information
bef committed Feb 18, 2021
1 parent 0152871 commit fb9b378
Show file tree
Hide file tree
Showing 16 changed files with 220 additions and 86 deletions.
157 changes: 79 additions & 78 deletions src/sp_disabled_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ char* get_complete_function_path(zend_execute_data const* const execute_data) {
} else {
complete_path_function = estrdup(function_name);
}
sp_log_debug("%s", complete_path_function);

return complete_path_function;
}

Expand Down Expand Up @@ -98,107 +100,105 @@ static bool is_local_var_matching(
return false;
}

static inline const char* get_fn_arg_name(zend_function *fn, uint32_t i) {
if (fn->type == ZEND_USER_FUNCTION || (fn->common.fn_flags & ZEND_ACC_USER_ARG_INFO)) {
return ZSTR_VAL(fn->op_array.arg_info[i].name);
} else {
return fn->internal_function.arg_info[i].name;
}
}

static bool is_param_matching(zend_execute_data* execute_data,
sp_disabled_function const* const config_node,
const zend_string* builtin_param,
const char* builtin_param_name,
const char** arg_name,
const zend_string** arg_value_str) {
int nb_param = ZEND_CALL_NUM_ARGS(execute_data);
int i = 0;
zval* arg_value;

if (config_node->pos != -1) {
if (config_node->pos > nb_param - 1) {
char* complete_function_path = get_complete_function_path(execute_data);
sp_log_warn("config",
"It seems that you wrote a rule filtering on the "
"%d%s argument of the function '%s', but it takes only %d "
"arguments. "
"Matching on _all_ arguments instead.",
config_node->pos, GET_SUFFIX(config_node->pos),
complete_function_path, nb_param);
efree(complete_function_path);
} else {
i = config_node->pos;
nb_param = (config_node->pos) + 1;
}
}

// builtin functions
if (builtin_param) {
/* We're matching on a language construct (here named "builtin"),
* and they can only take a single argument, but PHP considers them
* differently than functions arguments. */
* and they can only take a single argument, but PHP considers them
* differently than functions arguments. */
*arg_name = builtin_param_name;
*arg_value_str = builtin_param;
return sp_match_value(builtin_param, config_node->value,
config_node->r_value);
} else if (config_node->r_param || config_node->pos != -1) {
// We're matching on a function (and not a language construct)
for (; i < nb_param; i++) {
if (ZEND_USER_CODE(execute_data->func->type)) { // yay consistency
*arg_name = ZSTR_VAL(execute_data->func->common.arg_info[i].name);
} else {
*arg_name = execute_data->func->internal_function.arg_info[i].name;
}
const bool pcre_matching =
config_node->r_param &&
(true == sp_is_regexp_matching(config_node->r_param, *arg_name));
}

/* This is the parameter name we're looking for. */
if (true == pcre_matching || config_node->pos != -1) {
arg_value = ZEND_CALL_ARG(execute_data, i + 1);
// safeguards
if (!execute_data || !execute_data->func) {
sp_log_debug("no execute data -> silently ignore parameter matching");
return false;
}

if (config_node->param_type) { // Are we matching on the `type`?
if (config_node->param_type == Z_TYPE_P(arg_value)) {
return true;
}
} else if (Z_TYPE_P(arg_value) == IS_ARRAY) {
*arg_value_str = sp_zval_to_zend_string(arg_value);
if (config_node->key || config_node->r_key) {
if (sp_match_array_key(arg_value, config_node->key,
config_node->r_key)) {
return true;
}
} else if (sp_match_array_value(arg_value, config_node->value,
config_node->r_value)) {
return true;
}
} else {
*arg_value_str = sp_zval_to_zend_string(arg_value);
if (sp_match_value(*arg_value_str, config_node->value,
config_node->r_value)) {
return true;
}
}
}
*arg_name = NULL;
int call_num_args = EX_NUM_ARGS();
zend_function *fn = execute_data->func;
int fn_num_args = fn->common.num_args;

if (!call_num_args) {
sp_log_debug("no call arguments -> return");
return false; // no arguments to check
}

if (config_node->pos > call_num_args - 1 || config_node->pos > fn_num_args) {
// trying to match argument beyond last given argument OR beyond last declared argument.
// this is perfectly normal for functions with
// (a) optional arguments
// (b) excess arguments
// (c) variadic arguments which are not supported
return false;
}

zval* arg_value = NULL;

if (config_node->pos > -1) {
if (config_node->pos < fn_num_args) {
*arg_name = get_fn_arg_name(fn, config_node->pos);
}
arg_value = ZEND_CALL_ARG(execute_data, config_node->pos + 1);
} else if (config_node->param) {
*arg_name = config_node->param->value;
arg_value = sp_get_var_value(execute_data, config_node->param, true);
} else if (config_node->r_param) {
for (int i = 0; i < call_num_args; i++) {
*arg_name = get_fn_arg_name(fn, i);
if (true == sp_is_regexp_matching(config_node->r_param, *arg_name)) {
arg_value = ZEND_CALL_ARG(execute_data, i + 1);
}
}
}

if (!arg_value) {
sp_log_debug("no argument match -> return");
return false;
}

if (arg_value) {
*arg_value_str = sp_zval_to_zend_string(arg_value);
if (config_node->param_type) { // Are we matching on the `type`?
if (config_node->param_type == Z_TYPE_P(arg_value)) {
return true;
}
} else if (Z_TYPE_P(arg_value) == IS_ARRAY) {
if (config_node->key || config_node->r_key) {
if (sp_match_array_key(arg_value, config_node->key,
config_node->r_key)) {
return true;
}
} else if (sp_match_array_value(arg_value, config_node->value,
config_node->r_value)) {
return true;
}
} else if (sp_match_value(*arg_value_str, config_node->value,
config_node->r_value)) {
if (config_node->param_type) {
if (config_node->param_type == Z_TYPE_P(arg_value)) {
if (!(config_node->key || config_node->r_key || config_node->value || config_node->r_value)) { // Are we matching on the `type` only?
sp_log_debug("arg type match only.");
return true;
}
} else {
sp_log_debug("arg type mismatch -> return");
return false;
}
}

*arg_value_str = sp_zval_to_zend_string(arg_value);
if (Z_TYPE_P(arg_value) == IS_ARRAY) {
if (config_node->key || config_node->r_key) {
if (sp_match_array_key(arg_value, config_node->key, config_node->r_key)) {
return true;
}
} else if (sp_match_array_value(arg_value, config_node->value, config_node->r_value)) {
return true;
}
} else if (sp_match_value(*arg_value_str, config_node->value, config_node->r_value)) {
return true;
}

return false;
}

Expand Down Expand Up @@ -287,6 +287,7 @@ static void should_disable(zend_execute_data* execute_data,
const sp_list_node* config,
const zend_string* current_filename) {
char current_file_hash[SHA256_SIZE * 2 + 1] = {0};
// sp_log_debug("%s %s %s", complete_function_path, builtin_param, builtin_param_name);

while (config) {
sp_disabled_function const* const config_node =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sp.disable_function.function("foo_excess_args").pos("3").value("blubb").drop()
12 changes: 12 additions & 0 deletions src/tests/disable_function/config/disabled_function_named_args.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
sp.disable_function.function("foo_named_args_pos").pos("0").value("bob").drop()
sp.disable_function.function("foo_named_args_param").param("name").value("bob").drop()

sp.disable_function.function("foo_named_args_ooo_pos").pos("0").value("bob").drop()
sp.disable_function.function("foo_named_args_ooo_param").param("name").value("bob").drop()

sp.disable_function.function("foo_named_args_ooo_opt_pos").pos("2").value("green").drop()
sp.disable_function.function("foo_named_args_ooo_opt_param").param("color").value("green").drop()

sp.disable_function.function("foo_named_args_skip_pos").pos("2").value("green").drop()
sp.disable_function.function("foo_named_args_skip_param").param("color").value("green").drop()

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
sp.disable_function.function("system").pos("1337").value("id").drop();
sp.disable_function.function("system").pos("0").value("id").drop();
sp.disable_function.function("system").pos("1").param_type("ARRAY").alias("1").drop();
sp.disable_function.function("system").pos("0").param_type("ARRAY").alias("1").drop();
sp.disable_function.function("strtoupper").pos("0").value("id").alias("strlen array").drop();
14 changes: 14 additions & 0 deletions src/tests/disable_function/disabled_function_excess_args.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with excess arguments
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_excess_args.ini
--FILE--
<?php
function foo_excess_args($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_excess_args("bob", "hi", "green", "blubb");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_excess_args' in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with out-of-order named optional arguments by matching argument name
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_ooo_opt_param($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_ooo_opt_param("bob", color: "green", greeting: "xxx");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_opt_param', because its argument '$color'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with out-of-order named optional arguments by matching argument position
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_ooo_opt_pos($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_ooo_opt_pos("bob", color: "green", greeting: "xxx");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_opt_pos', because its argument 'color'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with out-of-order named arguments by matching argument name
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_ooo_param($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_ooo_param(greeting: "hello!", name: "bob");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_param', because its argument '$name'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with out-of-order named arguments by matching argument position
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_ooo_pos($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_ooo_pos(greeting: "hello!", name: "bob");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_ooo_pos', because its argument 'name'%s matched a rule in %s.php on line %d
14 changes: 14 additions & 0 deletions src/tests/disable_function/disabled_function_named_args_param.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with named arguments by matching argument name
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_param($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_param(name: "bob", greeting: "hello!");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_param', because its argument '$name'%s matched a rule in %s.php on line %d
14 changes: 14 additions & 0 deletions src/tests/disable_function/disabled_function_named_args_pos.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with named arguments by matching argument position
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_pos($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_pos(name: "bob", greeting: "hello!");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_pos', because its argument 'name'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with named arguments (skipping opt. args) by matching argument name
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_skip_param($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_skip_param("bob", color: "green");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_skip_param', because its argument '$color'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Disable functions with named arguments (skipping opt. args) by matching argument position
--SKIPIF--
<?php if (!extension_loaded("snuffleupagus") || PHP_VERSION_ID < 80000) print "skip"; ?>
--INI--
sp.configuration_file={PWD}/config/disabled_function_named_args.ini
--FILE--
<?php
function foo_named_args_skip_pos($name, $greeting='HI!', $color='red') {
echo "boo\n";
}
foo_named_args_skip_pos("bob", color: "green");
--EXPECTF--
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'foo_named_args_skip_pos', because its argument 'color'%s matched a rule in %s.php on line %d
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ echo strcmp([1,23], "pouet") . "\n";
--EXPECTF--
0

Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'strcmp', because its argument '$str1' content (ARRAY) matched a rule in %a/disabled_functions_name_type.php on line 3
Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'strcmp', because its argument '$str1'%smatched a rule in %s/disabled_functions_name_type.php on line 3
2 changes: 0 additions & 2 deletions src/tests/disable_function/disabled_functions_param_pos.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,4 @@ sp.configuration_file={PWD}/config/disabled_functions_pos.ini
system("id");
?>
--EXPECTF--
Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1337th argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_param_pos.php on line 2

Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument 'command' content (id) matched a rule in %a/disabled_functions_param_pos.php on line %d
4 changes: 0 additions & 4 deletions src/tests/disable_function/disabled_functions_pos_type.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,4 @@ sp.configuration_file={PWD}/config/disabled_functions_pos.ini
system([123, 456]);
?>
--EXPECTF--
Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1337th argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_pos_type.php on line %d

Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you wrote a rule filtering on the 1st argument of the function 'system', but it takes only 1 arguments. Matching on _all_ arguments instead. in %a/disabled_functions_pos_type.php on line %d

Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'system', because its argument 'command' content (?) matched the rule '1' in %a/disabled_functions_pos_type.php on line %d

0 comments on commit fb9b378

Please sign in to comment.