extended checks for readonly_exec, enabled by default

introduced config options:
readonly_exec.extended_checks() or xchecks()
readonly_exec.no_extended_checks() or noxchecks()

4 files changed, 73 insertions, 14 deletions

diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c
index ebb7f9c..c723199 100644
--- a/src/snuffleupagus.c
+++ b/src/snuffleupagus.c
@@ -348,6 +348,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);
 
@@ -499,6 +500,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 6d48240..e7d4e4d 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 9cf44e1..3a474a3 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(