[PATCH] Bug #39751: putenv causes string copy of freed memory region, causing crash

I wish I could write bug reports this good!
:-)

Nice work!

============ 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.

--
Some people have a "gift" link here.
Know what I want?
I want you to buy a CD from some starving artist.
http://cdbaby.com/browse/from/lynch
Yeah, I get a buck. So?