Skip to end of metadata
Go to start of metadata

The POSIX function putenv() is used to set environment variable values. The putenv() function does not create a copy of the string supplied to it as an argument; rather, it inserts a pointer to the string into the environment array. If a pointer to a buffer of automatic storage duration is supplied as an argument to putenv(), the memory allocated for that buffer may be overwritten when the containing function returns and stack memory is recycled. This behavior is noted in the Open Group Base Specifications, Issue 6 [Open Group 2004]:

A potential error is to call putenv() with an automatic variable as the argument, then return from the calling function while string is still part of the environment.

The actual problem occurs when passing a pointer to an automatic variable to putenv(). An automatic pointer to a static buffer would work as intended.

Noncompliant Code Example

In this noncompliant code example, a pointer to a buffer of automatic storage duration is used as an argument to putenv() [Dowd 2006]. The TEST environment variable may take on an unintended value if it is accessed after func() has returned and the stack frame containing env has been recycled.

Note that this example also violates DCL30-C. Declare objects with appropriate storage durations.

int func(const char *var) {
  char env[1024];
  int retval = snprintf(env, sizeof(env),"TEST=%s", var);
  if (retval < 0 || (size_t)retval >= sizeof(env)) {
    /* Handle error */
  }

  return putenv(env);
}

Compliant Solution (static)

This compliant solution uses a static array for the argument to putenv().

int func(const char *var) {
  static char env[1024];

  int retval = snprintf(env, sizeof(env),"TEST=%s", var);
  if (retval < 0 || (size_t)retval >= sizeof(env)) {
    /* Handle error */
  }

  return putenv(env);
}

Compliant Solution (Heap Memory)

This compliant solution dynamically allocates memory for the argument to putenv():

int func(const char *var) {
  static char *oldenv; 
  const char *env_format = "TEST=%s";
  const size_t len = strlen(var) + strlen(env_format);
  char *env = (char *) malloc(len);
  if (env == NULL) {
    return -1;
  }
  int retval = snprintf(env, len, env_format, var);
  if (retval < 0 || (size_t)retval >= len) {
    /* Handle error */
  }
  if (putenv(env) != 0) {
    free(env);
    return -1;
  }
  if (oldenv != NULL) {
    free(oldenv); /* avoid memory leak */ 
  }
  oldenv = env; 
  return 0;
}

The POSIX setenv() function is preferred over this function [Open Group 2004].

Compliant Solution (setenv())

The setenv() function allocates heap memory for environment variables, which eliminates the possibility of accessing volatile stack memory:

int func(const char *var) {
  return setenv("TEST", var, 1);
}

Using setenv() is easier and consequently less error prone than using putenv().

Risk Assessment

Providing a pointer to a buffer of automatic storage duration as an argument to putenv() may cause that buffer to take on an unintended value. Depending on how and when the buffer is used, it can cause unexpected program behavior or possibly allow an attacker to run arbitrary code.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

POS34-C

high

unlikely

medium

P6

L2

Automated Detection

Tool

Version

Checker

Description

Axivion Bauhaus Suite

6.9.0

CertC-POS34
CodeSonar
5.1p0
(customization)Users can add a custom check for all uses of putenv().
Compass/ROSE




Parasoft C/C++test
10.4.2

CERT_C-POS34-a
CERT_C-POS34-b

Usage of system properties (environment variables) should be restricted
Do not call putenv() with a pointer to an automatic variable as the argument

Polyspace Bug Finder

R2019b

CERT C: Rule POS34-CChecks for use of automatic variable as putenv-family function argument (rule fully covered)
PRQA QA-C
9.5
5024Partially implemented

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

Key here (explains table format and definitions)

Taxonomy

Taxonomy item

Relationship

CERT-CWE Mapping Notes

Key here for mapping notes

CWE-252/CWE-253/CWE-391 and ERR33-C/POS34-C

Independent( ERR33-C, POS54-C, FLP32-C, ERR34-C)

Intersection( CWE-252, CWE-253) = Ø

CWE-391 = Union( CWE-252, CWE-253)

CWE-391 = Union( ERR33-C, POS34-C, list) where list =

  • Ignoring return values of functions outside the C or POSIX standard libraries

Bibliography

[Dowd 2006]Chapter 10, "UNIX Processes"
[ISO/IEC 9899:2011]Section 6.2.4, "Storage Durations of Objects"
Section 7.22.3, "Memory Management Functions"
[Open Group 2004]putenv()
setenv()



17 Comments

  1. Here's a question. Is this an example of secure code that uses putenv? (Specifically, it gives putenv a dynamically-allocated pointer.)

    int POS34_c(char *var) {
    char env[1024];

    if (snprintf(env, sizeof(env),"TEST=%s", var) < 0) {
    /* Handle Error */
    }
    size_t len = strlen( env);
    char* env2 = NULL;
    env2 = (char*) calloc( len, sizeof( char));
    strncpy( env2, env, len);

    return putenv(env2);
    }

    1. Yes, this is a valid solution.  However, it causes a memory leak if TEST is ever redefined, since the string pointed to by env2 will never be freed.  The putenv() function only frees memory that putenv() itself allocated.

      I have changed the language in the rule to clarify the difference between an automatic variable and a pointer to an automatic variable in the context of putenv().  The POSIX standard was slightly inaccurate on this point.

    2. No - the space allocated is one byte short of what is needed.

      If you fix the off-by-one bug, then yes, it is fine.  Note, however, that if you subsequently set the value again, you are leaking memory.  Actually, it is quite hard to avoid leaking memory with putenv().  One of the advantages of setenv() is that it does avoid such leakages.

      1. I'm quite confused by this.

        POSIX Draft 5 says:

        "The standard developers noted that putenv( ) is the only function available to add to the
        environment without permitting memory leaks."

        Although I have seen older documents which claim both can leak.

  2. I don't see the point of the implementation details, and think they should be removed.

    A compliant solution using putenv is
    int func(char *var) {
      char env[1024];
      static char *p;
      if (snprintf(env, sizeof(env),"TEST=%s", var) < 0) {
        /* Handle Error */
      }
      if (p != NULL)
        free(p); // avoid memory leak
      if ((p = strdup(env)) == NULL) {
        /* Handle Error */
      }
      return putenv(p);
    }

    1. Implementation Details

      The putenv() function is not required to be thread-safe, and the one in libc4, libc5 and glibc2.0 is not, but the glibc2.1 version is.

      Description for libc4, libc5, glibc: If the argument string is of the form name, and does not contain an `=' character, then the variable name is removed from the environment. If putenv() has to allocate a new array environ, and the previous array was also allocated by putenv(), then it will be freed. In no case will the old storage associated to the environment variable itself be freed.

      The libc4 and libc5 and glibc 2.1.2 versions conform to SUSv2: the pointer argument given to putenv() is used. In particular, this string becomes part of the environment; changing it later will change the environment. (Thus, it is an error to call putenv() with a pointer to a buffer of automatic storage duration as the argument, then return from the calling function while the string is still part of the environment.) However, glibc 2.0-2.1.1 differs: a copy of the string is used. On the one hand this causes a memory leak, and on the other hand it violates SUSv2. This has been fixed in glibc2.1.2.

      The BSD4.4 version, like glibc 2.0, uses a copy.

      SUSv2 removes the `const' from the prototype, and so does glibc 2.1.3.

      The FreeBSD implementation of putenv() copies the value of the provided string, and the old values remain accessible indefinitely. As a result, a second call to putenv() assigning a differently sized value to the same name results in a memory leak.

    2. The code should not free the old buffer until after the putenv(). Otherwise when putenv() searches environ to see if the TEST variable already exists, it will try to access the freed memory.

      Is there some reason for doing the snprintf() to a local buffer and then copying it to a buffer on the heap, rather than just using malloc() to obtain a buffer of the necessary length and doing the snprintf() straight to that buffer?

      Also, the snprintf() return value check does not detect truncation.

      1. I can't think of a reason why this needs to be done on the stack.

        Please feel free to take another crack at the putenv() example.

  3. In the putenv() CS, is there any algorithmic way for ROSE to know that oldenv is allocated dynamically?

    1. In the CS, it obviously is allocated dynamically, as it is the result of a call to malloc(). In general, one usually can't tell, as it may be a paramter, or may be assigned from another variable...one would need runtime dataflow analysis to know for sure.

      1. It is not obvious that it is allocated dynamically. There aren't any writes to oldenv before the call to free(). oldenv gets the value of env which is allocated dynamically, but only after the line with free. That is not obvious to a static checker.

        Currently, I am having ROSE bail on global and static vars.

  4. The compilent sample is wrong, because memory for env was freed.

    Here is a "compilent" sampe, if you use only one environment variable.

    The only difference is the "static" definition.

    int func(const char *var) {
      static char env[1024];
    
      if (snprintf(env, sizeof(env),"TEST=%s", var) < 0) {
        /* Handle error */
      }
      return putenv(env);
    }
    

    If you have to set more than one environmet variable, you can define env as an array of strings.
    Always: Keep your solution small and simple!

    1. Not sure what problem you're citing. I did find a problem in that the oldenv var was freed, while never being initialized, so I took it out. There might be a memory leak now, if the old environment variable was also allocated on the heap, but that is less harmful than freeing memory twice or freeing memory not on the heap.

      I also added your code sample as another compliant solution...thanks!

      1. The oldenv var was statically initialized. I can't see any problem with that old code.

        There are some oddities concerning snprintf(). The heap memory compliant solution has a check on whether the snprintf() return was larger than the buffer. This can't happen because the buffer was sized to be large enough (although the check doesn't do any harm and should probably stay.) However, the two earlier examples do not have this check but do need it.

        1. What value was oldenv statically initialized to?

          The C99 {[snprintf()}} method prevents buffer overflows, and right now it is present in all examples (except the setenv one). You're right, it could be replaced with old sprintf() for the heap example. But the first two examples need it to prevent buffer overflow.

          1. What value was oldenv statically initialized to?

            A null pointer, of course.

            The C99 {[snprintf()}} method prevents buffer overflows, and right now it is present in all examples (except the setenv one). You're right, it could be replaced with old sprintf() for the heap example. But the first two examples need it to prevent buffer overflow.

            I wasn't suggesting using sprintf(). My point was that if the fixed-size buffer in those examples is not large enough, snprintf() will return a value larger than the buffer size but the code will not detect that this happened.

            1. I see. I keep forgetting that static vars are never uninitialized. Brought back oldenv, and added the snprintf checks you suggest.