[SCM] Samba Shared Repository - branch v3-2-test updated - release-3-2-0pre2-2199-gcc23f91

[Alexander Bokovoy]

Jeremy,

Jeremy Allison wrote:
> @@ -67,6 +68,13 @@
>  
>  #define CONST_DISCARD(type, ptr)      ((type) ((void *) (ptr)))
>  
> +#ifndef SAFE_FREE
> +#define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x=NULL;} } while(0)
> +#endif
> +
> +#define MOUNT_PASSWD_SIZE 64
> +#define DOMAIN_SIZE 64
> +
>  const char *thisprogram;
>  int verboseflag = 0;
>  static int got_password = 0;
> @@ -152,10 +160,7 @@ static void mount_cifs_usage(void)
>  	printf("\nTo display the version number of the mount helper:");
>  	printf("\n\t%s -V\n",thisprogram);
>  
> -	if(mountpassword) {
> -		memset(mountpassword,0,64);
> -		free(mountpassword);
> -	}
> +	SAFE_FREE(mountpassword);
Here we have non-equal behavior. Previously, the mountpassword content
was zeroed before freeing it due to security reasons. As
mount_cifs_usage() could be called multiple times (its call is in the
getopt_long()'s loop) and, particulary, after password has been filled
in, mountpassword's memory could still keep the password. Thus, memset()
is still needed.

> @@ -289,10 +285,7 @@ static int open_cred_file(char * file_name)
>  
>  	}
>  	fclose(fs);
> -	if(line_buf) {
> -		memset(line_buf,0,4096);
> -		free(line_buf);
> -	}
> +	SAFE_FREE(line_buf);
Same here.

>  	return 0;
>  }
>  
> @@ -303,9 +296,9 @@ static int get_password_from_file(int file_descript, char * filename)
>  	char c;
>  
>  	if(mountpassword == NULL)
> -		mountpassword = (char *)calloc(65,1);
> +		mountpassword = (char *)calloc(MOUNT_PASSWD_SIZE+1,1);
There is still one place where (char *)calloc(65,1); is left after this
commit: line 1197 (see below).


> @@ -1116,9 +1109,6 @@ int main(int argc, char ** argv)
>  			MOUNT_CIFS_VERSION_MAJOR,
>  			MOUNT_CIFS_VERSION_MINOR,
>  			MOUNT_CIFS_VENDOR_SUFFIX);
> -			if(mountpassword) {
> -				memset(mountpassword,0,64);
> -			}
Again, mountpassword could be already filled in after some loops with
the getopt_long().

> @@ -1206,7 +1196,7 @@ int main(int argc, char ** argv)
>  		if(mountpassword == NULL)
>  			mountpassword = (char *)calloc(65,1);
>  		if(mountpassword) {
> -			strncpy(mountpassword,getenv("PASSWD"),64);
> +			strlcpy(mountpassword,getenv("PASSWD"),MOUNT_PASSWD_SIZE);
>  			got_password = 1;
>  		}
>  	} else if (getenv("PASSWD_FD")) {
Here it is: calloc(65,1) isn't replaced by calloc(MOUNT_PASSWD_SIZE+1,1).


-- 
/ Alexander Bokovoy
Samba Team                      http://www.samba.org/
ALT Linux Team                  http://www.altlinux.org/
Midgard Project Ry              http://www.midgard-project.org/