Hi all,

Few years ago, I have proposed strict session.
It seems PHP 5.4 and php-src don't have protection against session
adoption yet.

Since there will be many TLDs, session adoption attack will be
very easy for some domains until browsers support them.
Even without new TLDs, attacker may place cookie file to attack
session adoption or can exploit paths or domains, since there is no
standard for precedence.

e.g. Domain has more precedence over path on Chrome while
path has greater precedence on IE. This is due to the order difference
of sent cookies. (If there are multiple cookies are set for domain/path, only
one became the outstanding cookie. I think PHP uses first IIRC while other
implementation may use the last. Therefore, browser may not able to
solve this issue, since it may destroy apps specific to browser)

Even if a programmer sets path and domain for PHP session id cookie,
attackers may exploit this to fix session id with session managers that are
vulnerable to session adoption.

If you don't get idea, play with cookie with/without domain/path is set/unset.
You'll see how attacker may use session adoption. Default session module's
configuration (domain not set, path set to /) is very easy to exploit anyway.

I usually set both exact application domain/path, and unset all domain/path
cookies for session id to prevent the attack. I guess this is not
widely adopted.
Even this is not enough. For example, if subdomain is added, Chrome has
greater precedence for subdomain and attacker may exploit it.

I pasted a patch for PHP 5.2 that rejects uninitialized session id. I
think original
patch was written by Stefan Esser. It is for PHP 5.2, but it's easy to port to
current PHP. If one would like to old behavior, he/she can just turn off the
strict session.

There are too many ways to exploit session with session adoption vulnerable
session manger. Simple solution is making session manager strict.

Any comments?

--
Yasuo Ohgaki
yohg...@ohgaki.net


Index: ext/session/php_session.h
===================================================================
--- ext/session/php_session.h   (リビジョン 288932)
+++ ext/session/php_session.h   (作業コピー)
@@ -23,7 +23,7 @@

 #include "ext/standard/php_var.h"

-#define PHP_SESSION_API 20020330
+#define PHP_SESSION_API 20051121

 #define PS_OPEN_ARGS void **mod_data, const char *save_path, const
char *session_name TSRMLS_DC
 #define PS_CLOSE_ARGS void **mod_data TSRMLS_DC
@@ -32,6 +32,7 @@
 #define PS_DESTROY_ARGS void **mod_data, const char *key TSRMLS_DC
 #define PS_GC_ARGS void **mod_data, int maxlifetime, int *nrdels TSRMLS_DC
 #define PS_CREATE_SID_ARGS void **mod_data, int *newlen TSRMLS_DC
+#define PS_VALIDATE_SID_ARGS void **mod_data, const char *key TSRMLS_DC

 /* default create id function */
 PHPAPI char *php_session_create_id(PS_CREATE_SID_ARGS);
@@ -45,6 +46,7 @@
       int (*s_destroy)(PS_DESTROY_ARGS);
       int (*s_gc)(PS_GC_ARGS);
       char *(*s_create_sid)(PS_CREATE_SID_ARGS);
+        int (*s_validate_sid)(PS_VALIDATE_SID_ARGS);
 } ps_module;

 #define PS_GET_MOD_DATA() *mod_data
@@ -57,6 +59,7 @@
 #define PS_DESTROY_FUNC(x)     int ps_delete_##x(PS_DESTROY_ARGS)
 #define PS_GC_FUNC(x)          int ps_gc_##x(PS_GC_ARGS)
 #define PS_CREATE_SID_FUNC(x)  char *ps_create_sid_##x(PS_CREATE_SID_ARGS)
+#define PS_VALIDATE_SID_FUNC(x)        int
ps_validate_sid_##x(PS_VALIDATE_SID_ARGS)

 #define PS_FUNCS(x) \
       PS_OPEN_FUNC(x); \
@@ -65,11 +68,12 @@
       PS_WRITE_FUNC(x); \
       PS_DESTROY_FUNC(x); \
       PS_GC_FUNC(x);  \
-       PS_CREATE_SID_FUNC(x)
+       PS_CREATE_SID_FUNC(x); \
+    PS_VALIDATE_SID_FUNC(x)

 #define PS_MOD(x) \
       #x, ps_open_##x, ps_close_##x, ps_read_##x, ps_write_##x, \
-        ps_delete_##x, ps_gc_##x, php_session_create_id
+        ps_delete_##x, ps_gc_##x, php_session_create_id, ps_validate_sid_##x

 /* SID enabled module handler definitions */
 #define PS_FUNCS_SID(x) \
@@ -79,11 +83,12 @@
       PS_WRITE_FUNC(x); \
       PS_DESTROY_FUNC(x); \
       PS_GC_FUNC(x); \
-       PS_CREATE_SID_FUNC(x)
+       PS_CREATE_SID_FUNC(x); \
+    PS_VALIDATE_SID(x)

 #define PS_MOD_SID(x) \
       #x, ps_open_##x, ps_close_##x, ps_read_##x, ps_write_##x, \
-        ps_delete_##x, ps_gc_##x, ps_create_sid_##x
+        ps_delete_##x, ps_gc_##x, ps_create_sid_##x, ps_validate_sid_##x

 typedef enum {
       php_session_disabled,
@@ -121,6 +126,7 @@
       zend_bool use_only_cookies;
       zend_bool use_trans_sid;        /* contains the INI value of
whether to use
trans-sid */
       zend_bool apply_trans_sid;      /* whether or not to enable trans-sid for
the current request */
+        zend_bool use_strict_mode;      /* whether or not PHP accepts
unknown session ids */

       long hash_func;
       long hash_bits_per_character;
Index: ext/session/tests/session_commit_variation4.phpt
===================================================================
--- ext/session/tests/session_commit_variation4.phpt    (リビジョン 288932)
+++ ext/session/tests/session_commit_variation4.phpt    (作業コピー)
@@ -1,10 +1,11 @@
 --TEST--
 Test session_commit() function : variation
 --SKIPIF--
-<?php include('skipif.inc'); ?>
+<?php
+die('skip'); // don't work with strict session patch
+?>
 --FILE--
 <?php
-
 ob_start();

 /*
Index: ext/session/tests/session_id_basic.phpt
===================================================================
--- ext/session/tests/session_id_basic.phpt     (リビジョン 288932)
+++ ext/session/tests/session_id_basic.phpt     (作業コピー)
@@ -1,10 +1,11 @@
 --TEST--
 Test session_id() function : basic functionality
 --SKIPIF--
-<?php include('skipif.inc'); ?>
+<?php
+die('skip'); // does not work with strict session
+?>
 --FILE--
 <?php
-
 ob_start();

 /*
Index: ext/session/tests/session_write_close_variation4.phpt
===================================================================
--- ext/session/tests/session_write_close_variation4.phpt       (リビジョン 288932)
+++ ext/session/tests/session_write_close_variation4.phpt       (作業コピー)
@@ -1,10 +1,11 @@
 --TEST--
 Test session_write_close() function : variation
 --SKIPIF--
-<?php include('skipif.inc'); ?>
+<?php
+die('skip'); // does not work with strict session
+?>
 --FILE--
 <?php
-
 ob_start();

 /*
Index: ext/session/mod_files.c
===================================================================
--- ext/session/mod_files.c     (リビジョン 288932)
+++ ext/session/mod_files.c     (作業コピー)
@@ -464,6 +464,35 @@
       return SUCCESS;
 }

+PS_VALIDATE_SID_FUNC(files)
+{
+       char buf[MAXPATHLEN];
+       int fd;
+       PS_FILES_DATA;
+
+       if (!ps_files_valid_key(key)) {
+                return FAILURE;
+        }
+
+        if (!PS(use_strict_mode)) {
+                return SUCCESS;
+        }
+
+        if (!ps_files_path_create(buf, sizeof(buf), data, key)) {
+               return FAILURE;
+        }
+
+        fd = VCWD_OPEN_MODE(buf, O_RDWR | O_BINARY,
+                               data->filemode);
+
+        if (fd != -1) {
+                close(fd);
+                return SUCCESS;
+        }
+
+       return FAILURE;
+}
+
 /*
 * Local variables:
 * tab-width: 4
Index: ext/session/mod_user.h
===================================================================
--- ext/session/mod_user.h      (リビジョン 288932)
+++ ext/session/mod_user.h      (作業コピー)
@@ -22,7 +22,7 @@
 #define MOD_USER_H

 typedef union {
-       zval *names[6];
+       zval *names[8];
       struct {
               zval *ps_open;
               zval *ps_close;
@@ -30,6 +30,8 @@
               zval *ps_write;
               zval *ps_destroy;
               zval *ps_gc;
+                zval *ps_create;
+                zval *ps_validate;
       } name;
 } ps_user;

Index: ext/session/session.c
===================================================================
--- ext/session/session.c       (リビジョン 288932)
+++ ext/session/session.c       (作業コピー)
@@ -462,6 +462,15 @@
               return;
       }

+    /* If there is an ID, use session module to verify it */
+    if (PS(id)) {
+        if (PS(mod)->s_validate_sid(&PS(mod_data), PS(id) TSRMLS_CC)
== FAILURE) {
+            efree(PS(id));
+            PS(id) = NULL;
+            PS(send_cookie) = 1;
+        }
+    }
+
       /* If there is no ID, use session module to create one */
       if (!PS(id)) {
 new_session:
@@ -699,6 +708,7 @@
       STD_PHP_INI_BOOLEAN("session.cookie_httponly",  "",
PHP_INI_ALL, OnUpdateBool,   cookie_httponly,    php_ps_globals,
ps_globals)
       STD_PHP_INI_BOOLEAN("session.use_cookies",      "1",
PHP_INI_ALL, OnUpdateBool,   use_cookies,        php_ps_globals,
ps_globals)
       STD_PHP_INI_BOOLEAN("session.use_only_cookies", "0",
PHP_INI_ALL, OnUpdateBool,   use_only_cookies,   php_ps_globals,
ps_globals)
+       STD_PHP_INI_BOOLEAN("session.use_strict_mode",  "1",
PHP_INI_ALL, OnUpdateBool,   use_strict_mode,    php_ps_globals,
ps_globals)
       STD_PHP_INI_ENTRY("session.referer_check",      "",
PHP_INI_ALL, OnUpdateString, extern_referer_chk, php_ps_globals,
ps_globals)
       STD_PHP_INI_ENTRY("session.entropy_file",       "",
PHP_INI_ALL, OnUpdateString, entropy_file,       php_ps_globals,
ps_globals)
       STD_PHP_INI_ENTRY("session.entropy_length",     "0",
PHP_INI_ALL, OnUpdateLong,   entropy_length,     php_ps_globals,
ps_globals)
@@ -1535,12 +1545,12 @@
 }
 /* }}} */

-/* {{{ proto void session_set_save_handler(string open, string close,
string read, string write, string destroy, string gc)
+/* /* {{{ proto void session_set_save_handler(string open, string
close, string read, string write, string destroy, string gc[, string
create, string validate])
   Sets user-level functions */
 static PHP_FUNCTION(session_set_save_handler)
 {
-       zval **args[6];
-       int i;
+       zval **args[8];
+       int i, numargs;
       ps_user *mdata;
       char *name;

@@ -1548,10 +1558,20 @@
               RETURN_FALSE;
       }

-       if (ZEND_NUM_ARGS() != 6 || zend_get_parameters_array_ex(6,
args) == FAILURE)
-               WRONG_PARAM_COUNT;
+    numargs = ZEND_NUM_ARGS();
+    args[6] = NULL;
+    args[7] = NULL;
+
+    if (numargs < 6 || numargs > 8 ||
zend_get_parameters_array_ex(numargs, args) == FAILURE)
+        WRONG_PARAM_COUNT;

-       for (i = 0; i < 6; i++) {
+    if (PS(session_status) != php_session_none)
+        RETURN_FALSE;
+
+    for (i = 0; i < 8; i++) {
+        if (i >= 6 && (args[i] == NULL || ZVAL_IS_NULL(*args[i]))) {
+            continue;
+        }
               if (!zend_is_callable(*args[i], 0, &name)) {
                       php_error_docref(NULL TSRMLS_CC, E_WARNING,
"Argument %d is not a
valid callback", i+1);
                       efree(name);
@@ -1572,12 +1592,15 @@
       }

       mdata = emalloc(sizeof(*mdata));
-
-       for (i = 0; i < 6; i++) {
+
+       for (i = 0; i < 8; i++) {
+                if (i >= 6 && (args[i] == NULL || ZVAL_IS_NULL(*args[i]))) {
+                        mdata->names[i] = NULL;
+                        continue;
+                }
               ZVAL_ADDREF(*args[i]);
               mdata->names[i] = *args[i];
       }
-
       PS(mod_data) = (void *) mdata;

       RETURN_TRUE;
Index: ext/session/mod_mm.c
===================================================================
--- ext/session/mod_mm.c        (リビジョン 288932)
+++ ext/session/mod_mm.c        (作業コピー)
@@ -445,6 +445,42 @@
       return SUCCESS;
 }

+PS_VALIDATE_SID_FUNC(mm)
+{
+       PS_MM_DATA;
+       ps_sd *sd;
+       const char *p;
+       char c;
+       int ret = SUCCESS;
+
+       for (p = key; (c = *p); p++) {
+               /* valid characters are a..z,A..Z,0..9 */
+               if (!((c >= 'a' && c <= 'z')
+              || (c >= 'A' && c <= 'Z')
+              || (c >= '0' && c <= '9')
+              || c == ','
+              || c == '-')) {
+                       return FAILURE;
+               }
+       }
+
+    if (!PS(use_strict_mode)) {
+        return SUCCESS;
+    }
+
+       mm_lock(data->mm, MM_LOCK_RD);
+
+       sd = ps_sd_lookup(data, key, 0);
+       if (sd) {
+        mm_unlock(data->mm);
+               return SUCCESS;
+       }
+
+       mm_unlock(data->mm);
+
+       return FAILURE;
+}
+
 #endif

 /*
Index: ext/session/mod_user.c
===================================================================
--- ext/session/mod_user.c      (リビジョン 288932)
+++ ext/session/mod_user.c      (作業コピー)
@@ -23,7 +23,7 @@
 #include "mod_user.h"

 ps_module ps_mod_user = {
-       PS_MOD(user)
+       PS_MOD_SID(user)
 };

 #define SESS_ZVAL_LONG(val, a)                                         \
@@ -167,6 +167,83 @@
       FINISH;
 }

+PS_CREATE_SID_FUNC(user)
+{
+       int i;
+        char *val = NULL;
+       zval *retval;
+       ps_user *mdata = PS_GET_MOD_DATA();
+
+    if (!mdata)
+               return estrndup("", 0);
+
+    if (PSF(create) == NULL || ZVAL_IS_NULL(PSF(create))) {
+        return php_session_create_id(mod_data, newlen TSRMLS_CC);
+    }
+       retval = ps_call_handler(PSF(create), 0, NULL TSRMLS_CC);
+
+       if (retval) {
+               if (Z_TYPE_P(retval) == IS_STRING) {
+                       val = estrndup(Z_STRVAL_P(retval), Z_STRLEN_P(retval));
+               } else {
+            val = estrndup("", 0);
+        }
+               zval_ptr_dtor(&retval);
+       } else {
+        val = estrndup("", 0);
+    }
+
+    return val;
+}
+
+static int ps_user_valid_key(const char *key TSRMLS_DC)
+{
+       size_t len;
+       const char *p;
+       char c;
+       int ret = SUCCESS;
+
+       for (p = key; (c = *p); p++) {
+               /* valid characters are a..z,A..Z,0..9 */
+               if (!((c >= 'a' && c <= 'z')
+              || (c >= 'A' && c <= 'Z')
+              || (c >= '0' && c <= '9')
+              || c == ','
+              || c == '-')) {
+                       ret = FAILURE;
+                       break;
+               }
+       }
+
+       len = p - key;
+
+       if (len == 0)
+               ret = FAILURE;
+
+       return ret;
+}
+
+PS_VALIDATE_SID_FUNC(user)
+{
+       zval *args[1];
+       STDVARS;
+
+        if (PSF(validate) == NULL || ZVAL_IS_NULL(PSF(validate))) {
+                return ps_user_valid_key(key TSRMLS_CC);
+        }
+       SESS_ZVAL_STRING(key, args[0]);
+
+       retval = ps_call_handler(PSF(validate), 1, args TSRMLS_CC);
+
+       if (retval) {
+               convert_to_long(retval);
+               ret = Z_LVAL_P(retval) ? SUCCESS : FAILURE;
+               zval_ptr_dtor(&retval);
+       }
+
+        return ret;
+}
+
 /*
 * Local variables:
 * tab-width: 4
Index: ext/sqlite/sess_sqlite.c
===================================================================
--- ext/sqlite/sess_sqlite.c    (リビジョン 288932)
+++ ext/sqlite/sess_sqlite.c    (作業コピー)
@@ -189,6 +189,70 @@
       return SQLITE_RETVAL(rv);
 }

+
+PS_VALIDATE_SID_FUNC(sqlite)
+{
+       PS_SQLITE_DATA;
+       char *query;
+       const char *tail;
+       sqlite_vm *vm;
+       int colcount, result;
+       const char **rowdata, **colnames;
+       char *error;
+    char *p, c;
+
+       int ret = FAILURE;
+
+       for (p = key; (c = *p); p++) {
+               /* valid characters are a..z,A..Z,0..9 */
+               if (!((c >= 'a' && c <= 'z')
+              || (c >= 'A' && c <= 'Z')
+              || (c >= '0' && c <= '9')
+              || c == ','
+              || c == '-')) {
+                       return FAILURE;
+               }
+       }
+
+    if (!PS(use_strict_mode)) {
+        return SUCCESS;
+    }
+
+       query = sqlite_mprintf("SELECT value FROM session_data WHERE
sess_id='%q' LIMIT 1", key);
+       if (query == NULL) {
+               /* no memory */
+               return FAILURE;
+       }
+
+       if (sqlite_compile(db, query, &tail, &vm, &error) != SQLITE_OK) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "SQLite: Could not
compile session validate query: %s", error);
+               sqlite_freemem(error);
+               sqlite_freemem(query);
+               return FAILURE;
+       }
+
+       switch ((result = sqlite_step(vm, &colcount, &rowdata, &colnames))) {
+               case SQLITE_ROW:
+                       if (rowdata[0] != NULL) {
+                ret = SUCCESS;
+                       }
+                       break;
+               default:
+                       sqlite_freemem(error);
+                       error = NULL;
+       }
+
+       if (SQLITE_OK != sqlite_finalize(vm, &error)) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "SQLite: session
validate: error %s", error);
+               sqlite_freemem(error);
+               error = NULL;
+       }
+       sqlite_freemem(query);
+
+       return ret;
+}
+
+
 #endif /* HAVE_PHP_SESSION && !defined(COMPILE_DL_SESSION) */

 /*

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to