[PATCH]: Fix environment variable size restriction in Cygwin version

[Corinna Vinschen]

On Sat, Dec 29, 2001 at 02:00:03PM +1100, Damien Miller wrote:
> 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?

It's ok with me, especially that you're using the same function
for both, PAM and Cygwin.  The difference is just that I'm trying
to avoid many single xmalloc/xfree pairs since that will probably
clutter up the memory in conjunction with the call to child_set_env()

    for all env_vars:
	xstrcpy
	    xmalloc
	xfree

which is the reason I used xrealloc.  But that's probably just a
theoretical difference so I don't care.

Thanks,
Corinna

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

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