From owner-freebsd-bugs  Fri Jan 30 14:20:08 1998
Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
Received: (from majordom@localhost)
          by hub.freebsd.org (8.8.8/8.8.8) id OAA07080
          for freebsd-bugs-outgoing; Fri, 30 Jan 1998 14:20:08 -0800 (PST)
          (envelope-from owner-freebsd-bugs@FreeBSD.ORG)
Received: (from gnats@localhost)
          by hub.freebsd.org (8.8.8/8.8.8) id OAA07012;
          Fri, 30 Jan 1998 14:20:04 -0800 (PST)
          (envelope-from gnats)
Received: from whistle.com (s205m131.whistle.com [207.76.205.131])
          by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id OAA06075
          for <FreeBSD-gnats-submit@freebsd.org>; Fri, 30 Jan 1998 14:15:31 -0800 (PST)
          (envelope-from archie@whistle.com)
Received: (from smap@localhost) by whistle.com (8.7.5/8.6.12) id OAA04327 for <FreeBSD-gnats-submit@freebsd.org>; Fri, 30 Jan 1998 14:03:39 -0800 (PST)
Received: from bubba.whistle.com(207.76.205.7) by whistle.com via smap (V1.3)
	id sma004319; Fri Jan 30 14:03:30 1998
Received: (from archie@localhost) by bubba.whistle.com (8.8.7/8.6.12) id OAA17264; Fri, 30 Jan 1998 14:03:30 -0800 (PST)
Message-Id: <199801302203.OAA17264@bubba.whistle.com>
Date: Fri, 30 Jan 1998 14:03:30 -0800 (PST)
From: Archie Cobbs <archie@whistle.com>
Reply-To: archie@whistle.com
To: FreeBSD-gnats-submit@FreeBSD.ORG
X-Send-Pr-Version: 3.2
Subject: bin/5604: memory leak and other bugs in setenv(3)
Sender: owner-freebsd-bugs@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org
X-To-Unsubscribe: mail to majordomo@FreeBSD.org "unsubscribe freebsd-bugs"


>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: