Project

General

Profile

Bug #7076

segfault in putenv + getenv

Added by Robert Mustacchi almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
2016-06-07
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

We encountered a case where a program was running and ended up segfaulting. We were able to shrink it to something small:

#include <stdlib.h>

int
main ()
{
  if (putenv ("hello"))
    return 2;

  if (getenv ("hello") != 0)
    return 3;

  return 0;
}

which segfaults in the getenv as far as I can tell. It's not clear to me what the correct behavior is in this case.


The problem here is that putenv() is basically assuming valid input and
then gets thrown off on a later getenv when it doesn't encounter the '='
sign. This could potentially be rather bad. We have two general
behaviors. We could be like glibc and treat this as a request to remove
it from the environment. Or we could be like various BSDs and apparently
return EINVAL during putenv().

However, because this has been traditionally valid and doesn't result in
the program blowing up unless you ask for the thing you just removed, it
does not seem reasonable to start calling EINVAL here and instead we
should just treat this like an unsetenv() ala glibc.

History

#1

Updated by Toomas Soome almost 4 years ago

Robert Mustacchi wrote:

We encountered a case where a program was running and ended up segfaulting. We were able to shrink it to something small:

[...]

which segfaults in the getenv as far as I can tell. It's not clear to me what the correct behavior is in this case.


The problem here is that putenv() is basically assuming valid input and
then gets thrown off on a later getenv when it doesn't encounter the '='
sign. This could potentially be rather bad. We have two general
behaviors. We could be like glibc and treat this as a request to remove
it from the environment. Or we could be like various BSDs and apparently
return EINVAL during putenv().

However, because this has been traditionally valid and doesn't result in
the program blowing up unless you ask for the thing you just removed, it
does not seem reasonable to start calling EINVAL here and instead we
should just treat this like an unsetenv() ala glibc.

however, putenv(3C) states name=value as value and is talking only about updating/creating, not deleting. Same is here: http://pubs.opengroup.org/onlinepubs/9699919799/functions/putenv.html

I can see the temptation to follow glibc, altho Im not sure how good idea this would be. Also putenv() interface is dangerous because of the string memory management and static/automatic scope etc, so the faults are sort of "programmed" in this interface.

#2

Updated by Electric Monk almost 4 years ago

  • Status changed from New to Closed

git commit 60b81b86c4b2eb3a0481176c344f4b6e7a6276fa

commit  60b81b86c4b2eb3a0481176c344f4b6e7a6276fa
Author: Robert Mustacchi <rm@joyent.com>
Date:   2016-08-03T16:30:20.000Z

    7076 segfault in putenv + getenv
    Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
    Reviewed by: Dave Eddy <dave.eddy@joyent.com>
    Reviewed by: Cody Mello <melloc@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Gordon Ross <gordon.ross@nexenta.com>
    Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
    Approved by: Dan McDonald <danmcd@omniti.com>

Also available in: Atom PDF