php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69033 Request may get env. variables from previous requests if PHP works as FastCGI
Submitted: 2015-02-12 04:31 UTC Modified: 2015-03-17 23:55 UTC
From: sergey dot prokhorov at gmail dot com Assigned: ab
Status: Closed Package: Scripting Engine problem
PHP Version: 5.5.21 OS: Windows
Private report: No CVE-ID:
 [2015-02-12 04:31 UTC] sergey dot prokhorov at gmail dot com
Description:
------------
PHP 5.5.21 Non Thread Safe

Steps to reproduce:
1. Add FastCgiModule handler with php-cgi.exe executable on IIS site
2. Put script from next section to this site
3. Kill all FastCGI child processes: restart IIS App pool used by site or just exec taskkill /f /im php-cgi.exe
4. Run script twice, ensure that both requests handled by the same process php-cgi.exe. Result will be ex described in Actual result - second request will read value first one.

This bug was made during fix https://bugs.php.net/bug.php?id=66265 and affects only newly created variables.


Test script:
---------------
<?php

$originalValue = getenv('MY_ENV_VAR');
var_dump($originalValue);

$res = putenv('MY_ENV_VAR=SOME_VALUE');
var_dump($res);

$newValue = getenv('MY_ENV_VAR');
var_dump($newValue);

Expected result:
----------------
First request output:
bool(false) bool(true) string(10) "SOME_VALUE"

Second request output:
bool(false) bool(true) string(10) "SOME_VALUE"

Actual result:
--------------
First request output:
bool(false) bool(true) string(10) "SOME_VALUE"

Second request output:
string(10) "SOME_VALUE" bool(true) string(10) "SOME_VALUE"

Patches

bug69033_2.patch (last revision 2015-02-12 11:19 UTC) by ab@php.net)
bug69033_1.patch (last revision 2015-02-12 09:22 UTC) by ab@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-02-12 04:48 UTC] sergey dot prokhorov at gmail dot com
It seems I forgot to attach patch when submit this bug and I cannot attach it to private bug so I add it as comment:

--- ext/standard/basic_functions.c
+++ ext/standard/basic_functions.c
@@ -3421,6 +3421,10 @@ zend_module_entry basic_functions_module = { /* {{{ */
 #if defined(HAVE_PUTENV)
 static void php_putenv_destructor(putenv_entry *pe) /* {{{ */
 {
+# if !defined(ZTS) && defined(PHP_WIN32)
+	int key_len;
+	char *putenv_string;
+#endif
 	if (pe->previous_value) {
 #if _MSC_VER >= 1300
 		/* VS.Net has a bug in putenv() when setting a variable that
@@ -3438,6 +3442,14 @@ static void php_putenv_destructor(putenv_entry *pe) /* {{{ */
 		unsetenv(pe->key);
 # elif defined(PHP_WIN32)
 		SetEnvironmentVariable(pe->key, NULL);
+# ifndef ZTS
+		key_len = strlen(pe->key);
+		putenv_string = (char *) emalloc(key_len + 2);
+		memcpy(putenv_string, pe->key, key_len);
+		memset(putenv_string + key_len, 0, 2);
+		putenv_string[key_len] = '=';
+		_putenv(putenv_string);
+# endif
 # else
 		char **env;
 [2015-02-12 09:21 UTC] ab@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: ab
 [2015-02-12 09:22 UTC] ab@php.net
The following patch has been added/updated:

Patch Name: bug69033_1.patch
Revision:   1423732938
URL:        https://bugs.php.net/patch-display.php?bug=69033&patch=bug69033_1.patch&revision=1423732938
 [2015-02-12 09:25 UTC] ab@php.net
Thanks for reporting, the issue confirmed. I've attached a slightly modified patch, but just posting it to this comment as well so you could check:


======= START =======
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -3432,7 +3432,18 @@ static void php_putenv_destructor(putenv_entry *pe) /* {{{ */
 # if HAVE_UNSETENV
                unsetenv(pe->key);
 # elif defined(PHP_WIN32)
+#  ifndef ZTS
+               int key_len = strlen(pe->key);
+               char *putenv_string = (char *) emalloc(key_len + 2);
+#  endif
                SetEnvironmentVariable(pe->key, NULL);
+#  ifndef ZTS
+               memcpy(putenv_string, pe->key, key_len);
+               putenv_string[key_len] = '=';
+               putenv_string[key_len + 1] = '\0';
+               _putenv(putenv_string);
+               efree(putenv_string);
+#  endif
 # else
                char **env;

======= END =======

This should be applied ASAP.

Thanks.
 [2015-02-12 11:19 UTC] ab@php.net
The following patch has been added/updated:

Patch Name: bug69033_2.patch
Revision:   1423739951
URL:        https://bugs.php.net/patch-display.php?bug=69033&patch=bug69033_2.patch&revision=1423739951
 [2015-02-12 11:20 UTC] ab@php.net
@sergey actually there's a better way to do this, which also avoids additional memory overhead

--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -3433,6 +3433,9 @@ static void php_putenv_destructor(putenv_entry *pe) /* {{{ */
                unsetenv(pe->key);
 # elif defined(PHP_WIN32)
                SetEnvironmentVariable(pe->key, NULL);
+# ifndef ZTS
+               _putenv_s(pe->key, "");
+# endif
 # else
                char **env;
 [2015-02-12 11:47 UTC] sergey dot prokhorov at gmail dot com
Yes, it looks much better. I've applied your patch - it helps. Thanks.
 [2015-02-13 08:06 UTC] stas@php.net
Is 5.4 affected or only 5.5+?
 [2015-02-13 08:47 UTC] ab@php.net
Stas, it's 5.5+ only.

Cheers.
 [2015-03-17 23:55 UTC] stas@php.net
-Status: Verified +Status: Closed
 [2015-03-17 23:55 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.

Looks like this one is fixed
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun May 28 12:01:42 2017 UTC