bin/83358: [ PATCH ] improper handling of malloc failures within rexec()

[Dan Lukes]

>Number:         83358
>Category:       bin
>Synopsis:       [ PATCH ] improper handling of malloc failures within rexec()
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Jul 12 22:30:08 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dan Lukes
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
Obludarium
>Environment:
System: FreeBSD 5.4-STABLE #8: Sat Jul 9 16:31:08 CEST 2005 i386
lib/libcompat/4.3/rexec.c,v 1.6 2000/08/04 11:15:48 kris

>Description:
	1. malloc() not tested for failures within ruserpass()
	2. port variable within rexec() can be uset uninitialized causing
possible close of random socket (within bad: part of code)

>How-To-Repeat:
>Fix:

--- patch begins here ---
--- lib/libcompat/4.3/rexec.c.ORIG	Fri Aug  4 13:15:48 2000
+++ lib/libcompat/4.3/rexec.c	Wed Jul 13 00:09:08 2005
@@ -34,7 +34,7 @@
  */
 
 #if defined(LIBC_SCCS) && !defined(lint)
-static char sccsid[] = "@(#)rexec.c	8.1 (Berkeley) 6/4/93";
+static volatile char sccsid[] = "@(#)rexec.c	8.1 (Berkeley) 6/4/93";
 #endif /* LIBC_SCCS and not lint */
 
 #include <sys/types.h>
@@ -143,6 +143,7 @@
 	char *hdir, buf[BUFSIZ], *tmp;
 	char myname[MAXHOSTNAMELEN], *mydomain;
 	int t, i, c, usedefault = 0;
+	int myaname=0, myapass=0, myaacct=0;
 	struct stat stb;
 
 	hdir = getenv("HOME");
@@ -190,26 +191,33 @@
 		while ((t = token()) && t != MACH && t != DEFAULT) switch(t) {
 
 		case LOGIN:
-			if (token())
-				if (*aname == 0) {
-					*aname = malloc((unsigned) strlen(tokval) + 1);
-					(void) strcpy(*aname, tokval);
+			if (token()) {
+				if (*aname == NULL) {
+					if ((*aname = strdup(tokval)) == NULL) {
+	warnx("Error: Cannot allocate memory for user name");
+						goto bad;
+					}
+					myaname=1;
 				} else {
 					if (strcmp(*aname, tokval))
 						goto next;
 				}
+			}
 			break;
 		case PASSWD:
-			if ((*aname == 0 || strcmp(*aname, "anonymous")) &&
+			if ((*aname == NULL || strcmp(*aname, "anonymous")) &&
 			    fstat(fileno(cfile), &stb) >= 0 &&
 			    (stb.st_mode & 077) != 0) {
 	warnx("Error: .netrc file is readable by others.");
 	warnx("Remove password or make file unreadable by others.");
 				goto bad;
 			}
-			if (token() && *apass == 0) {
-				*apass = malloc((unsigned) strlen(tokval) + 1);
-				(void) strcpy(*apass, tokval);
+			if (token() && *apass == NULL) {
+				if ((*apass = strdup(tokval)) == NULL) {
+	warnx("Error: Cannot allocate memory for password");
+					goto bad;
+				}
+				myapass=1;
 			}
 			break;
 		case ACCOUNT:
@@ -219,9 +227,12 @@
 	warnx("Remove account or make file unreadable by others.");
 				goto bad;
 			}
-			if (token() && *aacct == 0) {
-				*aacct = malloc((unsigned) strlen(tokval) + 1);
-				(void) strcpy(*aacct, tokval);
+			if (token() && *aacct == NULL) {
+				if ((*aacct = strdup(tokval)) == NULL) {
+	warnx("Error: Cannot allocate memory for account");
+					goto bad;
+				}
+				myaacct=1;
 			}
 			break;
 		case MACDEF:
@@ -291,6 +302,18 @@
 	(void) fclose(cfile);
 	return (0);
 bad:
+	if (myaname == 1) {
+		free(*aname);
+		*aname = NULL;
+	}
+	if (myapass == 1) {
+		free(*apass);
+		*apass = NULL;
+	}
+	if (myaacct == 1) {
+		free(*aacct);
+		*aacct = NULL;
+	}
 	(void) fclose(cfile);
 	return (-1);
 }
@@ -304,8 +327,7 @@
 {
 	struct sockaddr_in sin, sin2, from;
 	struct hostent *hp;
-	u_short port;
-	int s, timo = 1, s3;
+	int s, timo = 1, s3 = -1;
 	char c;
 
 	hp = gethostbyname(*ahost);
@@ -336,10 +358,10 @@
 	}
 	if (fd2p == 0) {
 		(void) write(s, "", 1);
-		port = 0;
 	} else {
 		char num[8];
 		int s2, sin2len;
+		u_short port;
 
 		s2 = socket(AF_INET, SOCK_STREAM, 0);
 		if (s2 < 0) {
@@ -362,7 +384,6 @@
 		  close(s2);
 		  if (s3 < 0) {
 			perror("accept");
-			port = 0;
 			goto bad;
 		  }
 		}
@@ -386,7 +407,7 @@
 	}
 	return (s);
 bad:
-	if (port)
+	if (s3 >= 0)
 		(void) close(*fd2p);
 	(void) close(s);
 	return (-1);
--- patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted: