Description:
------------
Introduction
------------

The file based session handler uses the session.save_path ini variable to determine where to write it's session files.

However, it also attempts to parse two other bits of information out of session.save_path, iff session.save_path contains one or two semi-colons. The format is:

"[x;[y;]]pathname"

Where both x and y are optional and x is a directory hashing depth for session files (defaults to 0) and y is the create mode for new session files, defaults to 0700.

Whenever session.save_path is updated, a verification function is called. For this variable, this is the funtion "static PHP_INI_MH(OnUpdateSaveDir)" contained in ext/session/session.c.

There is no verification in this function if PHP is running in regular mode. However, when in safe-mode, this function performs two verifications. First, it attempts to check ownership of the directory, and secondly, it does a php_check_open_basedir() on the path supplied.

Problem description
-------------------

In this function, OnUpdateSaveDir() in ext/session/session.c, no attempt is made to recognize and take out the optional parameters contained in the supplied new session.save_path value.

So, when setting session.save_path to "0;0707;/whatever", PHP will supply php_checkuid() and php_check_open_basedir with the entire string, not just with /whatever. This causes the check to fail.

Patch
------

The following patch should be applied to ext/session/session.c to fix this problem:

*** ext/session/session.orig.c  Fri Sep 29 11:35:05 2006
--- ext/session/session.c       Fri Sep 29 12:39:26 2006
***************
*** 133,145 ****
  
  static PHP_INI_MH(OnUpdateSaveDir)
  {
        /* Only do the safemode/open_basedir check at runtime */
        if (stage == PHP_INI_STAGE_RUNTIME) {
!               if (PG(safe_mode) && (!php_checkuid(new_value, NULL, CHECKUID_ALLOW_ONLY_DIR))) {
                        return FAILURE;
                }
  
!               if (php_check_open_basedir(new_value TSRMLS_CC)) {
                        return FAILURE;
                }
        }
--- 133,150 ----
  
  static PHP_INI_MH(OnUpdateSaveDir)
  {
+       char *p, *q;
+       int i;
        /* Only do the safemode/open_basedir check at runtime */
        if (stage == PHP_INI_STAGE_RUNTIME) {
!               /* Parse out optional hash level & file mode */
!               for (i=0, q=new_value; i<2 && (p=strchr(q, ';'))!=NULL; q=p+1, i++);
! 
!               if (PG(safe_mode) && (!php_checkuid(q, NULL, CHECKUID_ALLOW_ONLY_DIR))) {
                        return FAILURE;
                }
  
!               if (php_check_open_basedir(q TSRMLS_CC)) {
                        return FAILURE;
                }
        }

Remarks
--------

Both parameters are, at least for the most part, undocumented. It would not be a bad idea to update
the session.save_path description with information about these two variables.

Submitted: 29/9/2006
Patch author: Roel Bouwman <roel@bouwman.net>

Reproduce code:
---------------
<?php
 /* Preconditions:
  *  - server running in safe mode
  *  - /whatever and this script owned by samed user.
  */

  ini_set("session.save_path","0;0707;/whatever");
?>

Expected result:
----------------
1. PHP checking wether "/whatever" is owned by script owner.
2. PHP performing basepath check for "/whatever"
3. session.save_path set to "0;0707;/whatever".

Actual result:
--------------
PHP will produce an error claiming that it cannot open the save_path directory as the uid check is performed not just for /whatever, but for the entire string.

Result is that session.save_path will not be modified.