SEI CERT C Coding Standard

Space shortcuts

Page tree

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);
}

According to the [Open Group 2004] entry for putenv():

...the string pointed to by string shall become part of the environment, so altering the string shall change the environment.

This means that the call to putenv() is only necessary the first time func() is called, since subsequent changes to the string update the environment. If func() were called more than once, an additional variable could be added to avoid calling it unnecessarily.

Compliant Solution (Heap Memory)

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

int func(const char *var) {
  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;
  }
  return 0;
}

The POSIX setenv() function is preferred over this function [Open Group 2004]. In particular, using putenv() will necessarily leak memory if called multiple times for the same environment variable, due to restrictions on when you can safely free the old value. According to the [Open Group 2004] entry for putenv():

Although the space used by string is no longer used once a new string which defines name is passed to putenv(), if any thread in the application has used getenv() to retrieve a pointer to this variable, it should not be freed by calling free(). If the changed environment variable is one known by the system (such as the locale environment variables) the application should never free the buffer used by earlier calls to putenv() for the same variable.

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

Astrée
24.04

Supported: Can be checked with appropriate analysis stubs.
Axivion Bauhaus Suite

7.2.0

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




Helix QAC

2024.1

C5024
Klocwork

2024.1

CERT.PUTENV.AUTO_VARIABLE


Parasoft C/C++test
2023.1

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

PC-lint Plus

1.4

2601

Fully supported

Polyspace Bug Finder

R2023b

CERT C: Rule POS34-CChecks for use of automatic variable as putenv-family function argument (rule fully covered)

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()


25 Comments

  1. David Svoboda

    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. David Keaton

      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. Jonathan Leffler

      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. Robert Seacord (Manager)

        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. Douglas A. Gwyn

    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. Robert Seacord

      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. Geoff Clare

      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. Robert Seacord

        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. Alex Volkovitsky

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

    1. David Svoboda

      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. Alex Volkovitsky

        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. Stefan Kueppers

    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. David Svoboda

      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. Geoff Clare

        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. David Svoboda

          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. Geoff Clare

            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. David Svoboda

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

  5. Joseph C. Sible

    I don't like the static or heap memory compliant solutions. In the static one, the call to putenv is redundant (and I'm not sure if it's even safe) on all calls to that function except the first, since POSIX says "altering the string shall change the environment". In the heap memory one, we free() the old value even though POSIX says this:

    Although the space used by string is no longer used once a new string which defines name is passed to putenv(), if any thread in the application has used getenv() to retrieve a pointer to this variable, it should not be freed by calling free(). If the changed environment variable is one known by the system (such as the locale environment variables) the application should never free the buffer used by earlier calls to putenv() for the same variable.

    I think we should add static _Bool called_putenv to the static one so that putenv is only called once, and we should remove the logic to free the old value from the heap memory one.

    1. David Svoboda

      Joe:
      Now that you have editing superpowers, why don't you try fixing these two CS's? Here are my suggestions:

      * I agree with you that in the static CS the call to 'putenv()' is redundant after the first call to func(). However, don't change the code as it is more readable with putenv() than with it wrapped in an if statement. Instead add some text (after the CS explaining this issue wrt putenv().

      * For the dynamic CS you can replace the call to free() with a comment saying something like "don't free...see below", and provide some text explaining the issue. Include the POSIX quote you cited. Be sure to explain why the CS doesn't violate MEM31-C.

      1. Joseph C. Sible

        "Be sure to explain why the CS doesn't violate MEM31-C." It does violate MEM31-C. That's why I commented over there that we need an exception for this.

        1. David Svoboda

          We do need an exception, but not in the way you suggest:

          In particular, putenv() is a pathological system call, and has no similarly pathological function in ISO C which is why we don't address it in MEM31-C. Since putenv() imposes a pathological constraint on the lifetime of its argument, I would argue that putenv() itself is culpable in violating MEM31-C, rather than any code that calls putenv(). That's why the CS should not free the pointer. That's also one reason that people should prefer setenv() or putenv().

          1. Joseph C. Sible

            "I would argue that putenv() itself is culpable in violating MEM31-C, rather than any code that calls putenv()." Fair enough. I updated this accordingly.

            1. David Svoboda

              It might be worthwhile to add a recommendation (and possibly a rule) that says "if your function imposes constraints on the lifetime of its pointer arguments, you must abide by them", and cite putenv() as an example.  I wouldn't add that to MSC31-C, although it could reference the new guideline.

      2. David Svoboda

        Joseph C. Sible :  why did you add this line? What is wrong with calling putenv() multiple times on the same pointer?

        Note that func should only be called once in this example, since otherwise putenv() will be called multiple times with the same pointer.

        Your other changes look good to me.

        1. Joseph C. Sible

          I thought I could make it break somehow with it, but everywhere I've looked/tried it, it does just seem to be redundant. I adjusted it accordingly.