From 5b25788a81bf7ad233d99cf3f5e9ce3dcc5e8602 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 2 May 2022 22:36:53 +0200 Subject: Add more tests for php8 --- src/tests/deny_writable/deny_writable_execution_simulation.phpt | 1 - 1 file changed, 1 deletion(-) (limited to 'src/tests/deny_writable/deny_writable_execution_simulation.phpt') diff --git a/src/tests/deny_writable/deny_writable_execution_simulation.phpt b/src/tests/deny_writable/deny_writable_execution_simulation.phpt index 1118dc0..e3460e4 100644 --- a/src/tests/deny_writable/deny_writable_execution_simulation.phpt +++ b/src/tests/deny_writable/deny_writable_execution_simulation.phpt @@ -1,7 +1,6 @@ --TEST-- Readonly execution attempt (simulation mode) --SKIPIF-- -= 80000) print "skip"; ?> val; diff --git a/src/sp_config.h b/src/sp_config.h index 3d92f2f..ec73310 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -35,6 +35,7 @@ typedef struct { typedef struct { bool enable; bool simulation; + bool extended_checks; zend_string *dump; zend_string *textual_representation; } sp_config_readonly_exec; diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index f7be731..e0e5166 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -96,7 +96,7 @@ SP_PARSE_FN(parse_unserialize) { } SP_PARSE_FN(parse_readonly_exec) { - bool enable = false, disable = false; + bool enable = false, disable = false, xchecks = false, no_xchecks = false; sp_config_readonly_exec *cfg = (sp_config_readonly_exec*)retval; sp_config_keyword config_keywords[] = { @@ -105,6 +105,10 @@ SP_PARSE_FN(parse_readonly_exec) { {parse_empty, SP_TOKEN_SIMULATION, &(cfg->simulation)}, {parse_empty, SP_TOKEN_SIM, &(cfg->simulation)}, {parse_str, SP_TOKEN_DUMP, &(cfg->dump)}, + {parse_empty, "extended_checks", &(xchecks)}, + {parse_empty, "xchecks", &(xchecks)}, + {parse_empty, "no_extended_checks", &(no_xchecks)}, + {parse_empty, "noxchecks", &(no_xchecks)}, {0, 0, 0}}; SP_PROCESS_CONFIG_KEYWORDS_ERR(); @@ -112,6 +116,7 @@ SP_PARSE_FN(parse_readonly_exec) { cfg->textual_representation = sp_get_textual_representation(parsed_rule); SP_SET_ENABLE_DISABLE(enable, disable, cfg->enable); + if (xchecks) { cfg->extended_checks = true; } else if (no_xchecks) { cfg->extended_checks = false; } return SP_PARSER_STOP; } diff --git a/src/sp_execute.c b/src/sp_execute.c index cc401aa..56d25c5 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -1,4 +1,5 @@ #include "php_snuffleupagus.h" +#include "ext/standard/php_string.h" static void (*orig_execute_ex)(zend_execute_data *execute_data) = NULL; static void (*orig_zend_execute_internal)(zend_execute_data *execute_data, @@ -12,22 +13,72 @@ static zend_result (*orig_zend_stream_open)(zend_file_handle *handle) = NULL; // FIXME handle symlink ZEND_COLD static inline void terminate_if_writable(const char *filename) { const sp_config_readonly_exec *config_ro_exec = &(SPCFG(readonly_exec)); + char *errmsg = "unknown access problem"; + + // check write access if (0 == access(filename, W_OK)) { - if (config_ro_exec->dump) { - sp_log_request(config_ro_exec->dump, config_ro_exec->textual_representation); - } - if (true == config_ro_exec->simulation) { - sp_log_simulation("readonly_exec", "Attempted execution of a writable file (%s)", filename); - } else { - sp_log_drop("readonly_exec", "Attempted execution of a writable file (%s)", filename); - } + errmsg = "Attempted execution of a writable file"; + goto violation; + } + if (errno != EACCES) { + goto err; + } + + // other checks are 'extended checks' that can be enabled/disabled via config + if (!config_ro_exec->extended_checks) { + return; + } + + // check effective uid + struct stat buf; + if (0 != stat(filename, &buf)) { + goto err; + } + if (buf.st_uid == geteuid()) { + errmsg = "Attempted execution of file owned by process"; + goto violation; + } + + // check write access on directory + char *dirname = estrndup(filename, strlen(filename)); + php_dirname(dirname, strlen(dirname)); + if (0 == access(dirname, W_OK)) { + errmsg = "Attempted execution of file in writable directory"; + efree(dirname); + goto violation; + } + if (errno != EACCES) { + efree(dirname); + goto err; + } + + // check effecite uid of directory + if (0 != stat(dirname, &buf)) { + efree(dirname); + goto err; + } + efree(dirname); + if (buf.st_uid == geteuid()) { + errmsg = "Attempted execution of file in directory owned by process"; + goto violation; + } + + // we would actually need to check all parent directories as well, but that task is left for other tools + return; + +violation: + if (config_ro_exec->dump) { + sp_log_request(config_ro_exec->dump, config_ro_exec->textual_representation); + } + if (config_ro_exec->simulation) { + sp_log_simulation("readonly_exec", "%s (%s)", errmsg, filename); } else { - if (EACCES != errno) { - // LCOV_EXCL_START - sp_log_err("Writable execution", "Error while accessing %s: %s", filename, strerror(errno)); - // LCOV_EXCL_STOP - } + sp_log_drop("readonly_exec", "%s (%s)", errmsg, filename); } + return; + +err: + sp_log_err("readonly_exec", "Error while accessing %s: %s", filename, strerror(errno)); } inline static void is_builtin_matching( diff --git a/src/tests/deny_writable/deny_writable_execution_simulation.phpt b/src/tests/deny_writable/deny_writable_execution_simulation.phpt index e3460e4..abc276f 100644 --- a/src/tests/deny_writable/deny_writable_execution_simulation.phpt +++ b/src/tests/deny_writable/deny_writable_execution_simulation.phpt @@ -41,10 +41,14 @@ unlink("$dir/non_writable_file.txt"); unlink("$dir/writable_file.txt"); ?> --EXPECTF-- -Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/deny_writable_execution_simulation.php). in %a/deny_writable_execution_simulation.php on line 2 +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/deny_writable_execution_simulation.php) in %a/deny_writable_execution_simulation.php on line 2 -Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/writable_file.txt). in %a/deny_writable_execution_simulation.php on line 12 +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/writable_file.txt) in %a/deny_writable_execution_simulation.php on line 12 -Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/writable_file.txt). in %a/writable_file.txt on line 1 +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/writable_file.txt) in %a/writable_file.txt on line 1 Code execution within a writable file. + +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of file owned by process (%s/tests/deny_writable/non_writable_file.txt) in %s/tests/deny_writable/deny_writable_execution_simulation.php on line 13 + +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of file owned by process (%s/tests/deny_writable/non_writable_file.txt) in %src/tests/deny_writable/non_writable_file.txt on line 1 Code execution within a non-writable file. -- cgit v1.3 From 8d6496efcab420267a228c35f9f627fec209d031 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 12 Jul 2022 23:03:46 +0200 Subject: Refactoring of the previous commit --- src/php_snuffleupagus.h | 1 + src/sp_execute.c | 20 +++++++------------- .../deny_writable_execution_simulation.phpt | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) (limited to 'src/tests/deny_writable/deny_writable_execution_simulation.phpt') diff --git a/src/php_snuffleupagus.h b/src/php_snuffleupagus.h index 95caa65..3eeb9db 100644 --- a/src/php_snuffleupagus.h +++ b/src/php_snuffleupagus.h @@ -38,6 +38,7 @@ #include "ext/standard/head.h" #include "ext/standard/info.h" #include "ext/standard/url.h" +#include "ext/standard/php_string.h" #include "ext/standard/php_var.h" #include "ext/session/php_session.h" #include "php.h" diff --git a/src/sp_execute.c b/src/sp_execute.c index 56d25c5..65a32db 100644 --- a/src/sp_execute.c +++ b/src/sp_execute.c @@ -1,5 +1,4 @@ #include "php_snuffleupagus.h" -#include "ext/standard/php_string.h" static void (*orig_execute_ex)(zend_execute_data *execute_data) = NULL; static void (*orig_zend_execute_internal)(zend_execute_data *execute_data, @@ -11,11 +10,10 @@ static zend_result (*orig_zend_stream_open)(zend_file_handle *handle) = NULL; #endif // FIXME handle symlink -ZEND_COLD static inline void terminate_if_writable(const char *filename) { - const sp_config_readonly_exec *config_ro_exec = &(SPCFG(readonly_exec)); - char *errmsg = "unknown access problem"; +ZEND_COLD static inline void terminate_if_writable(char const* const filename) { + sp_config_readonly_exec const* const config_ro_exec = &(SPCFG(readonly_exec)); + char const *errmsg = "unknown access problem"; - // check write access if (0 == access(filename, W_OK)) { errmsg = "Attempted execution of a writable file"; goto violation; @@ -29,21 +27,19 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { return; } - // check effective uid struct stat buf; if (0 != stat(filename, &buf)) { goto err; } if (buf.st_uid == geteuid()) { - errmsg = "Attempted execution of file owned by process"; + errmsg = "Attempted execution of a file owned by the PHP process"; goto violation; } - // check write access on directory - char *dirname = estrndup(filename, strlen(filename)); + char *const dirname = estrndup(filename, strlen(filename)); php_dirname(dirname, strlen(dirname)); if (0 == access(dirname, W_OK)) { - errmsg = "Attempted execution of file in writable directory"; + errmsg = "Attempted execution of a file in a writable directory"; efree(dirname); goto violation; } @@ -52,18 +48,16 @@ ZEND_COLD static inline void terminate_if_writable(const char *filename) { goto err; } - // check effecite uid of directory if (0 != stat(dirname, &buf)) { efree(dirname); goto err; } efree(dirname); if (buf.st_uid == geteuid()) { - errmsg = "Attempted execution of file in directory owned by process"; + errmsg = "Attempted execution of a file in directory owned by the PHP process"; goto violation; } - // we would actually need to check all parent directories as well, but that task is left for other tools return; violation: diff --git a/src/tests/deny_writable/deny_writable_execution_simulation.phpt b/src/tests/deny_writable/deny_writable_execution_simulation.phpt index abc276f..d4e4801 100644 --- a/src/tests/deny_writable/deny_writable_execution_simulation.phpt +++ b/src/tests/deny_writable/deny_writable_execution_simulation.phpt @@ -48,7 +48,7 @@ Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a writable file (%a/writable_file.txt) in %a/writable_file.txt on line 1 Code execution within a writable file. -Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of file owned by process (%s/tests/deny_writable/non_writable_file.txt) in %s/tests/deny_writable/deny_writable_execution_simulation.php on line 13 +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a file owned by the PHP process (%s/tests/deny_writable/non_writable_file.txt) in %s/tests/deny_writable/deny_writable_execution_simulation.php on line 13 -Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of file owned by process (%s/tests/deny_writable/non_writable_file.txt) in %src/tests/deny_writable/non_writable_file.txt on line 1 +Warning: [snuffleupagus][0.0.0.0][readonly_exec][simulation] Attempted execution of a file owned by the PHP process (%s/tests/deny_writable/non_writable_file.txt) in %src/tests/deny_writable/non_writable_file.txt on line 1 Code execution within a non-writable file. -- cgit v1.3