Bug #7076
closed
segfault in putenv + getenv
100%
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.
Updated by Toomas Soome over 7 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.
Updated by Electric Monk over 7 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>