httpd: memory hole in function addEnv() and more

Dirk Clemens develop at cle-mens.de
Sun Sep 4 00:00:28 UTC 2005


Let's have a look into addEnv() from httpd.c

static void addEnv(const char *name_before_underline,
            const char *name_after_underline, const char *value)
{
  char *s = NULL;
  const char *underline;

  if (!value)
    value = "";
  underline = *name_after_underline ? "_" : "";
  asprintf(&s, "%s%s%s=%s", name_before_underline, underline,
                    name_after_underline, value);
  if(s) {
    putenv(s);
  }
}

************

1.) memory hole

Them memory alloced by asprintf() is never free'd.
Solution: insert free(s) behind putenv(s).

And here is my recommendation for this function:

static void addEnv(const char *name_before_underline,
            const char *name_after_underline, const char *value)
{
  char *s = NULL;
  asprintf(&s, "%s%s%s=%s", name_before_underline, *name_after_underline
? "_" : "",
                    name_after_underline, value ? value : "" );
  if(s) {
    putenv(s);
    free(s);
  }
}

************

2.) interface of addEnv()

addEnv() takes 2 parameters for bulding the variable name.
I have analysed all callings and there is no need (including addEnvPort())
to have 2 of them. And this makes the function and all callings smaller
again:

static void addEnv(const char *name, const char *value)
{
  char *s = NULL;
  asprintf(&s, "%s=%s", name, value ? value : "" );
  if(s) {
    putenv(s);
    free(s);
  }
}

If we use this we need also some modification of addEnvPort()

static void addEnvPort(const char *name)
{
      char buf[16];

      sprintf(buf, "%u", config->port);
      addEnv(name, buf);
}

and the calls must change from
      addEnvPort("REMOTE");
to
      addEnvPort("REMOTE_PORT");

************

3.) usage of addEnv()
In some source positions I found callings like
      addEnv("SERVER",         "PROTOCOL", "HTTP/1.0");
with 3 string constants.
I think that we can save memory and cpu time if we change such lines into
    putenv("SERVER_PROTOCOL=HTTP/1.0");

************

4.)
In my opinion it does not make sense:
      addEnv("PATH",           "",         getenv("PATH"));



Dirk




More information about the busybox mailing list