Re: environ function patch for review

On Thu, Dec 03, 2009 at 06:04:47PM -0600, Sean C. Farley wrote:
> Regarding the recent security issue with the unsetenv() calls in rtld, I 
> have made a patch[1] I would like reviewed prior to commit.  It changes 
> the behavior of all the *env() routines that cause an internal 
> environment to be created.  This is putenv(), setenv() and unsetenv(). 
> getenv() will not cause an internal environment to be created.  I have 
> tested the patch without the rltd fix, and it prevents the security 
> issue.

> Instead of returning an error when tripping upon a corrupt environment, 
> it will return an error when the caller passes bad argument(s) (EINVAL) 
> or if unable to allocate memory (ENOMEM).  Except for the possibility 
> for ENOMEM, this should make the behavior the same as FreeBSD 6 and 
> below.

It would be very nice to avoid ENOMEM on unsetenv() somehow, such as by
rewriting environ in place. Neither FreeBSD6 nor POSIX mention the
possibility of ENOMEM on unsetenv(). The only error listed by POSIX is
an invalid variable name (unsetting a variable that does not exist is
not an error), so it seems reasonable to assume unsetenv() is successful
if passed a valid constant variable name.

If unsetenv() has to copy the environment, there cannot be any setenv()
or putenv() strings in it. So it seems correct to remove the pointer
from environ without doing anything else, instead.

If the environment has already been copied, the unsetenv() should
proceed without needing to allocate any memory. This seems to be the
case.

-- 
Jilles Tjoelker