============ Bug summary ============
If putenv deletes an environment variable (putenv("VAR=")) after it was set
previously (putenv("VAR=xxx")) then pe->previous_entry (pe is the internal hash
table entry for the environment variable) will point to a freed memory region.
This bug is triggered on windows because pe->previous_entry is set to point
directly to the environment string (environ[...]) instead of a *copy* of the
string. The MSVCRT library will free that string when an environment variable
is deleted. Thus, pe->previous_entry can point to a free memory region when it
is used later in the putenv_ht hashtable deconstructor.
============ Impact ============
Allows possible denial of service attack by crashing Apache.
============ PHP Versions ============
I've used this bug to crash Apache using PHP 5.1.2, 5.1.4, 5.2, and the latest
version from CVS.
============ Bug Fix List ============
Fixes bug #39751 and probably bug# 36819.
============ Patch Details ============
PHP_FUNCTION(putenv) is called the first time. The TZ environment variable
value is set and an entry is created in the putenv_ht hashtable with
pe.previous_value=NULL.
Now the second call to PHP_FUNCTION(putenv) it first executes:
basic_functions.c line 4416
zend_hash_del(&BG(putenv_ht), pe.key, pe.key_len+1);
This deletes the old entry from the putenv_ht hash table (ok). Then it looks
through the C environment to see if there is a previous value
(basic_functions.c lines 4418 - 4425).
Because this is the second time through there is a previous value for the TZ
environment variable (in my case, it's US/Eastern).
So pe.previous_value = *env on line 4422:
pe.previous_value = *env;
Note that its directly pointing to the string managed by the C runtime. This is
the root cause of the bug (crash).
Now it sets the environment variable using the new value, using the putenv lib
call. (basic_functions.c line 4435) (In this case, "TZ="). Because we're
removing the value of the environment variable the bug will be triggered.
putenv eventually calls __crtsetenv
In __crtsetenv the remove variable is set to 1, because the string is "TZ="
setenv.c line 94
remove = (*(equal + 1) == _T('\0'));
The memory for the old environment variable is freed. Any pointers that point
to env[ix] are now INVALID pointers. This includes the pe.previous_value
pointer!
setenv.c line 183
_free_crt(env[ix]);
Now the pe information is inserted into the putenv_ht hashtable. Note that the
pe.previous_value field is now pointing to a freed block of memory (in debug
builds, this is immediately set to 0xDD -- in release builds it still points to
the old data until that memory region is allocated and overwritten).
The next time the TZ environment variable is set or when the PHP interpreter is
exiting and cleaning up (zm_deactivate_basic), the destructor on the old hash
table entry will be called (the php_putenv_destructor function).
It then checks to see if there is a previous value (if previous_value is
nonzero). If it is, it calls putenv with the previous value.
basic_functions.c lines 3846 - 3855
...
putenv(pe->previous_value);
...
However, the pe.previous_value variable points to a freed block of memory.
putenv immediately tries to copy the new value of the environment variable
(pe->previous_value).
putenv.c lines 127 - 129:
if ( (newoption = (_TSCHAR *)_malloc_crt((_tcslen(option)+1) *
sizeof(_TSCHAR))) == NULL )
return -1;
If the new value (pe->previous_value) has been overwritten and is no longer
zero-terminated then the call to _tcslen (strlen) can possibly keep reading
memory until it hits an invalid memory region. At this point it causes an
invalid memory access fault.
============ Test Case ============
It's hard to reproduce this bug without a lot of complex code in between the
place where the environment variable is set to nothing and the place where it's
set to a value again. I have written an example script that causes some memory
access assertions sometimes.
Index: ext/standard/basic_functions.c
===================================================================
RCS file: /repository/php-src/ext/standard/basic_functions.c,v
retrieving revision 1.829
diff -u -r1.829 basic_functions.c
--- ext/standard/basic_functions.c 5 Dec 2006 04:52:44 -0000 1.829
+++ ext/standard/basic_functions.c 6 Dec 2006 05:13:02 -0000
@@ -3852,6 +3852,9 @@
SetEnvironmentVariable(pe->key, "bugbug");
#endif
putenv(pe->previous_value);
+# if defined(PHP_WIN32)
+ efree(pe->previous_value);
+# endif
} else {
# if HAVE_UNSETENV
unsetenv(pe->key);
@@ -4419,7 +4422,12 @@
pe.previous_value = NULL;
for (env = environ; env != NULL && *env != NULL; env++) {
if (!strncmp(*env, pe.key, pe.key_len) &&
(*env)[pe.key_len] == '=') { /* found it */
+#if defined(PHP_WIN32)
+ /* must copy previous value because MSVCRT's
putenv can free the string without notice */
+ pe.previous_value = estrndup(*env, 1024);
+#else
pe.previous_value = *env;
+#endif
break;
}
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
