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')

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