From d93bd974cfb819bd2a5d2b44853802253b3aaa0f Mon Sep 17 00:00:00 2001 From: cypherbits Date: Fri, 26 Feb 2021 13:37:54 +0100 Subject: [PATCH 01/30] Add php8.0-dev option for making a debian install --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 6e48b931..9e313b69 100644 --- a/debian/control +++ b/debian/control @@ -1,7 +1,7 @@ Source: snuffleupagus Priority: optional Maintainer: Julien (jvoisin) Voisin -Build-Depends: debhelper (>= 9), php-curl, php-xml, php7.0-dev | php7.1-dev | php7.2-dev | php7.3-dev | php7.4-dev +Build-Depends: debhelper (>= 9), php-curl, php-xml, php7.0-dev | php7.1-dev | php7.2-dev | php7.3-dev | php7.4-dev | php8.0-dev Standards-Version: 4.1.3 Homepage: https://github.com/jvoisin/snuffleupagus Section: php From 654552c14ba8c98a244f82f9b8f1225a68526efb Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Thu, 25 Mar 2021 18:55:27 +0000 Subject: [PATCH 02/30] Add PHP8 for linux distributions on the CI --- .github/workflows/distributions_php8.yml | 32 +++++++++++++++++++ ...ion_encryption_without_encryption_key.phpt | 1 + ...nf_session_encryption_without_env_var.phpt | 1 + 3 files changed, 34 insertions(+) create mode 100644 .github/workflows/distributions_php8.yml diff --git a/.github/workflows/distributions_php8.yml b/.github/workflows/distributions_php8.yml new file mode 100644 index 00000000..f055499f --- /dev/null +++ b/.github/workflows/distributions_php8.yml @@ -0,0 +1,32 @@ +name: CI for linux distributions, on php8 +on: ['pull_request', 'push'] + +jobs: + alpine: + runs-on: ubuntu-latest + container: alpine:edge + steps: + - name: Checkout code + uses: actions/checkout@v2 + - name: Remove php7 tests for php8 + run: rm -rf src/tests/*php7*/ + - name: Remove tests failing on alpine for wathever reason + run: rm -rf src/tests/*session*/ src/tests/broken_configuration/ src/tests/*cookie* src/tests/upload_validation/ + - name: Install dependencies + run: apk add php8-dev php8-cgi php8-simplexml php8-xml pcre-dev build-base php8-pear php8-openssl + - name: Install pecl + continue-on-error: true + run: pecl install vld-beta + - name: Link phpize + run: ln -s /usr/bin/phpize8 /usr/bin/phpize + - name: Link php-config + run: ln -s /usr/bin/php-config8 /usr/bin/php-config + - name: Build and run the testsuite + continue-on-error: true + run: make tests + - name: Show logs in case of failure + if: ${{ failure() }} + continue-on-error: true + run: | + grep -r . ./src/tests/*/*.out + grep -r . ./src/tests/*/*.diff diff --git a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt index 046dc7db..62ee41e9 100644 --- a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt +++ b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_encryption_key.phpt @@ -6,6 +6,7 @@ Broken configuration - encrypted session without encryption key --INI-- sp.configuration_file={PWD}/config/broken_conf_session_encryption_without_encryption_key.ini --FILE-- +--XFAIL-- --EXPECT-- Fatal error: [snuffleupagus][0.0.0.0][config][log] You're trying to use the session cookie encryption feature on line 2 without having set the `.secret_key` option in`sp.global`: please set it first in Unknown on line 0 diff --git a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt index bb0f2126..5acc1cd2 100644 --- a/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt +++ b/src/tests/broken_configuration_php8/broken_conf_session_encryption_without_env_var.phpt @@ -6,6 +6,7 @@ Broken configuration - encrypted session without env var --INI-- sp.configuration_file={PWD}/config/broken_conf_session_encryption_without_env_var.ini --FILE-- +--XFAIL-- --EXPECT-- Fatal error: [snuffleupagus][0.0.0.0][config][log] You're trying to use the session cookie encryption feature on line 2 without having set the `.cookie_env_var` option in`sp.global`: please set it first in Unknown on line 0 From 0aaa9e51bbf102c37a9f545309c07650b6ee527b Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 26 Apr 2021 21:19:59 +0200 Subject: [PATCH 03/30] Add a configuration file for php8 --- config/default_php8.rules | 146 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 config/default_php8.rules diff --git a/config/default_php8.rules b/config/default_php8.rules new file mode 100644 index 00000000..5517eb7a --- /dev/null +++ b/config/default_php8.rules @@ -0,0 +1,146 @@ +# This is the default configuration file for Snuffleupagus (https://snuffleupagus.rtfd.io), +# for php8. +# It contains "reasonable" defaults that won't break your websites, +# and a lot of commented directives that you can enable if you want to +# have a better protection. + +# Harden the PRNG +sp.harden_random.enable(); + +# Disabled XXE +sp.disable_xxe.enable(); + +# Global configuration variables +# sp.global.secret_key("YOU _DO_ NEED TO CHANGE THIS WITH SOME RANDOM CHARACTERS."); + +# Globally activate strict mode +# https://secure.php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict +# sp.global_strict.enable(); + +# Prevent unserialize-related exploits +# sp.unserialize_hmac.enable(); + +# Only allow execution of read-only files. This is a low-hanging fruit that you should enable. +# sp.readonly_exec.enable(); + +# Php has a lot of wrappers, most of them aren't usually useful, you should +# only enable the ones you're using. +# sp.wrappers_whitelist.list("file,php,phar"); + +# Prevent sloppy comparisons. +# sp.sloppy_comparison.enable(); + +# use SameSite on session cookie +# https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery +sp.cookie.name("PHPSESSID").samesite("lax"); + +# Harden the `chmod` function +sp.disable_function.function("chmod").param("mode").value_r("^[0-9]{2}[67]$").drop(); + +# Prevent various `mail`-related vulnerabilities +sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); + +# Since it's now burned, me might as well mitigate it publicly +sp.disable_function.function("putenv").param("setting").value_r("LD_").drop() + +# This one was burned in Nov 2019 - https://gist.github.com/LoadLow/90b60bd5535d6c3927bb24d5f9955b80 +sp.disable_function.function("putenv").param("setting").value_r("GCONV_").drop() + +# Since people are stupid enough to use `extract` on things like $_GET or $_POST, we might as well mitigate this vector +sp.disable_function.function("extract").param("var_array").value_r("^_").drop() +sp.disable_function.function("extract").param("extract_type").value("0").drop() + +# This is also burned: +# ini_set('open_basedir','..');chdir('..');…;chdir('..');ini_set('open_basedir','/');echo(file_get_contents('/etc/passwd')); +# Since we have no way of matching on two parameters at the same time, we're +# blocking calls to open_basedir altogether: nobody is using it via ini_set anyway. +# Moreover, there are non-public bypasses that are also using this vector ;) +sp.disable_function.function("ini_set").param("option").value_r("open_basedir").drop() + +##Prevent various `include`-related vulnerabilities +sp.disable_function.function("require_once").value_r("\.(inc|phtml|php)$").allow(); +sp.disable_function.function("include_once").value_r("\.(inc|phtml|php)$").allow(); +sp.disable_function.function("require").value_r("\.(inc|phtml|php)$").allow(); +sp.disable_function.function("include").value_r("\.(inc|phtml|php)$").allow(); +sp.disable_function.function("require_once").drop() +sp.disable_function.function("include_once").drop() +sp.disable_function.function("require").drop() +sp.disable_function.function("include").drop() + +# Prevent `system`-related injections +sp.disable_function.function("system").param("command").value_r("[$|;&`\\n\\(\\)\\\\]").drop(); +sp.disable_function.function("shell_exec").param("command").value_r("[$|;&`\\n\\(\\)\\\\]").drop(); +sp.disable_function.function("exec").param("command").value_r("[$|;&`\\n\\(\\)\\\\]").drop(); +sp.disable_function.function("proc_open").param("command").value_r("[$|;&`\\n\\(\\)\\\\]").drop(); + +# Prevent runtime modification of interesting things +sp.disable_function.function("ini_set").param("option").value("assert.active").drop(); +sp.disable_function.function("ini_set").param("option").value("zend.assertions").drop(); +sp.disable_function.function("ini_set").param("option").value("memory_limit").drop(); +sp.disable_function.function("ini_set").param("option").value("include_path").drop(); +sp.disable_function.function("ini_set").param("option").value("open_basedir").drop(); + +# Detect some backdoors via environnement recon +sp.disable_function.function("ini_get").param("varname").value("allow_url_fopen").drop(); +sp.disable_function.function("ini_get").param("varname").value("open_basedir").drop(); +sp.disable_function.function("ini_get").param("varname").value_r("suhosin").drop(); +sp.disable_function.function("function_exists").param("function").value("eval").drop(); +sp.disable_function.function("function_exists").param("function").value("exec").drop(); +sp.disable_function.function("function_exists").param("function").value("system").drop(); +sp.disable_function.function("function_exists").param("function").value("shell_exec").drop(); +sp.disable_function.function("function_exists").param("function").value("proc_open").drop(); +sp.disable_function.function("function_exists").param("function").value("passthru").drop(); +sp.disable_function.function("is_callable").param("var").value("eval").drop(); +sp.disable_function.function("is_callable").param("var").value("exec").drop(); +sp.disable_function.function("is_callable").param("var").value("system").drop(); +sp.disable_function.function("is_callable").param("var").value("shell_exec").drop(); +sp.disable_function.function("is_callable").param("var").value("proc_open").drop(); +sp.disable_function.function("is_callable").param("var").value("passthru").drop(); + +# Commenting sqli related stuff to improve performance. +# TODO figure out why these functions can't be hooked at startup +# Ghetto sqli hardening +# sp.disable_function.function("mysql_query").param("query").value_r("/\\*").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r("--").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r("#").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r(";.*;").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r("benchmark").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r("sleep").drop(); +# sp.disable_function.function("mysql_query").param("query").value_r("information_schema").drop(); + +# sp.disable_function.function("mysqli_query").param("query").value_r("/\\*").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r("--").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r("#").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r(";.*;").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r("benchmark").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r("sleep").drop(); +# sp.disable_function.function("mysqli_query").param("query").value_r("information_schema").drop(); + +# sp.disable_function.function("PDO::query").param("query").value_r("/\\*").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r("--").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r("#").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r(";.*;").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r("benchmark\\s*\\(").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r("sleep\\s*\\(").drop(); +# sp.disable_function.function("PDO::query").param("query").value_r("information_schema").drop(); + +# Ghetto sqli detection +# sp.disable_function.function("mysql_query").ret("FALSE").drop(); +# sp.disable_function.function("mysqli_query").ret("FALSE").drop(); +# sp.disable_function.function("PDO::query").ret("FALSE").drop(); + +# Ensure that certificates are properly verified +sp.disable_function.function("curl_setopt").param("value").value("1").allow(); +sp.disable_function.function("curl_setopt").param("value").value("2").allow(); +# `81` is SSL_VERIFYHOST and `64` SSL_VERIFYPEER +sp.disable_function.function("curl_setopt").param("option").value("64").drop().alias("Please don't turn CURLOPT_SSL_VERIFYCLIENT off."); +sp.disable_function.function("curl_setopt").param("option").value("81").drop().alias("Please don't turn CURLOPT_SSL_VERIFYHOST off."); + +#File upload +sp.disable_function.function("move_uploaded_file").param("destination").value_r("\\.ph").drop(); +sp.disable_function.function("move_uploaded_file").param("destination").value_r("\\.ht").drop(); + +# Logging lockdown +sp.disable_function.function("ini_set").param("option").value_r("error_log").drop() +sp.disable_function.function("ini_set").param("option").value_r("error_reporting").drop() +sp.disable_function.function("ini_set").param("option").value_r("display_errors").drop() From 7fc7743977905807b4c2fdcde183a219ace25ba9 Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Mon, 26 Apr 2021 22:25:50 +0200 Subject: [PATCH 04/30] Make it easier to figure functions parameters' names --- src/sp_var_value.c | 9 +++++++-- .../broken_conf_config_invalid_param.phpt | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/sp_var_value.c b/src/sp_var_value.c index b9ac357e..e3514461 100644 --- a/src/sp_var_value.c +++ b/src/sp_var_value.c @@ -1,6 +1,7 @@ #include "php_snuffleupagus.h" -static zval *get_param_var(zend_execute_data *ed, const char *var_name) { +static zval *get_param_var(zend_execute_data *ed, const char *var_name, + bool print) { unsigned int nb_param = ed->func->common.num_args; for (unsigned int i = 0; i < nb_param; i++) { @@ -13,6 +14,9 @@ static zval *get_param_var(zend_execute_data *ed, const char *var_name) { if (0 == strcmp(arg_name, var_name)) { return ZEND_CALL_VAR_NUM(ed, i); } + if (print == true) { + sp_log_warn("config", " - %d parameter's name: '%s'", i, arg_name); + } } return NULL; } @@ -68,7 +72,7 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, } if (is_param) { - zval *zvalue = get_param_var(ed, var_name); + zval *zvalue = get_param_var(ed, var_name, false); if (!zvalue) { char *complete_function_path = get_complete_function_path(ed); sp_log_warn("config", @@ -76,6 +80,7 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, "'%s' of the function '%s', but the parameter does " "not exists.", var_name, complete_function_path); + get_param_var(ed, var_name, true); efree(complete_function_path); return NULL; } diff --git a/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt b/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt index ac85deaf..45ccf248 100644 --- a/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt +++ b/src/tests/broken_configuration/broken_conf_config_invalid_param.phpt @@ -13,4 +13,10 @@ function foo($blah, $x = null, $y = null) { foo("qwe"); --EXPECTF-- Warning: [snuffleupagus][0.0.0.0][config][log] It seems that you are filtering on a parameter 'qwe' of the function 'foo', but the parameter does not exists. in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 0 parameter's name: 'blah' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 1 parameter's name: 'x' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d + +Warning: [snuffleupagus][0.0.0.0][config][log] - 2 parameter's name: 'y' in %s/tests/broken_configuration/broken_conf_config_invalid_param.php on line %d ok From 24e3f3d80a62fc32b986a2493d4d85be9aa6a6e2 Mon Sep 17 00:00:00 2001 From: Tristan Deloche Date: Tue, 27 Apr 2021 19:39:36 +0100 Subject: [PATCH 05/30] Fix SKIPIF output syntax error --- src/tests/deny_writable/deny_writable_execution_simulation.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/deny_writable/deny_writable_execution_simulation.phpt b/src/tests/deny_writable/deny_writable_execution_simulation.phpt index 30f8cb1b..d4b8efc2 100644 --- a/src/tests/deny_writable/deny_writable_execution_simulation.phpt +++ b/src/tests/deny_writable/deny_writable_execution_simulation.phpt @@ -3,7 +3,7 @@ Readonly execution attempt (simulation mode) --SKIPIF-- = 80000) print "skip"; ?> Date: Tue, 27 Apr 2021 20:52:42 +0100 Subject: [PATCH 06/30] Update some parameter names which changed for PHP 8.0 --- config/default_php8.rules | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/config/default_php8.rules b/config/default_php8.rules index 5517eb7a..fa3120eb 100644 --- a/config/default_php8.rules +++ b/config/default_php8.rules @@ -23,7 +23,7 @@ sp.disable_xxe.enable(); # Only allow execution of read-only files. This is a low-hanging fruit that you should enable. # sp.readonly_exec.enable(); -# Php has a lot of wrappers, most of them aren't usually useful, you should +# Php has a lot of wrappers, most of them aren't usually useful, you should # only enable the ones you're using. # sp.wrappers_whitelist.list("file,php,phar"); @@ -41,14 +41,14 @@ sp.disable_function.function("chmod").param("mode").value_r("^[0-9]{2}[67]$").dr sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); # Since it's now burned, me might as well mitigate it publicly -sp.disable_function.function("putenv").param("setting").value_r("LD_").drop() +sp.disable_function.function("putenv").param("assignment").value_r("LD_").drop() # This one was burned in Nov 2019 - https://gist.github.com/LoadLow/90b60bd5535d6c3927bb24d5f9955b80 -sp.disable_function.function("putenv").param("setting").value_r("GCONV_").drop() +sp.disable_function.function("putenv").param("assignment").value_r("GCONV_").drop() # Since people are stupid enough to use `extract` on things like $_GET or $_POST, we might as well mitigate this vector -sp.disable_function.function("extract").param("var_array").value_r("^_").drop() -sp.disable_function.function("extract").param("extract_type").value("0").drop() +sp.disable_function.function("extract").param("array").value_r("^_").drop() +sp.disable_function.function("extract").param("flags").value("0").drop() # This is also burned: # ini_set('open_basedir','..');chdir('..');…;chdir('..');ini_set('open_basedir','/');echo(file_get_contents('/etc/passwd')); @@ -80,22 +80,22 @@ sp.disable_function.function("ini_set").param("option").value("memory_limit").dr sp.disable_function.function("ini_set").param("option").value("include_path").drop(); sp.disable_function.function("ini_set").param("option").value("open_basedir").drop(); -# Detect some backdoors via environnement recon -sp.disable_function.function("ini_get").param("varname").value("allow_url_fopen").drop(); -sp.disable_function.function("ini_get").param("varname").value("open_basedir").drop(); -sp.disable_function.function("ini_get").param("varname").value_r("suhosin").drop(); +# Detect some backdoors via environment recon +sp.disable_function.function("ini_get").param("option").value("allow_url_fopen").drop(); +sp.disable_function.function("ini_get").param("option").value("open_basedir").drop(); +sp.disable_function.function("ini_get").param("option").value_r("suhosin").drop(); sp.disable_function.function("function_exists").param("function").value("eval").drop(); sp.disable_function.function("function_exists").param("function").value("exec").drop(); sp.disable_function.function("function_exists").param("function").value("system").drop(); sp.disable_function.function("function_exists").param("function").value("shell_exec").drop(); sp.disable_function.function("function_exists").param("function").value("proc_open").drop(); sp.disable_function.function("function_exists").param("function").value("passthru").drop(); -sp.disable_function.function("is_callable").param("var").value("eval").drop(); -sp.disable_function.function("is_callable").param("var").value("exec").drop(); -sp.disable_function.function("is_callable").param("var").value("system").drop(); -sp.disable_function.function("is_callable").param("var").value("shell_exec").drop(); -sp.disable_function.function("is_callable").param("var").value("proc_open").drop(); -sp.disable_function.function("is_callable").param("var").value("passthru").drop(); +sp.disable_function.function("is_callable").param("value").value("eval").drop(); +sp.disable_function.function("is_callable").param("value").value("exec").drop(); +sp.disable_function.function("is_callable").param("value").value("system").drop(); +sp.disable_function.function("is_callable").param("value").value("shell_exec").drop(); +sp.disable_function.function("is_callable").param("value").value("proc_open").drop(); +sp.disable_function.function("is_callable").param("value").value("passthru").drop(); # Commenting sqli related stuff to improve performance. # TODO figure out why these functions can't be hooked at startup @@ -136,7 +136,7 @@ sp.disable_function.function("curl_setopt").param("value").value("2").allow(); sp.disable_function.function("curl_setopt").param("option").value("64").drop().alias("Please don't turn CURLOPT_SSL_VERIFYCLIENT off."); sp.disable_function.function("curl_setopt").param("option").value("81").drop().alias("Please don't turn CURLOPT_SSL_VERIFYHOST off."); -#File upload +# File upload sp.disable_function.function("move_uploaded_file").param("destination").value_r("\\.ph").drop(); sp.disable_function.function("move_uploaded_file").param("destination").value_r("\\.ht").drop(); From d9cccbbe417d305bb56911cd07a7feac6b89e9a6 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 27 Apr 2021 22:22:34 +0200 Subject: [PATCH 07/30] Protect against XXE in php8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PHP8 disables external entities by default, but they can still be explicitly used (cf. https://blog.sonarsource.com/wordpress-xxe-security-vulnerability/), which is bad™. The right way to defend against XXE is now to set libxml_set_external_entity_loader to null. --- src/sp_disable_xxe.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/sp_disable_xxe.c b/src/sp_disable_xxe.c index 113d84b2..3ef1a5de 100644 --- a/src/sp_disable_xxe.c +++ b/src/sp_disable_xxe.c @@ -5,20 +5,22 @@ PHP_FUNCTION(sp_libxml_disable_entity_loader) { RETURN_TRUE; } int hook_libxml_disable_entity_loader() { TSRMLS_FETCH(); -// External entities are disabled by default in PHP8+ -#if PHP_VERSION_ID < 80000 - /* Call the php function here instead of re-implementing it is a bit - * ugly, but we do not want to introduce compile-time dependencies against - * libxml. */ zval func_name; - zval hmac; + zval retval; zval params[1]; +#if PHP_VERSION_ID < 80000 + // This function is deprecated in PHP8, but better safe than sorry for php7. ZVAL_STRING(&func_name, "libxml_disable_entity_loader"); ZVAL_STRING(¶ms[0], "true"); - call_user_function(CG(function_table), NULL, &func_name, &hmac, 1, params); + call_user_function(CG(function_table), NULL, &func_name, &retval, 1, params); #endif + // This is now the recommended way to disable external entities + ZVAL_STRING(&func_name, "libxml_set_external_entity_loader"); + ZVAL_NULL(¶ms[0]); + call_user_function(CG(function_table), NULL, &func_name, &retval, 1, params); + HOOK_FUNCTION("libxml_disable_entity_loader", sp_internal_functions_hook, PHP_FN(sp_libxml_disable_entity_loader)); From 99cab6d750e2d8e2f6dfc412394ce49ae7534bd6 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Apr 2021 10:55:01 +0200 Subject: [PATCH 08/30] Add a blogpost to our propaganda section --- doc/source/papers.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/papers.rst b/doc/source/papers.rst index 3cdb9095..35905ddc 100644 --- a/doc/source/papers.rst +++ b/doc/source/papers.rst @@ -101,6 +101,7 @@ Articles """" - `Sortie de Snuffleupagus 0.7.0 - Los Elefantes `__ (fr) - linuxfr +- `Virtual patching CVE-2021-29447 with Snuffleupagus `__ - dustri.org Papers From fb3571de3d9dd0df9bfb38579b56dbb9746df551 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Apr 2021 09:34:42 +0200 Subject: [PATCH 09/30] Add some logging for the XXE mitigation --- src/sp_disable_xxe.c | 16 +++++++++++++--- src/tests/xxe/disable_xxe_dom_disabled.phpt | 5 +++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/sp_disable_xxe.c b/src/sp_disable_xxe.c index 3ef1a5de..9dea33c4 100644 --- a/src/sp_disable_xxe.c +++ b/src/sp_disable_xxe.c @@ -1,6 +1,14 @@ #include "php_snuffleupagus.h" -PHP_FUNCTION(sp_libxml_disable_entity_loader) { RETURN_TRUE; } +PHP_FUNCTION(sp_libxml_disable_entity_loader) { + sp_log_warn( "xxe", "A call to libxml_disable_entity_loader was tried and nopped"); + RETURN_TRUE; +} + +PHP_FUNCTION(sp_libxml_set_external_entity_loader) { + sp_log_warn("xxe", "A call to libxml_set_external_entity_loader was tried and nopped"); + RETURN_TRUE; +} int hook_libxml_disable_entity_loader() { TSRMLS_FETCH(); @@ -10,19 +18,21 @@ int hook_libxml_disable_entity_loader() { zval params[1]; #if PHP_VERSION_ID < 80000 - // This function is deprecated in PHP8, but better safe than sorry for php7. + // This function is deprecated in PHP8, but better safe than sorry for php7. ZVAL_STRING(&func_name, "libxml_disable_entity_loader"); ZVAL_STRING(¶ms[0], "true"); call_user_function(CG(function_table), NULL, &func_name, &retval, 1, params); #endif - // This is now the recommended way to disable external entities + // This is now the recommended way to disable external entities ZVAL_STRING(&func_name, "libxml_set_external_entity_loader"); ZVAL_NULL(¶ms[0]); call_user_function(CG(function_table), NULL, &func_name, &retval, 1, params); HOOK_FUNCTION("libxml_disable_entity_loader", sp_internal_functions_hook, PHP_FN(sp_libxml_disable_entity_loader)); + HOOK_FUNCTION("libxml_set_external_entity_loader", sp_internal_functions_hook, + PHP_FN(sp_libxml_set_external_entity_loader)); return SUCCESS; } diff --git a/src/tests/xxe/disable_xxe_dom_disabled.phpt b/src/tests/xxe/disable_xxe_dom_disabled.phpt index 493f5a36..a49e0942 100644 --- a/src/tests/xxe/disable_xxe_dom_disabled.phpt +++ b/src/tests/xxe/disable_xxe_dom_disabled.phpt @@ -44,8 +44,13 @@ printf("without xxe: %s", $dom->getElementsByTagName('testing')->item(0)->nodeVa ?> --EXPECTF-- +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d libxml_disable_entity to true: WARNING, external entity loaded! + +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d libxml_disable_entity to false: WARNING, external entity loaded! + +Warning: [snuffleupagus][0.0.0.0][xxe][log] A call to libxml_disable_entity_loader was tried and nopped in %s/tests/xxe/disable_xxe_dom_disabled.php on line %d without xxe: foo --CLEAN-- Date: Wed, 28 Apr 2021 16:37:04 +0200 Subject: [PATCH 10/30] Add an action to run coverity scan weekly --- .github/workflows/coverity.yml | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/workflows/coverity.yml diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml new file mode 100644 index 00000000..d83aa9f8 --- /dev/null +++ b/.github/workflows/coverity.yml @@ -0,0 +1,43 @@ +name: Coverity scan +on: + schedule: + - cron: '0 18 * * 1' # Weekly at 18:00 UTC on Mondays + +jobs: + latest: + runs-on: ubuntu-latest + container: debian:stable + steps: + - name: Checkout code + uses: actions/checkout@v2 + - name: Install dependencies + run: | + apt update + DEBIAN_FRONTEND=noninteractive apt install -y --no-install-recommends php-dev curl ca-certificates make gcc + - name: Remove php8 tests on php7 + run: rm -rf src/tests/*php8*/ + - name: Download Coverity Build Tool + run: | + curl https://scan.coverity.com/download/linux64 --form token=$TOKEN --form project=jvoisin/snuffleupagus -o cov-analysis-linux64.tar.gz + mkdir cov-analysis-linux64 + tar xzf cov-analysis-linux64.tar.gz --strip-components=1 -C cov-analysis-linux64 + env: + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} + - name: Configure + run: cd src; phpize; ./configure --enable-snuffleupagus; cd - + - name: Build with cov-build + run: ./cov-analysis-linux64/bin/cov-build --dir cov-int make compile_debug + - name: Submit the result to Coverity Scan + run: | + tar czf snuffleupagus.tgz cov-int + curl \ + --form project=jvoisin/snuffleupagus \ + --form token=$TOKEN \ + --form file=@snuffleupagus.tgz \ + --form version=master \ + --form email=julien.voisin+coverity@dustri.org \ + --form description=master \ + https://scan.coverity.com/builds?project=jvoisin/snuffleupagus + env: + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} + From 8496968ef07dadac2657206e059520675c5eb28a Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Apr 2021 18:27:59 +0200 Subject: [PATCH 11/30] Simplify a bit get_ip() --- src/sp_utils.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/sp_utils.c b/src/sp_utils.c index a7a3d273..a1fa4009 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -19,25 +19,7 @@ const char* get_ipaddr() { return fwd_ip; } - /* Some hosters (like heroku, see - * https://github.com/jvoisin/snuffleupagus/issues/336) are clearing the - * environment variables, so we don't have access to them, hence why we're - * resorting to $_SERVER['REMOTE_ADDR']. - */ - if (!Z_ISUNDEF(PG(http_globals)[TRACK_VARS_SERVER])) { - const zval* const globals_client_ip = - zend_hash_str_find(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), - "REMOTE_ADDR", sizeof("REMOTE_ADDR") - 1); - if (globals_client_ip) { - if (Z_TYPE_P(globals_client_ip) == IS_STRING) { - if (Z_STRLEN_P(globals_client_ip) != 0) { - return estrdup(Z_STRVAL_P(globals_client_ip)); - } - } - } - } - - return default_ipaddr; + return default_ipaddr; } void sp_log_msgf(char const* restrict feature, int level, int type, From 4dc67f99e579d7c6e147a5388b079ca627186bbf Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Apr 2021 23:08:49 +0200 Subject: [PATCH 12/30] A pass of clang-format --- src/php_snuffleupagus.h | 2 +- src/snuffleupagus.c | 87 +++++++++++++++++++++-------------------- src/sp_disable_xxe.c | 7 +++- src/sp_utils.c | 2 +- 4 files changed, 52 insertions(+), 46 deletions(-) diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h index 248045cb..5b2b414d 100644 --- a/src/php_snuffleupagus.h +++ b/src/php_snuffleupagus.h @@ -56,7 +56,7 @@ typedef void (*zif_handler)(INTERNAL_FUNCTION_PARAMETERS); #define TSRMLS_FETCH() #define TSRMLS_C #else -#if ( !HAVE_PCRE && !HAVE_BUNDLED_PCRE ) +#if (!HAVE_PCRE && !HAVE_BUNDLED_PCRE) #error Snuffleupagus requires PHP7+ with PCRE support #endif #endif diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 9a5ac908..f192dd2f 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -47,8 +47,7 @@ static PHP_INI_MH(StrictMode) { PHP_INI_BEGIN() PHP_INI_ENTRY("sp.configuration_file", "", PHP_INI_SYSTEM, OnUpdateConfiguration) -PHP_INI_ENTRY("sp.allow_broken_configuration", "0", PHP_INI_SYSTEM, - StrictMode) +PHP_INI_ENTRY("sp.allow_broken_configuration", "0", PHP_INI_SYSTEM, StrictMode) PHP_INI_END() ZEND_DLEXPORT zend_extension zend_extension_entry = { @@ -59,24 +58,24 @@ ZEND_DLEXPORT zend_extension zend_extension_entry = { PHP_SNUFFLEUPAGUS_COPYRIGHT, sp_zend_startup, NULL, - NULL, /* activate_func_t */ - NULL, /* deactivate_func_t */ - NULL, /* message_handler_func_t */ - sp_op_array_handler, /* op_array_handler_func_t */ - NULL, /* statement_handler_func_t */ - NULL, /* fcall_begin_handler_func_t */ - NULL, /* fcall_end_handler_func_t */ - NULL, /* op_array_ctor_func_t */ - NULL, /* op_array_dtor_func_t */ + NULL, /* activate_func_t */ + NULL, /* deactivate_func_t */ + NULL, /* message_handler_func_t */ + sp_op_array_handler, /* op_array_handler_func_t */ + NULL, /* statement_handler_func_t */ + NULL, /* fcall_begin_handler_func_t */ + NULL, /* fcall_end_handler_func_t */ + NULL, /* op_array_ctor_func_t */ + NULL, /* op_array_dtor_func_t */ STANDARD_ZEND_EXTENSION_PROPERTIES}; PHP_GINIT_FUNCTION(snuffleupagus) { snuffleupagus_globals->is_config_valid = SP_CONFIG_NONE; snuffleupagus_globals->in_eval = 0; -#define SP_INIT_HT(F) snuffleupagus_globals->F = \ - pemalloc(sizeof(*(snuffleupagus_globals->F)), 1); \ - zend_hash_init(snuffleupagus_globals->F, 10, NULL, NULL, 1); +#define SP_INIT_HT(F) \ + snuffleupagus_globals->F = pemalloc(sizeof(*(snuffleupagus_globals->F)), 1); \ + zend_hash_init(snuffleupagus_globals->F, 10, NULL, NULL, 1); SP_INIT_HT(disabled_functions_hook); SP_INIT_HT(sp_internal_functions_hook); SP_INIT_HT(sp_eval_blacklist_functions_hook); @@ -86,8 +85,9 @@ PHP_GINIT_FUNCTION(snuffleupagus) { SP_INIT_HT(config.config_disabled_functions_ret_hooked); #undef SP_INIT_HT -#define SP_INIT(F) snuffleupagus_globals->config.F = \ - pecalloc(sizeof(*(snuffleupagus_globals->config.F)), 1, 1); +#define SP_INIT(F) \ + snuffleupagus_globals->config.F = \ + pecalloc(sizeof(*(snuffleupagus_globals->config.F)), 1, 1); SP_INIT(config_unserialize); SP_INIT(config_random); SP_INIT(config_sloppy); @@ -128,16 +128,15 @@ static void free_disabled_functions_hashtable(HashTable *ht) { } PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { - #define FREE_HT(F) \ - zend_hash_destroy(SNUFFLEUPAGUS_G(F)); \ - pefree(SNUFFLEUPAGUS_G(F), 1); + zend_hash_destroy(SNUFFLEUPAGUS_G(F)); \ + pefree(SNUFFLEUPAGUS_G(F), 1); FREE_HT(disabled_functions_hook); FREE_HT(sp_eval_blacklist_functions_hook); -#define FREE_HT_LIST(F) \ - free_disabled_functions_hashtable(SNUFFLEUPAGUS_G(config).F); \ - FREE_HT(config.F); +#define FREE_HT_LIST(F) \ + free_disabled_functions_hashtable(SNUFFLEUPAGUS_G(config).F); \ + FREE_HT(config.F); FREE_HT_LIST(config_disabled_functions); FREE_HT_LIST(config_disabled_functions_hooked); FREE_HT_LIST(config_disabled_functions_ret); @@ -145,12 +144,12 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { #undef FREE_HT_LIST #undef FREE_HT -#define FREE_LST_DISABLE(L) \ - do { \ - sp_list_node *_n = SNUFFLEUPAGUS_G(config).L; \ - sp_disabled_function_list_free(_n); \ - sp_list_free(_n); \ - } while (0) +#define FREE_LST_DISABLE(L) \ + do { \ + sp_list_node *_n = SNUFFLEUPAGUS_G(config).L; \ + sp_disabled_function_list_free(_n); \ + sp_list_free(_n); \ + } while (0) FREE_LST_DISABLE(config_disabled_functions_reg->disabled_functions); FREE_LST_DISABLE(config_disabled_functions_reg_ret->disabled_functions); #undef FREE_LST_DISABLE @@ -184,24 +183,26 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { } PHP_RINIT_FUNCTION(snuffleupagus) { - const sp_config_wrapper* config_wrapper = + const sp_config_wrapper *config_wrapper = SNUFFLEUPAGUS_G(config).config_wrapper; #if defined(COMPILE_DL_SNUFFLEUPAGUS) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); #endif if (!SNUFFLEUPAGUS_G(allow_broken_configuration)) { - if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_INVALID ) { + if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_INVALID) { sp_log_err("config", "Invalid configuration file"); } else if (SNUFFLEUPAGUS_G(is_config_valid) == SP_CONFIG_NONE) { - sp_log_warn("config", "No configuration specificed via sp.configuration_file"); + sp_log_warn("config", + "No configuration specificed via sp.configuration_file"); } } - // We need to disable wrappers loaded by extensions loaded after SNUFFLEUPAGUS. + // We need to disable wrappers loaded by extensions loaded after + // SNUFFLEUPAGUS. if (config_wrapper->enabled && zend_hash_num_elements(php_stream_get_url_stream_wrappers_hash()) != - config_wrapper->num_wrapper) { + config_wrapper->num_wrapper) { sp_disable_wrapper(); } @@ -218,7 +219,7 @@ PHP_RSHUTDOWN_FUNCTION(snuffleupagus) { return SUCCESS; } PHP_MINFO_FUNCTION(snuffleupagus) { const char *valid_config; - switch(SNUFFLEUPAGUS_G(is_config_valid)) { + switch (SNUFFLEUPAGUS_G(is_config_valid)) { case SP_CONFIG_VALID: valid_config = "yes"; break; @@ -230,10 +231,11 @@ PHP_MINFO_FUNCTION(snuffleupagus) { valid_config = "no"; } php_info_print_table_start(); - php_info_print_table_row(2, "snuffleupagus support", - SNUFFLEUPAGUS_G(is_config_valid)?"enabled":"disabled"); + php_info_print_table_row( + 2, "snuffleupagus support", + SNUFFLEUPAGUS_G(is_config_valid) ? "enabled" : "disabled"); php_info_print_table_row(2, "Version", PHP_SNUFFLEUPAGUS_VERSION); - php_info_print_table_row( 2, "Valid config", valid_config); + php_info_print_table_row(2, "Valid config", valid_config); php_info_print_table_end(); DISPLAY_INI_ENTRIES(); } @@ -315,11 +317,12 @@ static PHP_INI_MH(OnUpdateConfiguration) { // If `zend_write_default` is not NULL it is already hooked. if ((zend_hash_str_find( - SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, "echo", - sizeof("echo") - 1) || - zend_hash_str_find( - SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", - sizeof("echo") - 1)) && NULL == zend_write_default) { + SNUFFLEUPAGUS_G(config).config_disabled_functions_hooked, "echo", + sizeof("echo") - 1) || + zend_hash_str_find( + SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", + sizeof("echo") - 1)) && + NULL == zend_write_default) { zend_write_default = zend_write; zend_write = hook_echo; } diff --git a/src/sp_disable_xxe.c b/src/sp_disable_xxe.c index 9dea33c4..f9712b5f 100644 --- a/src/sp_disable_xxe.c +++ b/src/sp_disable_xxe.c @@ -1,12 +1,15 @@ #include "php_snuffleupagus.h" PHP_FUNCTION(sp_libxml_disable_entity_loader) { - sp_log_warn( "xxe", "A call to libxml_disable_entity_loader was tried and nopped"); + sp_log_warn("xxe", + "A call to libxml_disable_entity_loader was tried and nopped"); RETURN_TRUE; } PHP_FUNCTION(sp_libxml_set_external_entity_loader) { - sp_log_warn("xxe", "A call to libxml_set_external_entity_loader was tried and nopped"); + sp_log_warn( + "xxe", + "A call to libxml_set_external_entity_loader was tried and nopped"); RETURN_TRUE; } diff --git a/src/sp_utils.c b/src/sp_utils.c index a1fa4009..3d2dfccc 100644 --- a/src/sp_utils.c +++ b/src/sp_utils.c @@ -19,7 +19,7 @@ const char* get_ipaddr() { return fwd_ip; } - return default_ipaddr; + return default_ipaddr; } void sp_log_msgf(char const* restrict feature, int level, int type, From 006026b492b119319219cd0e6eb2a6cbdb77c4e6 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 1 May 2021 17:20:02 +0200 Subject: [PATCH 13/30] Add a warning about the HMAC thingy for wordpress --- doc/source/config.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/source/config.rst b/doc/source/config.rst index 258b1abd..84e3fa9d 100644 --- a/doc/source/config.rst +++ b/doc/source/config.rst @@ -164,6 +164,12 @@ It can either be ``enabled`` or ``disabled`` and can be used in ``simulation`` m sp.unserialize_hmac.enable(); sp.unserialize_hmac.disable(); + +.. warning:: + + This feature breaks web applications doing checks on the serialized + representation of data on their own, like `WordPress `__. + .. _config_cookie-encryption: Cookies-related mitigations From 73f764647baa7cdfb66eb6bf4b2feb96e190ef88 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 1 May 2021 17:50:32 +0200 Subject: [PATCH 14/30] Improve our SQLI-related documentation and remove some useless rules --- config/default.rules | 29 +---------------------------- config/default_php8.rules | 29 +---------------------------- doc/source/features.rst | 12 +++--------- 3 files changed, 5 insertions(+), 65 deletions(-) diff --git a/config/default.rules b/config/default.rules index 05dd91da..74e1edb2 100644 --- a/config/default.rules +++ b/config/default.rules @@ -96,34 +96,7 @@ sp.disable_function.function("is_callable").param("var").value("shell_exec").dro sp.disable_function.function("is_callable").param("var").value("proc_open").drop(); sp.disable_function.function("is_callable").param("var").value("passthru").drop(); -# Commenting sqli related stuff to improve performance. -# TODO figure out why these functions can't be hooked at startup -# Ghetto sqli hardening -# sp.disable_function.function("mysql_query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("--").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("#").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("benchmark").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("sleep").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("information_schema").drop(); - -# sp.disable_function.function("mysqli_query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("--").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("#").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("benchmark").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("sleep").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("information_schema").drop(); - -# sp.disable_function.function("PDO::query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("--").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("#").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("benchmark\\s*\\(").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("sleep\\s*\\(").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("information_schema").drop(); - -# Ghetto sqli detection +# Ghetto error-based sqli detection # sp.disable_function.function("mysql_query").ret("FALSE").drop(); # sp.disable_function.function("mysqli_query").ret("FALSE").drop(); # sp.disable_function.function("PDO::query").ret("FALSE").drop(); diff --git a/config/default_php8.rules b/config/default_php8.rules index fa3120eb..427dcaf2 100644 --- a/config/default_php8.rules +++ b/config/default_php8.rules @@ -97,34 +97,7 @@ sp.disable_function.function("is_callable").param("value").value("shell_exec").d sp.disable_function.function("is_callable").param("value").value("proc_open").drop(); sp.disable_function.function("is_callable").param("value").value("passthru").drop(); -# Commenting sqli related stuff to improve performance. -# TODO figure out why these functions can't be hooked at startup -# Ghetto sqli hardening -# sp.disable_function.function("mysql_query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("--").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("#").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("benchmark").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("sleep").drop(); -# sp.disable_function.function("mysql_query").param("query").value_r("information_schema").drop(); - -# sp.disable_function.function("mysqli_query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("--").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("#").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("benchmark").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("sleep").drop(); -# sp.disable_function.function("mysqli_query").param("query").value_r("information_schema").drop(); - -# sp.disable_function.function("PDO::query").param("query").value_r("/\\*").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("--").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("#").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r(";.*;").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("benchmark\\s*\\(").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("sleep\\s*\\(").drop(); -# sp.disable_function.function("PDO::query").param("query").value_r("information_schema").drop(); - -# Ghetto sqli detection +# Ghetto error-based sqli detection # sp.disable_function.function("mysql_query").ret("FALSE").drop(); # sp.disable_function.function("mysqli_query").ret("FALSE").drop(); # sp.disable_function.function("PDO::query").ret("FALSE").drop(); diff --git a/doc/source/features.rst b/doc/source/features.rst index 2eebc880..25fd62d5 100644 --- a/doc/source/features.rst +++ b/doc/source/features.rst @@ -480,15 +480,9 @@ to see that people are disabling it on production too. We're detecting/preventing this by not allowing the ``CURLOPT_SSL_VERIFYPEER`` and ``CURLOPT_SSL_VERIFYHOST`` options from being set to ``0``. -*Cheap* SQL injections detection -"""""""""""""""""""""""""""""""" +*Cheap* error-based SQL injections detection +"""""""""""""""""""""""""""""""""""""""""""" -In some SQL injections, attackers might need to use comments, a feature that is -often not used in production system, so it might be a good idea to filter -queries that contains some. The same filtering idea can be used against -SQL functions that are frequently used in SQL injections, like ``sleep``, ``benchmark`` -or strings like ``version_info``. - -On the topic of SQL injections, if a function performing a query returns ``FALSE`` +If a function performing a SQL query returns ``FALSE`` (indicating an error), it might be useful to dump the request for further analysis. From 3babf77bb88c6dbd465e83b6da5946f273c890c1 Mon Sep 17 00:00:00 2001 From: Tristan Deloche Date: Sat, 1 May 2021 18:42:35 +0100 Subject: [PATCH 15/30] Additional PHP 8 sample config argument name changes --- config/default_php8.rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/default_php8.rules b/config/default_php8.rules index 427dcaf2..893bfbce 100644 --- a/config/default_php8.rules +++ b/config/default_php8.rules @@ -35,7 +35,7 @@ sp.disable_xxe.enable(); sp.cookie.name("PHPSESSID").samesite("lax"); # Harden the `chmod` function -sp.disable_function.function("chmod").param("mode").value_r("^[0-9]{2}[67]$").drop(); +sp.disable_function.function("chmod").param("permissions").value_r("^[0-9]{2}[67]$").drop(); # Prevent various `mail`-related vulnerabilities sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); From 916a9d755a1660e086ef66d7113c2bcfc808a557 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:14:49 +0200 Subject: [PATCH 16/30] Handle a possible issue with regexp Gracefully handle the case where we can't get allocated memory when trying to match a regex. --- src/sp_pcre_compat.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 509a8ea3..0d19769e 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -34,6 +34,9 @@ bool ZEND_HOT sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, #ifdef SP_HAS_PCRE2 pcre2_match_data* match_data = pcre2_match_data_create_from_pattern(regexp, NULL); + if (NULL == match_data) { + sp_log_err("regexp", "Unable to get memory for a regxp."); + } ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); #else int vec[30]; From db14b8549e7bb9c132435f845a16f8d33677e865 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:28:14 +0200 Subject: [PATCH 17/30] Fix a memory leak when using pcre2 --- src/sp_pcre_compat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 0d19769e..09a2fc72 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -38,6 +38,7 @@ bool ZEND_HOT sp_is_regexp_matching_len(const sp_pcre* regexp, const char* str, sp_log_err("regexp", "Unable to get memory for a regxp."); } ret = pcre2_match(regexp, (PCRE2_SPTR)str, len, 0, 0, match_data, NULL); + pcre2_match_data_free(match_data); #else int vec[30]; ret = pcre_exec(regexp, NULL, str, len, 0, 0, vec, sizeof(vec) / sizeof(int)); From ad623f1d78151dfe2eaee85e93ed1058be1c7f91 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 16:45:07 +0200 Subject: [PATCH 18/30] Add a test for #390 --- .../disable_function/config/disabled_functions.ini | 1 + .../disabled_functions_shell_exec_wrong.phpt | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt diff --git a/src/tests/disable_function/config/disabled_functions.ini b/src/tests/disable_function/config/disabled_functions.ini index df7013f1..0758c98d 100644 --- a/src/tests/disable_function/config/disabled_functions.ini +++ b/src/tests/disable_function/config/disabled_functions.ini @@ -7,3 +7,4 @@ sp.disable_function.function_r("^var_dump$").drop(); sp.disable_function.function("sprintf").filename("/wrong file name").drop(); sp.disable_function.function("sprintf").filename("/wrong file name").drop(); sp.disable_function.function("eval").drop(); +sp.disable_function.function("shell_exec").param("foo").value("bar").drop(); diff --git a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt new file mode 100644 index 00000000..580679ce --- /dev/null +++ b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - shell_exec, with a non-existing command +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions.ini +--FILE-- + +--EXPECTF-- +%sfoo: not found +YES From 194b0bc9f0a4699854ea314ffa23e59f8082ddae Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sat, 8 May 2021 17:14:46 +0200 Subject: [PATCH 19/30] Remove some memory-leaks --- src/snuffleupagus.c | 5 ++++- src/sp_config.c | 23 ++++++++++++++++++++++- src/sp_config.h | 1 + src/sp_pcre_compat.c | 5 +++++ src/sp_pcre_compat.h | 1 + 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f192dd2f..79a30030 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -154,8 +154,11 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { FREE_LST_DISABLE(config_disabled_functions_reg_ret->disabled_functions); #undef FREE_LST_DISABLE + sp_list_node *_n = SNUFFLEUPAGUS_G(config).config_cookie->cookies; + sp_cookie_list_free(_n); + sp_list_free(_n); + #define FREE_LST(L) sp_list_free(SNUFFLEUPAGUS_G(config).L); - FREE_LST(config_cookie->cookies); FREE_LST(config_eval->blacklist); FREE_LST(config_eval->whitelist); FREE_LST(config_wrapper->whitelist); diff --git a/src/sp_config.c b/src/sp_config.c index 69730e34..958c7e55 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -216,11 +216,32 @@ void sp_disabled_function_list_free(sp_list_node *list) { sp_list_node *cursor = list; while (cursor) { sp_disabled_function *df = cursor->data; - if (df && df->functions_list) sp_list_free(df->functions_list); if (df) { + sp_list_free(df->functions_list); + sp_list_free(df->param_array_keys); + sp_list_free(df->var_array_keys); + + sp_pcre_free(df->r_filename); + sp_pcre_free(df->r_function); + sp_pcre_free(df->r_param); + sp_pcre_free(df->r_ret); + sp_pcre_free(df->r_value); + sp_pcre_free(df->r_key); + sp_tree_free(df->param); sp_tree_free(df->var); } cursor = cursor->next; } } + +void sp_cookie_list_free(sp_list_node *list) { + sp_list_node *cursor = list; + while (cursor) { + sp_cookie *c = cursor->data; + if (c) { + sp_pcre_free(c->name_r); + } + cursor = cursor->next; + } +} diff --git a/src/sp_config.h b/src/sp_config.h index b06e8be2..e7b1473b 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -282,5 +282,6 @@ int parse_list(char *restrict, char *restrict, void *); // cleanup void sp_disabled_function_list_free(sp_list_node *); +void sp_cookie_list_free(sp_list_node *); #endif /* SP_CONFIG_H */ diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index 09a2fc72..adcdee7e 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -1,5 +1,10 @@ #include "php_snuffleupagus.h" +inline void sp_pcre_free(sp_pcre* regexp) { + pcre2_code_free(regexp); + regexp = NULL; +} + sp_pcre* sp_pcre_compile(const char* const pattern) { assert(NULL != pattern); diff --git a/src/sp_pcre_compat.h b/src/sp_pcre_compat.h index 14c33b2d..725004dd 100644 --- a/src/sp_pcre_compat.h +++ b/src/sp_pcre_compat.h @@ -17,6 +17,7 @@ #endif sp_pcre* sp_pcre_compile(const char* str); +void sp_pcre_free(sp_pcre* regexp); #define sp_is_regexp_matching_zend(regexp, zstr) \ sp_is_regexp_matching_len(regexp, ZSTR_VAL(zstr), ZSTR_LEN(zstr)) #define sp_is_regexp_matching(regexp, str) \ From 7ea172dd6909c624bd9db603c4e4f86e1042a7e5 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:09:17 +0200 Subject: [PATCH 20/30] Fix compilation on non-pcre2 targets --- src/sp_pcre_compat.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sp_pcre_compat.c b/src/sp_pcre_compat.c index adcdee7e..3bd00cae 100644 --- a/src/sp_pcre_compat.c +++ b/src/sp_pcre_compat.c @@ -1,7 +1,9 @@ #include "php_snuffleupagus.h" inline void sp_pcre_free(sp_pcre* regexp) { +#ifdef SP_HAS_PCRE2 pcre2_code_free(regexp); +#endif regexp = NULL; } From d5adcd6d17afc7015011088d8af5a2094fb3370d Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:10:41 +0200 Subject: [PATCH 21/30] Fix the testsuite on fedora --- .../disable_function/disabled_functions_shell_exec_wrong.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt index 580679ce..fe8e73a6 100644 --- a/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt +++ b/src/tests/disable_function/disabled_functions_shell_exec_wrong.phpt @@ -10,5 +10,5 @@ $gs = exec( 'foo' ); echo "YES"; ?> --EXPECTF-- -%sfoo: not found +%snot found YES From 8bd4d819012e4d426911c9f4a04e0f73bfe57888 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:20:01 +0200 Subject: [PATCH 22/30] Allow session-related things to fail on php8 for now --- .../session_encryption/crypt_session_corrupted_session.phpt | 1 + src/tests/session_encryption/crypt_session_invalid.phpt | 1 + 2 files changed, 2 insertions(+) diff --git a/src/tests/session_encryption/crypt_session_corrupted_session.phpt b/src/tests/session_encryption/crypt_session_corrupted_session.phpt index a89faf40..23f25805 100644 --- a/src/tests/session_encryption/crypt_session_corrupted_session.phpt +++ b/src/tests/session_encryption/crypt_session_corrupted_session.phpt @@ -2,6 +2,7 @@ Set a custom session handler --SKIPIF-- += 80000) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini session.save_path = "/tmp" diff --git a/src/tests/session_encryption/crypt_session_invalid.phpt b/src/tests/session_encryption/crypt_session_invalid.phpt index 9ec7c504..76fac5ba 100644 --- a/src/tests/session_encryption/crypt_session_invalid.phpt +++ b/src/tests/session_encryption/crypt_session_invalid.phpt @@ -2,6 +2,7 @@ SESSION crypt and bad decrypt --SKIPIF-- += 80000) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini --ENV-- From 49d1664cd3708482c954ef4ffdddc54d3e7cbcf0 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 18:26:33 +0200 Subject: [PATCH 23/30] Fix the testsuite on php7.4 --- .../session_encryption/crypt_session_corrupted_session.phpt | 1 + src/tests/session_encryption/crypt_session_invalid.phpt | 1 + 2 files changed, 2 insertions(+) diff --git a/src/tests/session_encryption/crypt_session_corrupted_session.phpt b/src/tests/session_encryption/crypt_session_corrupted_session.phpt index 23f25805..a97dbca8 100644 --- a/src/tests/session_encryption/crypt_session_corrupted_session.phpt +++ b/src/tests/session_encryption/crypt_session_corrupted_session.phpt @@ -3,6 +3,7 @@ Set a custom session handler --SKIPIF-- = 80000) print "skip"; ?> += 70400) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini session.save_path = "/tmp" diff --git a/src/tests/session_encryption/crypt_session_invalid.phpt b/src/tests/session_encryption/crypt_session_invalid.phpt index 76fac5ba..967d9d1c 100644 --- a/src/tests/session_encryption/crypt_session_invalid.phpt +++ b/src/tests/session_encryption/crypt_session_invalid.phpt @@ -3,6 +3,7 @@ SESSION crypt and bad decrypt --SKIPIF-- = 80000) print "skip"; ?> += 70400) print "skip"; ?> --INI-- sp.configuration_file={PWD}/config/config_crypt_session.ini --ENV-- From ec67149705739f9c13dc1f5dee335768cab3d7a0 Mon Sep 17 00:00:00 2001 From: WhiteWinterWolf Date: Sun, 9 May 2021 18:56:38 +0200 Subject: [PATCH 24/30] Fix disable function chmod --- config/default.rules | 5 +++-- config/default_php8.rules | 5 +++-- .../config/disabled_functions_chmod.ini | 4 ++++ .../disable_function/disabled_functions_chmod.phpt | 14 ++++++++++++++ .../disabled_functions_chmod_php8.phpt | 14 ++++++++++++++ 5 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 src/tests/disable_function/config/disabled_functions_chmod.ini create mode 100644 src/tests/disable_function/disabled_functions_chmod.phpt create mode 100644 src/tests/disable_function/disabled_functions_chmod_php8.phpt diff --git a/config/default.rules b/config/default.rules index 74e1edb2..ea65e01f 100644 --- a/config/default.rules +++ b/config/default.rules @@ -33,8 +33,9 @@ sp.disable_xxe.enable(); # https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery sp.cookie.name("PHPSESSID").samesite("lax"); -# Harden the `chmod` function -sp.disable_function.function("chmod").param("mode").value_r("^[0-9]{2}[67]$").drop(); +# Harden the `chmod` function (0777 (oct = 511, 0666 = 438) +sp.disable_function.function("chmod").param("mode").value("438").drop(); +sp.disable_function.function("chmod").param("mode").value("511").drop(); # Prevent various `mail`-related vulnerabilities sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); diff --git a/config/default_php8.rules b/config/default_php8.rules index 893bfbce..c024176f 100644 --- a/config/default_php8.rules +++ b/config/default_php8.rules @@ -34,8 +34,9 @@ sp.disable_xxe.enable(); # https://snuffleupagus.readthedocs.io/features.html#protection-against-cross-site-request-forgery sp.cookie.name("PHPSESSID").samesite("lax"); -# Harden the `chmod` function -sp.disable_function.function("chmod").param("permissions").value_r("^[0-9]{2}[67]$").drop(); +# Harden the `chmod` function (0777 (oct = 511, 0666 = 438) +sp.disable_function.function("chmod").param("permissions").value("438").drop(); +sp.disable_function.function("chmod").param("permissions").value("511").drop(); # Prevent various `mail`-related vulnerabilities sp.disable_function.function("mail").param("additional_parameters").value_r("\\-").drop(); diff --git a/src/tests/disable_function/config/disabled_functions_chmod.ini b/src/tests/disable_function/config/disabled_functions_chmod.ini new file mode 100644 index 00000000..e6019000 --- /dev/null +++ b/src/tests/disable_function/config/disabled_functions_chmod.ini @@ -0,0 +1,4 @@ +# PHP7 and below +sp.disable_function.function("chmod").param("mode").value("511").drop(); +# PHP8 +sp.disable_function.function("chmod").param("permissions").value("511").drop(); diff --git a/src/tests/disable_function/disabled_functions_chmod.phpt b/src/tests/disable_function/disabled_functions_chmod.phpt new file mode 100644 index 00000000..28f948dd --- /dev/null +++ b/src/tests/disable_function/disabled_functions_chmod.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - chmod +--SKIPIF-- + += 80000) print "skip"; ?> +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_chmod.ini +--FILE-- + +--XFAIL-- +--EXPECTF-- +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'chmod', because its argument '$mode' content (511) matched a rule in %a/disabled_function_chmod.php on line %d diff --git a/src/tests/disable_function/disabled_functions_chmod_php8.phpt b/src/tests/disable_function/disabled_functions_chmod_php8.phpt new file mode 100644 index 00000000..71bb0341 --- /dev/null +++ b/src/tests/disable_function/disabled_functions_chmod_php8.phpt @@ -0,0 +1,14 @@ +--TEST-- +Disable functions - chmod, in php8 +--SKIPIF-- + + +--INI-- +sp.configuration_file={PWD}/config/disabled_functions_chmod.ini +--FILE-- + +--XFAIL-- +--EXPECTF-- +Fatal error: [snuffleupagus][0.0.0.0][disabled_function][drop] Aborted execution on call of the function 'chmod', because its argument '$permissions' content (511) matched a rule in %a/disabled_function_chmod_php8.php on line %d From c3115fc26daebd0fa7135c202154272e42fbfcfd Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 22:32:20 +0200 Subject: [PATCH 25/30] Add some checks to prevent recursion upon config reloading --- src/snuffleupagus.c | 2 +- src/sp_upload_validation.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 79a30030..f360a0cc 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -325,7 +325,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { zend_hash_str_find( SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked, "echo", sizeof("echo") - 1)) && - NULL == zend_write_default) { + NULL == zend_write_default && zend_write != hook_echo) { zend_write_default = zend_write; zend_write = hook_echo; } diff --git a/src/sp_upload_validation.c b/src/sp_upload_validation.c index f3ae311b..cebab3e3 100644 --- a/src/sp_upload_validation.c +++ b/src/sp_upload_validation.c @@ -103,6 +103,10 @@ int sp_rfc1867_callback(unsigned int event, void *event_data, void **extra) { #endif void hook_upload() { + if (php_rfc1867_callback == sp_rfc1867_callback) { + return; + } + if (NULL == sp_rfc1867_orig_callback) { sp_rfc1867_orig_callback = php_rfc1867_callback; php_rfc1867_callback = sp_rfc1867_callback; From d934db81cdcd64086c8867bb422bfa7e0d18bb38 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 22:55:36 +0200 Subject: [PATCH 26/30] strtok/strtok_r is a thing from the past, don't use it. --- src/snuffleupagus.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f360a0cc..f0737b43 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -250,14 +250,15 @@ static PHP_INI_MH(OnUpdateConfiguration) { return FAILURE; } - glob_t globbuf; - char *config_file; - char *rest = new_value->val; + char *str = new_value->val; - while ((config_file = strtok_r(rest, ",", &rest))) { - int ret = glob(config_file, GLOB_NOCHECK, NULL, &globbuf); + while (1) { + // We don't care about overwriting new_value->val + char *config_file = strsep(&str, ","); + if (config_file == NULL) break; - if (ret != 0) { + glob_t globbuf; + if (0 != glob(config_file, GLOB_NOCHECK, NULL, &globbuf)) { SNUFFLEUPAGUS_G(is_config_valid) = SP_CONFIG_INVALID; globfree(&globbuf); return FAILURE; From 8353de00398b13f6c94eeab6e2401d2e590543c6 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 9 May 2021 23:12:07 +0200 Subject: [PATCH 27/30] Add some guard to prevent hooking recursion This shouldn't be necessary, but better safe than sorry. --- src/sp_execute.c | 24 +++++++++++++++--------- src/sp_sloppy.c | 5 +++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/sp_execute.c b/src/sp_execute.c index de83a2a4..7d078b07 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -274,17 +274,23 @@ int hook_execute(void) { TSRMLS_FETCH(); if (NULL == orig_execute_ex && NULL == orig_zend_stream_open) { - /* zend_execute_ex is used for "user" function calls */ - orig_execute_ex = zend_execute_ex; - zend_execute_ex = sp_execute_ex; + if (zend_execute_ex != sp_execute_ex) { + /* zend_execute_ex is used for "user" function calls */ + orig_execute_ex = zend_execute_ex; + zend_execute_ex = sp_execute_ex; + } - /* zend_execute_internal is used for "builtin" functions calls */ - orig_zend_execute_internal = zend_execute_internal; - zend_execute_internal = sp_zend_execute_internal; + if (zend_execute_internal != sp_zend_execute_internal) { + /* 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; + if (zend_stream_open_function != sp_stream_open) { + /* 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; + } } return SUCCESS; diff --git a/src/sp_sloppy.c b/src/sp_sloppy.c index f9ed7183..ff2d6448 100644 --- a/src/sp_sloppy.c +++ b/src/sp_sloppy.c @@ -99,12 +99,13 @@ PHP_FUNCTION(sp_array_keys) { void hook_sloppy() { TSRMLS_FETCH(); - if (NULL == orig_zend_compile_file) { + if (NULL == orig_zend_compile_file && zend_compile_file != sp_compile_file) { orig_zend_compile_file = zend_compile_file; zend_compile_file = sp_compile_file; } - if (NULL == orig_zend_compile_string) { + if (NULL == orig_zend_compile_string && + zend_compile_string != sp_compile_string) { orig_zend_compile_string = zend_compile_string; zend_compile_string = sp_compile_string; } From a27aa204d6b1e556dc30b0426f96d2c9244e75f8 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Thu, 22 Jul 2021 17:46:01 +0200 Subject: [PATCH 28/30] Sprinkle some const --- src/sp_var_parser.c | 2 +- src/sp_var_value.c | 26 +++++++++++++------------- src/sp_wrapper.c | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/sp_var_parser.c b/src/sp_var_parser.c index b066b840..bb5a5c03 100644 --- a/src/sp_var_parser.c +++ b/src/sp_var_parser.c @@ -20,7 +20,7 @@ static sp_list_node *parse_str_tokens(const char *str, return tokens_list; } -static bool is_var_name_valid(const char *name) { +static bool is_var_name_valid(const char *const name) { static sp_pcre *regexp_const = NULL; static sp_pcre *regexp_var = NULL; diff --git a/src/sp_var_value.c b/src/sp_var_value.c index e3514461..fe37f99f 100644 --- a/src/sp_var_value.c +++ b/src/sp_var_value.c @@ -1,7 +1,7 @@ #include "php_snuffleupagus.h" -static zval *get_param_var(zend_execute_data *ed, const char *var_name, - bool print) { +static zval *get_param_var(const zend_execute_data *const ed, + const char *const var_name, bool print) { unsigned int nb_param = ed->func->common.num_args; for (unsigned int i = 0; i < nb_param; i++) { @@ -21,7 +21,7 @@ static zval *get_param_var(zend_execute_data *ed, const char *var_name, return NULL; } -static zval *get_local_var(zend_execute_data *ed, const char *var_name) { +static zval *get_local_var(zend_execute_data *ed, const char *const var_name) { zend_execute_data *orig_execute_data = ed; zend_execute_data *current = ed; @@ -52,7 +52,7 @@ static zval *get_local_var(zend_execute_data *ed, const char *var_name) { return NULL; } -static zval *get_constant(const char *value) { +static zval *get_constant(const char *const value) { zend_string *name = zend_string_init(value, strlen(value), 0); zval *zvalue = zend_get_constant_ex(name, NULL, ZEND_FETCH_CLASS_SILENT); zend_string_release(name); @@ -90,8 +90,8 @@ static zval *get_var_value(zend_execute_data *ed, const char *var_name, } } -static void *get_entry_hashtable(const HashTable *ht, const char *entry, - size_t entry_len) { +static void *get_entry_hashtable(const HashTable *const ht, + const char *const entry, size_t entry_len) { zval *zvalue = zend_hash_str_find(ht, entry, entry_len); if (!zvalue) { @@ -109,8 +109,8 @@ static void *get_entry_hashtable(const HashTable *ht, const char *entry, return zvalue; } -static zval *get_array_value(zend_execute_data *ed, zval *zvalue, - const sp_tree *tree) { +static zval *get_array_value(zend_execute_data *ed, const zval *const zvalue, + const sp_tree *const tree) { zval *idx_value = sp_get_var_value(ed, tree->idx, false); if (!zvalue || !idx_value) { @@ -118,7 +118,7 @@ static zval *get_array_value(zend_execute_data *ed, zval *zvalue, } if (Z_TYPE_P(zvalue) == IS_ARRAY) { - const zend_string *idx = sp_zval_to_zend_string(idx_value); + const zend_string *const idx = sp_zval_to_zend_string(idx_value); return get_entry_hashtable(Z_ARRVAL_P(zvalue), ZSTR_VAL(idx), ZSTR_LEN(idx)); } @@ -128,10 +128,10 @@ static zval *get_array_value(zend_execute_data *ed, zval *zvalue, static zval *get_object_property(zend_execute_data *ed, zval *object, const char *property, bool is_param) { - char *class_name = object->value.obj->ce->name->val; + const char *const class_name = object->value.obj->ce->name->val; HashTable *array = Z_OBJPROP_P(object); zval *zvalue = NULL; - zval *property_val = get_var_value(ed, property, is_param); + const zval *property_val = get_var_value(ed, property, is_param); size_t len; if (property_val) { @@ -161,7 +161,7 @@ static zval *get_object_property(zend_execute_data *ed, zval *object, return zvalue; } -static zend_class_entry *get_class(const char *value) { +static zend_class_entry *get_class(const char *const value) { zend_string *name = zend_string_init(value, strlen(value), 0); zend_class_entry *ce = zend_lookup_class(name); zend_string_release(name); @@ -170,7 +170,7 @@ static zend_class_entry *get_class(const char *value) { static zval *get_unknown_type(const char *restrict value, zval *zvalue, zend_class_entry *ce, zend_execute_data *ed, - const sp_tree *tree, bool is_param) { + const sp_tree *const tree, bool is_param) { if (ce) { zvalue = get_entry_hashtable(&ce->constants_table, value, strlen(value)); ce = NULL; diff --git a/src/sp_wrapper.c b/src/sp_wrapper.c index 277f23a0..7610114f 100644 --- a/src/sp_wrapper.c +++ b/src/sp_wrapper.c @@ -1,6 +1,6 @@ #include "php_snuffleupagus.h" -static bool wrapper_is_whitelisted(const zend_string *zs) { +static bool wrapper_is_whitelisted(const zend_string *const zs) { const sp_list_node *list = SNUFFLEUPAGUS_G(config).config_wrapper->whitelist; if (!zs) { @@ -18,7 +18,7 @@ static bool wrapper_is_whitelisted(const zend_string *zs) { void sp_disable_wrapper() { HashTable *orig = php_stream_get_url_stream_wrappers_hash(); - HashTable *orig_complete = pemalloc(sizeof(*orig_complete), 1); + HashTable *orig_complete = pemalloc(sizeof(HashTable), 1); zval *zv; zend_string *zs; From 693041ba2ed4aa9bafaf0ae55fde02667799f385 Mon Sep 17 00:00:00 2001 From: WhiteWinterWolf Date: Sun, 25 Jul 2021 16:03:49 +0200 Subject: [PATCH 29/30] Replace an odd call to strtok_r(). --- src/sp_config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sp_config.c b/src/sp_config.c index 958c7e55..26476a28 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -73,7 +73,11 @@ int parse_list(char *restrict line, char *restrict keyword, void *list_ptr) { } tmp = ZSTR_VAL(value); - while ((token = strtok_r(tmp, ",", &tmp))) { + while (1) { + token = strsep(&tmp, ","); + if (token == NULL) { + break; + } *list = sp_list_insert(*list, zend_string_init(token, strlen(token), 1)); } From e62f226c3ed885808c832040872fc2d73ca46dac Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 28 Jul 2021 11:55:57 +0200 Subject: [PATCH 30/30] Sprinkle even more `const` --- src/snuffleupagus.c | 8 ++++---- src/sp_config.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index f0737b43..7bf3649a 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -21,7 +21,7 @@ ZEND_DLEXPORT int sp_zend_startup(zend_extension *extension) { } // LCOV_EXCL_END -static inline void sp_op_array_handler(zend_op_array *op) { +static inline void sp_op_array_handler(zend_op_array *const op) { // We need a filename, and strict mode not already enabled on this op if (NULL == op->filename || op->fn_flags & ZEND_ACC_STRICT_TYPES) { return; @@ -121,7 +121,7 @@ PHP_MINIT_FUNCTION(snuffleupagus) { return SUCCESS; } -static void free_disabled_functions_hashtable(HashTable *ht) { +static void free_disabled_functions_hashtable(HashTable *const ht) { void *ptr = NULL; ZEND_HASH_FOREACH_PTR(ht, ptr) { sp_list_free(ptr); } ZEND_HASH_FOREACH_END(); @@ -186,7 +186,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) { } PHP_RINIT_FUNCTION(snuffleupagus) { - const sp_config_wrapper *config_wrapper = + const sp_config_wrapper *const config_wrapper = SNUFFLEUPAGUS_G(config).config_wrapper; #if defined(COMPILE_DL_SNUFFLEUPAGUS) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); @@ -254,7 +254,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { while (1) { // We don't care about overwriting new_value->val - char *config_file = strsep(&str, ","); + const char *config_file = strsep(&str, ","); if (config_file == NULL) break; glob_t globbuf; diff --git a/src/sp_config.c b/src/sp_config.c index 26476a28..c12b4358 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -6,7 +6,7 @@ size_t sp_line_no; -sp_config_tokens const sp_func[] = { +static sp_config_tokens const sp_func[] = { {.func = parse_unserialize, .token = SP_TOKEN_UNSERIALIZE_HMAC}, {.func = parse_random, .token = SP_TOKEN_HARDEN_RANDOM}, {.func = parse_log_media, .token = SP_TOKEN_LOG_MEDIA},