Rewrite environment-cleaning code to not directly modify environ or its contents.

The old code didn't work correctly with GNO's environ implementation and could lead to crashes.

Makefile.common

@@ -21,6 +21,7 @@ TELNETD_SRCS = \
   libtelnet/posix_openpt.c \
   libtelnet/vasprintf.c \
   libtelnet/vfork.and.run.c \
+  libtelnet/cleanenv.c \
   telnetd/global.c \
   telnetd/slc.c \
   telnetd/state.c \

libtelnet/cleanenv.c

@@ -0,0 +1,117 @@
+/*-
+ * Copyright (c) 2016 Stephen Heumann
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+extern char **environ;
+
+/*
+ * UNSETENV_REORDERS_ENVIRON must be set to 1 if unsetenv() may
+ * change entries in environ _prior to_ the one being removed.
+ * The implementation in GNO 2.0.6's libc doesn't do this,
+ * which allows us to optimize things a bit.
+ */
+#ifdef __GNO__
+# define UNSETENV_REORDERS_ENVIRON 0
+#else
+# define UNSETENV_REORDERS_ENVIRON 1
+#endif
+
+/*
+ * Remove all environment entries except those indicated by retain,
+ * which is a null-terminated list of environment variable names.
+ */
+int cleanenv(const char *const retain[])
+{
+	int index = 0;
+	const char *entry;
+	const char *const *keep;
+	while ((entry = environ[index]) != NULL) {
+		char *equals = strchr(entry, '=');
+		char *name;
+		
+		if (equals) {
+			int match = 0;
+			size_t len = equals - entry + 1;
+			name = malloc(len);
+			if (name == NULL)
+				return -1;
+			strncpy(name, entry, len-1);
+			name[len-1] = 0;
+			for (keep = retain; *keep != NULL; keep++) {
+				if (strcmp(*keep, name) == 0) {
+					match = 1;
+					break;
+				}
+			}
+			if (match) {
+				index++;
+				free(name);
+				continue;
+			}
+		} else {
+			name = strdup(entry);
+			if (name == NULL)
+				return -1;
+		}
+		
+		unsetenv(name);
+		free(name);
+#if UNSETENV_REORDERS_ENVIRON
+		index = 0;
+#endif
+	}
+
+	return 0;
+}
+
+
+#ifdef DEBUG_CLEANENV
+
+#include <stdio.h>
+
+void dump_env(const char *s)
+{
+	int index;
+	const char *entry;
+	
+	puts(s);
+	for (index = 0; (entry = environ[index]) != NULL; index++) {
+		puts(entry);
+	}
+}
+
+int main(void)
+{
+	const char *retain[] = {"TERM", "USER", "PATH", "FOO", NULL};
+	
+	dump_env("Before:");
+	cleanenv(retain);
+	dump_env("\nAfter:");
+}
+
+#endif

libtelnet/cleanenv.h

@@ -0,0 +1 @@
+int cleanenv(const char *const retain[]);

telnetd/sys_term.c

@@ -52,6 +52,7 @@ static const char sccsid[] = "@(#)sys_term.c	8.4+1 (Berkeley) 5/30/95";
 #include "telnetd.h"
 #include "pathnames.h"
 #include "libtelnet/vfork.and.run.h"
+#include "libtelnet/cleanenv.h"
 
 #ifdef	AUTHENTICATION
 #include "libtelnet/auth.h"
@@ -60,9 +61,6 @@ static const char sccsid[] = "@(#)sys_term.c	8.4+1 (Berkeley) 5/30/95";
 int cleanopen(char *);
 void scrub_env(void);
 
-char	*envinit[3];
-extern char **environ;
-
 #define SCPYN(a, b)	(void) strncpy(a, b, sizeof(a))
 #define SCMPN(a, b)	strncmp(a, b, sizeof(a))
 
@@ -996,13 +994,10 @@ startslave(char *host, int autologin, char *autoname)
 void
 init_env(void)
 {
-	char **envp;
+	static const char *retain[] = {"TZ", NULL};
 
-	envp = envinit;
+	if (cleanenv(retain) != 0)
-	if ((*envp = getenv("TZ")))
+		fatalperror(net, "Couldn't initialize environment");
-		*envp++ -= 3;
-	*envp = 0;
-	environ = envinit;
 }
 
 
@@ -1232,58 +1227,19 @@ addarg(char **argv, const char *val)
 void
 scrub_env(void)
 {
-	static const char *rej[] = {
-		"TERMCAP=/",
-		NULL
-	};
-
 	static const char *acc[] = {
-		"XAUTH=", "XAUTHORITY=", "DISPLAY=",
+		"XAUTH", "XAUTHORITY", "DISPLAY",
-		"TERM=",
+		"TERM",
-		"EDITOR=",
+		"EDITOR",
-		"PAGER=",
+		"PAGER",
-		"LOGNAME=",
+		"LOGNAME",
-		"POSIXLY_CORRECT=",
+		"POSIXLY_CORRECT",
-		"PRINTER=",
+		"PRINTER",
 		NULL
 	};
 
-	char **cpp, **cpp2;
+	if (cleanenv(acc) != 0)
-	const char **p;
+		fatalperror(net, "Couldn't initialize environment");
-	char ** new_environ;
-	size_t count;
-
-	/* Allocate space for scrubbed environment. */
-	for (count = 1, cpp = environ; *cpp; count++, cpp++)
-		continue;
-	if ((new_environ = malloc(count * sizeof(char *))) == NULL) {
-		environ = NULL;
-		return;
-	}
-
- 	for (cpp2 = new_environ, cpp = environ; *cpp; cpp++) {
-		int reject_it = 0;
-
-		for(p = rej; *p; p++)
-			if(strncmp(*cpp, *p, strlen(*p)) == 0) {
-				reject_it = 1;
-				break;
-			}
-		if (reject_it)
-			continue;
-
-		for(p = acc; *p; p++)
-			if(strncmp(*cpp, *p, strlen(*p)) == 0)
-				break;
-		if(*p != NULL) {
-			if ((*cpp2++ = strdup(*cpp)) == NULL) {
-				environ = new_environ;
-				return;
-			}
-		}
- 	}
-	*cpp2 = NULL;
-	environ = new_environ;
 }
 
 

telnetd/telnetd.h

@@ -45,5 +45,4 @@
 #endif
 
 /* other external variables */
-extern	char **environ;
 extern	const char *altlogin;