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.

6 files changed, 23 insertions, 11 deletions

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 <https://blog.ripstech.com/2016/abantecart-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 <https://blog.ripstech.com/2016/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 <https://blog.ripstech.com/2016/phpkit-code-exection-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 <https://blog.ripstech.com/2016/coppermine-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--
+<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
+--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--
 <?php 
-system("echo 0");
+echo 0;
 ?>
 --EXPECTF--
 0