Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 30 Jan 1998 14:03:30 -0800 (PST)
From:      Archie Cobbs <archie@whistle.com>
To:        FreeBSD-gnats-submit@FreeBSD.ORG
Subject:   bin/5604: memory leak and other bugs in setenv(3)
Message-ID:  <199801302203.OAA17264@bubba.whistle.com>

next in thread | raw e-mail | index | archive | help

>Number:         5604
>Category:       bin
>Synopsis:       setenv(3) function has memory leak, other bugs
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:
>Keywords:
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri Jan 30 14:20:01 PST 1998
>Last-Modified:
>Originator:     Archie Cobbs
>Organization:
Whistle Communications, Inc.
>Release:        FreeBSD 2.2.5-STABLE i386
>Environment:

All versions of FreeBSD.

>Description:

There is a memory leak in the setenv() function. If you overwrite
a value with a longer value, the memory allocated for the shorter
value is never freed.

Also, in certain failure cases (such as failure in the realloc()
function), the proper return value is returned (-1) but the existing
environment is destroyed.

>How-To-Repeat:

Memory leak is exhibited by this program:

  #include <stdlib.h>
  #define BSIZE 1024
  char buf[BSIZE + 1];
  int main(int ac, char *av[])
  {
    int	x;
    memset(buf, 'b', BSIZE);
    buf[BSIZE] = 0;
    for (x = 0; 1; x++)
    {
      buf[x % BSIZE] = 0;
      setenv("foo", buf, 1);
      buf[x % BSIZE] = 'b';
    }
    return(0);
  }

Also, notice what happens to "environ" in the original code when
the realloc() function call fails.

Also, the "alloced" flag is incorrectly set if the original malloc()
fails.

Overall, this function exhibits SHODDY PROGRAMMING!! :-)

>Fix:

Index: setenv.c
===================================================================
RCS file: /cvs/freebsd/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.3
diff -c -r1.3 setenv.c
*** setenv.c	1996/07/12 18:55:21	1.3
--- setenv.c	1998/01/30 21:58:33
***************
*** 54,60 ****
  {
  	extern char **environ;
  	static int alloced;			/* if allocated space before */
! 	register char *c;
  	int l_value, offset;
  
  	if (*value == '=')			/* no `=' in value */
--- 54,60 ----
  {
  	extern char **environ;
  	static int alloced;			/* if allocated space before */
! 	register char *c, *new;
  	int l_value, offset;
  
  	if (*value == '=')			/* no `=' in value */
***************
*** 73,98 ****
  
  		for (p = environ, cnt = 0; *p; ++p, ++cnt);
  		if (alloced) {			/* just increase size */
! 			environ = (char **)realloc((char *)environ,
  			    (size_t)(sizeof(char *) * (cnt + 2)));
! 			if (!environ)
  				return (-1);
! 		}
! 		else {				/* get new space */
! 			alloced = 1;		/* copy old entries into it */
! 			p = malloc((size_t)(sizeof(char *) * (cnt + 2)));
  			if (!p)
  				return (-1);
  			bcopy(environ, p, cnt * sizeof(char *));
! 			environ = p;
  		}
! 		environ[cnt + 1] = NULL;
  		offset = cnt;
  	}
  	for (c = (char *)name; *c && *c != '='; ++c);	/* no `=' in name */
! 	if (!(environ[offset] =			/* name + `=' + value */
! 	    malloc((size_t)((int)(c - name) + l_value + 2))))
  		return (-1);
  	for (c = environ[offset]; (*c = *name++) && *c != '='; ++c);
  	for (*c++ = '='; (*c++ = *value++); );
  	return (0);
--- 73,101 ----
  
  		for (p = environ, cnt = 0; *p; ++p, ++cnt);
  		if (alloced) {			/* just increase size */
! 			p = (char **)realloc((char *)environ,
  			    (size_t)(sizeof(char *) * (cnt + 2)));
! 			if (!p)
  				return (-1);
! 		} else {	/* get new space and copy entries into it */
! 			p = (char **)malloc((size_t)
! 			    (sizeof(char *) * (cnt + 2)));
  			if (!p)
  				return (-1);
  			bcopy(environ, p, cnt * sizeof(char *));
! 			alloced = 1;
  		}
! 		environ = p;
! 		environ[cnt] = NULL;	/* indicates not previosly malloc'd */
! 		environ[cnt + 1] = NULL;		/* terminates array */
  		offset = cnt;
  	}
  	for (c = (char *)name; *c && *c != '='; ++c);	/* no `=' in name */
! 	if (!(new = malloc((size_t)((int)(c - name) + l_value + 2))))
  		return (-1);
+ 	if (environ[offset])			/* free old version, if any */
+ 		free(environ[offset]);
+ 	environ[offset] = new;
  	for (c = environ[offset]; (*c = *name++) && *c != '='; ++c);
  	for (*c++ = '='; (*c++ = *value++); );
  	return (0);

>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199801302203.OAA17264>