openssh unix dev - Dec 2001 - [PATCH]: Fix environment variable size restriction in Cygwin version

Hi,

the following patch changes the Cygwin specific function copy_environment()
to not restricting the strlen of a single environment variable to 512 byte.

The PAM specific function do_pam_environment() (also in session.c) has
the same problem but I don't know if that's important for PAM since
only PAM specific environment variables are copied in that function.

The below patch fixes that problem only for Cygwin for now.

Thanks,
Corinna

Index: session.c
==================================================================RCS file:
/cvs/openssh_cvs/session.c,v
retrieving revision 1.158
diff -u -p -r1.158 session.c
--- session.c	7 Dec 2001 17:26:49 -0000	1.158
+++ session.c	18 Dec 2001 19:07:14 -0000
@@ -918,25 +918,29 @@ void do_pam_environment(char ***env, u_i
 #ifdef HAVE_CYGWIN
 void copy_environment(char ***env, u_int *envsize)
 {
-	char *equals, var_name[512], var_val[512];
+	char *var_name = NULL, *var_val;
+	size_t size = 0, i_size;
 	int i;
 
 	for(i = 0; environ[i] != NULL; i++) {
-		if ((equals = strstr(environ[i], "=")) == NULL)
+		if ((i_size = strlen(environ[i]) + 1) < 3)
 			continue;
 
-		if (strlen(environ[i]) < (sizeof(var_name) - 1)) {
-			memset(var_name, '\0', sizeof(var_name));
-			memset(var_val, '\0', sizeof(var_val));
-
-			strncpy(var_name, environ[i], equals - environ[i]);
-			strcpy(var_val, equals + 1);
+		if (i_size > size) {
+			var_name = xrealloc(var_name, i_size);
+		    	size = i_size;
+		}
+		strcpy(var_name, environ[i]);
+		if ((var_val = strchr(var_name, '=')) != NULL) {
+			*var_val++ = '\0';
 
 			debug3("Copy environment: %s=%s", var_name, var_val);
 
 			child_set_env(env, envsize, var_name, var_val);
 		}
 	}
+	if (var_name)
+		xfree(var_name);
 }
 #endif
 

-- 
Corinna Vinschen
Cygwin Developer
Red Hat, Inc.
mailto:vinschen at redhat.com

On Tue, 18 Dec 2001, Corinna Vinschen wrote:
> Hi,
> 
> the following patch changes the Cygwin specific function
> tcopy_environment() o not restricting the strlen of a single
> tenvironment variable to 512 byte.
This patch is simpler. Comments?

Index: session.c
==================================================================RCS file:
/var/cvs/openssh/session.c,v
retrieving revision 1.162
diff -u -r1.162 session.c
--- session.c	2001/12/21 03:58:36	1.162
+++ session.c	2001/12/29 02:59:16
@@ -885,62 +885,28 @@
 	fclose(f);
 }
 
-#ifdef USE_PAM
-/*
- * Sets any environment variables which have been specified by PAM
- */
-void do_pam_environment(char ***env, u_int *envsize)
+void copy_environment(char **source, char ***env, u_int *envsize)
 {
-	char *equals, var_name[512], var_val[512];
-	char **pam_env;
+	char *var_name, *var_val;
 	int i;
 
-	if ((pam_env = fetch_pam_environment()) == NULL)
+	if (source == NULL)
 		return;
 
-	for(i = 0; pam_env[i] != NULL; i++) {
-		if ((equals = strstr(pam_env[i], "=")) == NULL)
+	for(i = 0; source[i] != NULL; i++) {
+		var_name = xstrdup(source[i]);
+		if ((var_val = strstr(var_name, "=")) == NULL) {
+			xfree(var_name);
 			continue;
-
-		if (strlen(pam_env[i]) < (sizeof(var_name) - 1)) {
-			memset(var_name, '\0', sizeof(var_name));
-			memset(var_val, '\0', sizeof(var_val));
-
-			strncpy(var_name, pam_env[i], equals - pam_env[i]);
-			strcpy(var_val, equals + 1);
-
-			debug3("PAM environment: %s=%s", var_name, var_val);
-
-			child_set_env(env, envsize, var_name, var_val);
 		}
-	}
-}
-#endif /* USE_PAM */
-
-#ifdef HAVE_CYGWIN
-void copy_environment(char ***env, u_int *envsize)
-{
-	char *equals, var_name[512], var_val[512];
-	int i;
-
-	for(i = 0; environ[i] != NULL; i++) {
-		if ((equals = strstr(environ[i], "=")) == NULL)
-			continue;
+		*var_val++ = '\0';
 
-		if (strlen(environ[i]) < (sizeof(var_name) - 1)) {
-			memset(var_name, '\0', sizeof(var_name));
-			memset(var_val, '\0', sizeof(var_val));
-
-			strncpy(var_name, environ[i], equals - environ[i]);
-			strcpy(var_val, equals + 1);
-
-			debug3("Copy environment: %s=%s", var_name, var_val);
-
-			child_set_env(env, envsize, var_name, var_val);
-		}
+		debug3("Copy environment: %s=%s", var_name, var_val);
+		child_set_env(env, envsize, var_name, var_val);
+		
+		xfree(var_name);
 	}
 }
-#endif
 
 #if defined(HAVE_GETUSERATTR)
 /*
@@ -1215,7 +1181,7 @@
 	 * The Windows environment contains some setting which are
 	 * important for a running system. They must not be dropped.
 	 */
-	copy_environment(&env, &envsize);
+	copy_environment(environ, &env, &envsize);
 #endif
 
 	if (!options.use_login) {
@@ -1299,7 +1265,7 @@
 #endif
 #ifdef USE_PAM
 	/* Pull in any environment variables that may have been set by PAM. */
-	do_pam_environment(&env, &envsize);
+	copy_environment(fetch_pam_environment(), &env, &envsize);
 #endif /* USE_PAM */
 
 	if (auth_get_socket_name() != NULL)