From 32476340c5fd3c76b86487a92fd5c5075342ca99 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 4 Dec 2017 16:09:50 +0100 Subject: Fix the configuration parser wrt. non-matching brackets This validation step is a bit idiotic, but we'll replace it with a proper parser anyway. --- config/examples.ini | 10 ++-------- src/sp_config_utils.c | 9 +++++++-- src/tests/broken_conf_quotes.phpt | 9 +++++++++ src/tests/broken_regexp.phpt | 1 + src/tests/config/broken_conf_quotes.ini | 3 +++ src/tests/example_configuration.phpt | 2 +- 6 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 src/tests/broken_conf_quotes.phpt create mode 100644 src/tests/config/broken_conf_quotes.ini diff --git a/config/examples.ini b/config/examples.ini index 68a363d..664a67a 100644 --- a/config/examples.ini +++ b/config/examples.ini @@ -9,7 +9,6 @@ sp.disable_function.function("system").drop(); # AbanteCart 1.2.8 - Multiple SQL Injections -sp.disable_function.filename("/static_pages/index.php").var("_SERVER[PHP_SELF").value_r("\"").drop().alias("XSS"); sp.disable_function.filename("/core/lib/language_manager.php").function("ALanguageManager>_clone_language_rows").param("from_language").value_r("[^0-9]").drop(); sp.disable_function.filename("/admin/model/tool/backup.php").function("ModelToolBackup>createBackupTask").param("data[table_list]").value_r("'").drop(); @@ -25,7 +24,7 @@ sp.disable_function.filename("/modules/Calendar/Activity.php").function("save_mo # The State of Wordpress Security # All In One WP Security & Firewall -sp.disable_function.filename("/admin/wp-security-dashboard-menu.php").function("render_tab3").var("_REQUEST[tab]]").value_r("\"").drop(); +sp.disable_function.filename("/admin/wp-security-dashboard-menu.php").function("render_tab3").var("_REQUEST[tab]").value_r("\"").drop(); # PHPKit 1.6.6: Code Execution for Privileged Users @@ -33,15 +32,10 @@ sp.disable_function.filename("/pkinc/func/default.php").function("move_uploaded_ # Coppermine 1.5.42: Second-Order Command Execution -sp.disable_function.filename("/include/imageobject_im.class.php").function("exec").var("CONFIG[im_options]).value_r("[^a-z0-9]").drop(); +sp.disable_function.filename("/include/imageobject_im.class.php").function("exec").var("CONFIG[im_options]").value_r("[^a-z0-9]").drop(); sp.disable_function.filename("/forgot_passwd.php").function("cpg_db_query").var("CLEAN[id]").value_r("[^a-z0-9]").drop(); -# CVE-2014-1610 - Mediawiki RCE -sp.disable_function.filename("/includes/media/DjVu.php") -sp.disable_function.filename("/includes/media/ImageHandler.php").var("_GET[page]").value_r("[^0-9]").drop() - - # CVE-2017-1001000 - https://blog.sucuri.net/2017/02/content-injection-vulnerability-wordpress-rest-api.html sp.disable_function.filename("/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php").function("register_routes").var("_GET[id]").value_r("[^0-9]").drop(); sp.disable_function.filename("/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php").function("register_routes").var("_POST[id]").value_r("[^0-9]").drop(); diff --git a/src/sp_config_utils.c b/src/sp_config_utils.c index 71dd373..1a797e5 100644 --- a/src/sp_config_utils.c +++ b/src/sp_config_utils.c @@ -20,7 +20,12 @@ static int validate_str(const char *value) { return -1; } } - return balance != 0; + if (balance != 0) { + sp_log_err("config", "You forgot to close %d bracket%c in the string '%s'", + balance, (balance>1)?'s':' ', value); + return -1; + } + return 0; } int parse_keywords(sp_config_functions *funcs, char *line) { @@ -112,7 +117,7 @@ err: sp_log_err("error", "There is an issue with the parsing of '%s': it doesn't look like a valid string on line %zu.", original_line ? original_line : "NULL", sp_line_no); -} + } line = NULL; return NULL; } diff --git a/src/tests/broken_conf_quotes.phpt b/src/tests/broken_conf_quotes.phpt new file mode 100644 index 0000000..7f754e6 --- /dev/null +++ b/src/tests/broken_conf_quotes.phpt @@ -0,0 +1,9 @@ +--TEST-- +Broken configuration - missing quote +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/broken_conf_quotes.ini +--FILE-- +--EXPECT-- +[snuffleupagus][0.0.0.0][config][error] You forgot to close 1 bracket in the string '_SERVER[PHP_SELF' diff --git a/src/tests/broken_regexp.phpt b/src/tests/broken_regexp.phpt index 3367997..680cf22 100644 --- a/src/tests/broken_regexp.phpt +++ b/src/tests/broken_regexp.phpt @@ -6,4 +6,5 @@ Broken regexp sp.configuration_file={PWD}/config/broken_regexp.ini --FILE-- --EXPECTF-- +[snuffleupagus][0.0.0.0][config][error] You forgot to close 1 bracket in the string '^$[' [snuffleupagus][0.0.0.0][config][error] '.value_r()' is expecting a valid regexp, and not '"^$["' on line 1. diff --git a/src/tests/config/broken_conf_quotes.ini b/src/tests/config/broken_conf_quotes.ini new file mode 100644 index 0000000..7c3b0cd --- /dev/null +++ b/src/tests/config/broken_conf_quotes.ini @@ -0,0 +1,3 @@ +sp.disable_function.filename("static_pages/index.php").var("_SERVER[PHP_SELF").value_r("\"").drop().alias("XSS"); +sp.disable_function.filename("include/imageobject_im.class.php").function("exec").var("CONFIG[im_options]).value_r("[^a-z0-9]").drop(); + diff --git a/src/tests/example_configuration.phpt b/src/tests/example_configuration.phpt index 0bbf59c..b7fec48 100644 --- a/src/tests/example_configuration.phpt +++ b/src/tests/example_configuration.phpt @@ -6,7 +6,7 @@ Shipped configuration sp.configuration_file={PWD}/../../config/examples.ini --FILE-- --EXPECTF-- 0 -- cgit v1.3