From efcc16ad74a32d4b735bad73690c49c5cdb63cb7 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 3 May 2022 00:04:50 +0200 Subject: Add a `const` --- src/sp_config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/sp_config.h') diff --git a/src/sp_config.h b/src/sp_config.h index 6d48240..87710a0 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -269,7 +269,7 @@ typedef struct { #define SP_TOKEN_LIST "list" -zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, sp_config_keyword *config_keywords); +zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, const sp_config_keyword *config_keywords); zend_result sp_parse_config(const char *filename); -- cgit v1.3 From 7c2d1d7d2713c0fa6bda63c376baf25d9f3d712c Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 3 May 2022 21:48:35 +0200 Subject: More const frenzy --- src/sp_config.c | 11 +++++------ src/sp_config.h | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'src/sp_config.h') diff --git a/src/sp_config.c b/src/sp_config.c index 5431eca..d29247b 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -6,7 +6,7 @@ static zend_result sp_process_config_root(sp_parsed_keyword *parsed_rule) { - sp_config_keyword sp_func[] = { + static const sp_config_keyword sp_func[] = { {parse_unserialize, SP_TOKEN_UNSERIALIZE_HMAC, &(SPCFG(unserialize))}, {parse_enable, SP_TOKEN_HARDEN_RANDOM, &(SPCFG(random).enable)}, {parse_log_media, SP_TOKEN_LOG_MEDIA, &(SPCFG(log_media))}, @@ -29,7 +29,7 @@ static zend_result sp_process_config_root(sp_parsed_keyword *parsed_rule) { return sp_process_rule(parsed_rule, sp_func); } -zend_result sp_parse_config(const char *filename) { +zend_result sp_parse_config(const char *const filename) { FILE *fd = fopen(filename, "rb"); if (fd == NULL) { sp_log_err("config", "Could not open configuration file %s : %s", filename, strerror(errno)); @@ -65,7 +65,7 @@ zend_result sp_parse_config(const char *filename) { } -zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, const sp_config_keyword *config_keywords) { +zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, const sp_config_keyword *const config_keywords) { for (sp_parsed_keyword *kw = parsed_rule; kw->kw; kw++) { bool found_kw = false; for (const sp_config_keyword *ckw = config_keywords; ckw->func; ckw++) { @@ -119,13 +119,12 @@ SP_PARSEKW_FN(parse_list) { CHECK_DUPLICATE_KEYWORD(retval); sp_list_node **list = retval; - char *tok, *tmp; SP_PARSE_ARG(value); - tmp = ZSTR_VAL(value); + char* tmp = ZSTR_VAL(value); while (1) { - tok = strsep(&tmp, ","); + const char* const tok = strsep(&tmp, ","); if (tok == NULL) { break; } diff --git a/src/sp_config.h b/src/sp_config.h index 87710a0..3d92f2f 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -176,7 +176,7 @@ typedef struct { HashTable *entries; // ht of sp_ini_entry } sp_config_ini; -#define SP_PARSE_FN_(fname, kwvar) int fname(char *token, sp_parsed_keyword *kwvar, void *retval) +#define SP_PARSE_FN_(fname, kwvar) int fname(char const *const token, sp_parsed_keyword *kwvar, void *retval) #define SP_PARSE_FN(fname) SP_PARSE_FN_(fname, parsed_rule) #define SP_PARSEKW_FN(fname) SP_PARSE_FN_(fname, kw) @@ -269,9 +269,9 @@ typedef struct { #define SP_TOKEN_LIST "list" -zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, const sp_config_keyword *config_keywords); +zend_result sp_process_rule(sp_parsed_keyword *parsed_rule, const sp_config_keyword *const config_keywords); -zend_result sp_parse_config(const char *filename); +zend_result sp_parse_config(const char *const filename); #define SP_PARSE_CHECK_ARG_EXISTS(value) \ if (!value) { \ -- cgit v1.3 From cd9031935ef2306f7ba2097856b20bf116f341ee Mon Sep 17 00:00:00 2001 From: Ben Fuhrmannek Date: Tue, 19 Apr 2022 19:01:52 +0200 Subject: extended checks for readonly_exec, enabled by default introduced config options: readonly_exec.extended_checks() or xchecks() readonly_exec.no_extended_checks() or noxchecks() --- src/snuffleupagus.c | 2 + src/sp_config.h | 1 + src/sp_config_keywords.c | 7 +- src/sp_execute.c | 77 ++++++++++++++++++---- .../deny_writable_execution_simulation.phpt | 10 ++- 5 files changed, 80 insertions(+), 17 deletions(-) (limited to 'src/sp_config.h') diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c index 5bcd57c..448fd76 100644 --- a/src/snuffleupagus.c +++ b/src/snuffleupagus.c @@ -340,6 +340,7 @@ static void dump_config() { add_assoc_bool(&arr, "readonly_exec.enable", SPCFG(readonly_exec).enable); add_assoc_bool(&arr, "readonly_exec.sim", SPCFG(readonly_exec).simulation); ADD_ASSOC_ZSTR(&arr, SP_TOKEN_READONLY_EXEC "." SP_TOKEN_DUMP, SPCFG(readonly_exec).dump); + add_assoc_bool(&arr, "readonly_exec.extended_checks", SPCFG(readonly_exec).extended_checks); add_assoc_bool(&arr, "global_strict.enable", SPCFG(global_strict).enable); @@ -494,6 +495,7 @@ static PHP_INI_MH(OnUpdateConfiguration) { // set some defaults SPCFG(show_old_php_warning) = true; + SPCFG(readonly_exec).extended_checks = true; char *str = new_value->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