Hello,
I'm CCing Dmitry Stogov as maintainer because he's listed as an author
in ext/opcache/ZendAccelerator.c and has recent commits.
I've attached a patch for bug #69090. You can find a more detailed
writeup at https://bugs.php.net/bug.php?id=69090 . In short, the patch
adds EUID or Windows username at the beginning of OPCache keys to
prevent cross-user cache access, which will hopefully alleviate security
concerns of enabling OPCache on shared hosting servers.
I took this in a different direction than that proposed in bug #69090
(prepending inode to key) because I feel it more effectively addresses
the cross-user security concerns.
I don't have a test script yet because the change is transparent to
scripts, but I could probably cobble one together by checking OPCache
debug log for key names. I do intend to port this forward to PHP7 head,
but in my opinion the existing behavior in 5.6 is a serious
vulnerability which warrants a maintenance patch. If needed I can
provide working exploit scripts to demonstrate how bad the existing
behavior is for shared servers using OPCache.
I was hoping to get some feedback before I put in the effort to port
this to PHP7.
Thanks,
--
- php-...@coydogsoftware.net
diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
index 985a4ef..3522861 100644
--- a/ext/opcache/ZendAccelerator.c
+++ b/ext/opcache/ZendAccelerator.c
@@ -52,6 +52,8 @@
typedef int uid_t;
typedef int gid_t;
#include <io.h>
+#include <Windows.h>
+#include <Lmcons.h> /* username max len */
#endif
#ifndef ZEND_WIN32
@@ -949,8 +951,36 @@ static unsigned int zend_accel_script_checksum(zend_persistent_script *persisten
char *accel_make_persistent_key_ex(zend_file_handle *file_handle, int path_length, int *key_len TSRMLS_DC)
{
int key_length;
+ int key_offset = 0;
+ char *user_id_str = NULL;
+ int user_id_len = 0;
- /* CWD and include_path don't matter for absolute file names and streams */
+#ifdef ZEND_WIN32
+ /* Windows has no direct equivalent of EUID. SID and RID are roughly
+ * analagous, but for now simply using the username seems most
+ * straightforward since we can get max len from UNLEN in Lmcons.h
+ */
+ int username_len = UNLEN + 1;
+ char username_str[UNLEN + 1]; /* not wide-character compatible */
+ if (GetUserName(username_str, &username_len) == 0) {
+ zend_accel_error(ACCEL_LOG_WARNING, "GetUserName for opcache key user_id_str failed!");
+ return NULL;
+ } else {
+ user_id_len = username_len - 1; /* subtract terminating zero */
+ user_id_str = username_str;
+ }
+#else
+ #define EUID_BUFFSIZE 40 /* hope we never see >128-bit uid_t's */
+ uid_t euid = geteuid();
+ char euid_str[EUID_BUFFSIZE];
+ if (user_id_len = snprintf(euid_str, EUID_BUFFSIZE - 1, "%d", euid) >= EUID_BUFFSIZE) {
+ return NULL;
+ } else {
+ user_id_str = euid_str;
+ }
+#endif
+
+ /* CWD and include_path don't matter for absolute file names and streams */
if (ZCG(accel_directives).use_cwd &&
!IS_ABSOLUTE_PATH(file_handle->filename, path_length) &&
!is_stream_path(file_handle->filename)) {
@@ -958,7 +988,6 @@ char *accel_make_persistent_key_ex(zend_file_handle *file_handle, int path_lengt
int include_path_len = 0;
const char *parent_script = NULL;
int parent_script_len = 0;
- int cur_len = 0;
int cwd_len;
char *cwd;
@@ -1027,7 +1056,7 @@ char *accel_make_persistent_key_ex(zend_file_handle *file_handle, int path_lengt
}
/* Calculate key length */
- key_length = cwd_len + path_length + include_path_len + 2;
+ key_length = user_id_len + cwd_len + path_length + include_path_len + 3; /* +3 for delimiter colons */
if (parent_script_len) {
key_length += parent_script_len + 1;
}
@@ -1036,39 +1065,57 @@ char *accel_make_persistent_key_ex(zend_file_handle *file_handle, int path_lengt
* Note - the include_path must be the last element in the key,
* since in itself, it may include colons (which we use to separate
* different components of the key)
- */
+ */
if ((size_t)key_length >= sizeof(ZCG(key))) {
ZCG(key_len) = 0;
return NULL;
}
- memcpy(ZCG(key), cwd, cwd_len);
- ZCG(key)[cwd_len] = ':';
-
- memcpy(ZCG(key) + cwd_len + 1, file_handle->filename, path_length);
-
- ZCG(key)[cwd_len + 1 + path_length] = ':';
- cur_len = cwd_len + 1 + path_length + 1;
-
- if (parent_script_len) {
- memcpy(ZCG(key) + cur_len, parent_script, parent_script_len);
- cur_len += parent_script_len;
- ZCG(key)[cur_len] = ':';
- cur_len++;
+ /* Key on euid to prevent cross-user cache access bypassing file
+ * permissions. Prevents filename collision in chroots IFF each
+ * chroot environment has a different user.
+ */
+ memcpy(ZCG(key), user_id_str, user_id_len);
+ key_offset += user_id_len;
+ ZCG(key)[key_offset] = ':';
+ key_offset++;
+
+ memcpy(ZCG(key + key_offset), cwd, cwd_len);
+ key_offset += cwd_len;
+ ZCG(key)[key_offset] = ':';
+ key_offset++;
+
+ memcpy(ZCG(key) + key_offset, file_handle->filename, path_length);
+ key_offset += path_length;
+ ZCG(key)[key_offset] = ':';
+ key_offset++;
+
+ if (parent_script_len) {
+ memcpy(ZCG(key) + key_offset, parent_script, parent_script_len);
+ key_offset += parent_script_len;
+ ZCG(key)[key_offset] = ':';
+ key_offset++;
}
- memcpy(ZCG(key) + cur_len, include_path, include_path_len);
+ memcpy(ZCG(key) + key_offset, include_path, include_path_len);
ZCG(key)[key_length] = '\0';
} else {
- /* not use_cwd */
- key_length = path_length;
+ /* not use_cwd and use_cwd cases where filename is absolute */
+ key_length = user_id_len + 1 + path_length; /* <EUID>:<path> */
if ((size_t)key_length >= sizeof(ZCG(key))) {
ZCG(key_len) = 0;
return NULL;
}
- memcpy(ZCG(key), file_handle->filename, key_length + 1);
+
+ memcpy(ZCG(key), user_id_str, user_id_len);
+ key_offset = user_id_len;
+ ZCG(key)[key_offset] = ':';
+ key_offset++;
+
+ memcpy(ZCG(key + key_offset), file_handle->filename, key_length + 1);
}
*key_len = ZCG(key_len) = key_length;
+ zend_accel_error(ACCEL_LOG_DEBUG, "make_persistent_key_ex() returning key: %s", ZCG(key));
return ZCG(key);
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
