Deal with environment variables w/o =

[dmik]

The standard environment variable format for DosExecPgm is NAME=VAL. However, It turns out that DosExecPgm accepts environment variables which only contain the NAME part (no VAL part and not even a = character). This makes the standard C environ array contain such variables as well. The setenv code supports such a case when looking for an existing variable but when assigning its new value it performs an out of bounds access and exposes a serious security risk with an arbitrary memory overwrite. The following program will demonstrate this:

// save to test_env.c and compile with gcc -Zomf test_env.c

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <process.h>

int main(int argc, char **argv)
{
  if (argc > 1)
  {
    // child
    char **e = environ;
    while (*e)
      printf ("environ[%s]\n", *e++);
    setenv("ENV1", "1", 1);
    e = environ;
    while (*e)
      printf ("environ[%s]\n", *e++);
  }
  else
  {
    char * arg [] = { "test", "child", NULL };
    char * env [] = { "ENV1", "ENV2=VAL2", NULL };
    int rc = spawnve (P_WAIT, "test_env.exe", arg, env);
    if (rc == -1)
      perror ("spawnve");
  }

  return 0;
}

The expected output:

environ[ENV1]
environ[ENV2=VAL2]
environ[ENV1=1]
environ[ENV2=VAL2]

The actual output:

environ[ENV1]
environ[ENV2=VAL2]
environ[ENV1]
environ[1]

I.e. a call to setenv("ENV1", "1", 1); overwrites the memory contents after ENV1 with a new value (1). In this particular case another environment variable, ENV2, comes after ENV1 and it just vanishes. It can be any other memory location in the data segment, depending on the C heap usage.

Discovered when working on https://github.com/bitwiseworks/libcx/issues/91.

[dmik]

Also, from what I see, setenv will expose memory leaks a certain variable is set more than once and the new value is longer than the previous one. I will try to fix that too within this ticket.

[dmik]

Created #103 to track setenv memory leaks.

[dmik]

There is a version of the test showing the problem w/o using the parent-child mechanism:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

char str [] = "ENV1\0ENV2=VAL2";

char * newenv [] = { str, str + 5, NULL };

int main(int argc, char **argv)
{
  environ = newenv;

  char * * e = environ;
  while (*e)
    printf ("environ[%s]\n", *e++);

  setenv("ENV1", "1", 1);

  e = environ;
  while (*e)
    printf ("environ[%s]\n", *e++);

  return 0;
}

[dmik]

Note that the first testcase works fine after just efb19af (because dead variables are no longer passed to the child) so a second test case was necessary which adds them manually to environ (something an app can still legally do). The second test case works well after 42ba446. Closing.