Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:26876
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain l-i-e.com from 67.139.134.202 cause and error)
Message-ID: <61694.209.254.223.2.1165694639.squirrel@www.l-i-e.com>
In-Reply-To: <011a01c718f6$996ac040$6400a8c0@S0HOMbvkxh1>
References:  <011a01c718f6$996ac040$6400a8c0@S0HOMbvkxh1>
Date: Sat, 9 Dec 2006 14:03:59 -0600 (CST)
To: "Kevin Hoffman" <kevinjohnhoffman@gmail.com>
Cc: internals@lists.php.net
Reply-To: ceo@l-i-e.com
User-Agent: Hostbaby Webmail
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-8859-1
Content-Transfer-Encoding: 8bit
Importance: Normal
Subject: Re: [PHP-DEV] [PATCH] Bug #39751: putenv causes string copy of 
     freed memory region, causing crash
From: ceo@l-i-e.com ("Richard Lynch")

I wish I could write bug reports this good!
:-)

Nice work!

On Tue, December 5, 2006 11:22 pm, Kevin Hoffman wrote:
> ============ 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.
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php


-- 
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?