[scponly] patches

[Dimitri Papadopoulos-Orfanos]

Hi,

Here are a few patches for scponly-4.0:

* You shouldn't need to call bzero() before strncpy() to initialize the 
destination buffer. According to the C standard strncpy() does set the 
whole destination buffer:
	char *strncpy(char * restrict s1,
		const char * restrict s2,
		size_t n);
	If the array pointed to by s2 is a string that is shorter than n
	characters, null characters are appended to the copy in the
	array pointed to by s1, until n characters in all have been
	written.

* On the other hand you should set the last element of the destination 
buffer to '\0' after a call to strncpy(). If you don't, it means you 
coudl have called strcpy() in the first place. According to the POSIX 
standard:
	If there is no null byte in the first n bytes of the array
	pointed to by s2, the result is not null-terminated.
http://www.opengroup.org/onlinepubs/009695399/functions/strncpy.html

* The above is espcially true with FILENAME_MAX because in theory file 
names may be longer than FILENAME_MAX. Indeed according to the C standard:
	FILENAME_MAX [...] expands to an integer constant expression
	that is the size needed for an array of char large enough to
	hold the longest file name string that the implementation
	guarantees can be opened.
	If the implementation imposes no practical limit on the length
	of file name strings, the value of FILENAME_MAX should instead
	be the recommended size of an array intended to hold a file name
	string. Of course, file name string contents are subject to
	other system-specific constraints; therefore all possible
	strings of length FILENAME_MAX cannot be expected to be opened
	successfully.
Basically there are absolutely no guarantees around FILENAME_MAX. So you 
should be extra careful when filling a buffer of size FILENAME_MAX.

--
Dimitri Papadopoulos
-------------- next part --------------
diff -uNr ../scponly-4.0.orig/helper.c ./helper.c
--- ../scponly-4.0.orig/helper.c	2004-11-28 01:53:21.000000000 +0100
+++ ./helper.c	2005-04-07 10:25:18.000000000 +0200
@@ -331,8 +331,8 @@
 			perror("malloc");
 			exit(EXIT_FAILURE);
 		}
-		bzero(tempbuf,(blen-slen+1));
 		strncpy(tempbuf, big, blen-slen);
+		tempbuf[blen-slen] = '\0';
 		return tempbuf;
 	}
 	return NULL;
@@ -390,7 +390,9 @@
 		syslog(LOG_DEBUG, "retrieved home directory of \"%s\" for user \"%s\"",
 		       userinfo->pw_dir, userinfo->pw_name);
 	strncpy(username,userinfo->pw_name,MAX_USERNAME);
+	username[MAX_USERNAME-1] = '\0';
 	strncpy(homedir,userinfo->pw_dir,FILENAME_MAX);
+	homedir[FILENAME_MAX-1] = '\0';
 	return 1;
 }
 
diff -uNr ../scponly-4.0.orig/scponly.c ./scponly.c
--- ../scponly-4.0.orig/scponly.c	2004-11-28 02:16:39.000000000 +0100
+++ ./scponly.c	2005-04-07 11:12:36.000000000 +0200
@@ -415,7 +415,7 @@
 #ifdef WINSCP_COMPAT
         if (strbeg(tmprequest,PROG_CD))
         {
-                char *destdir=(char *)malloc(reqlen);
+                char *destdir=(char *)malloc(reqlen-4);
                 if (destdir == NULL)
                 {
                         perror("malloc");
@@ -430,13 +430,13 @@
                  */
                 if ((tmprequest[(reqlen-1)]=='"') && (tmprequest[3]=='"'))
                 {
-                        bzero(destdir,reqlen);
                         strncpy(destdir,&tmprequest[4],reqlen-5);
+                        destdir[reqlen-5] = '\0';
                         if (debuglevel)
                                 syslog(LOG_INFO, "chdir: %s (%s)", tmprequest, logstamp());
                         retval=chdir(destdir);
                         free(destdir);
-						free(tmprequest);
+                        free(tmprequest);
                         return(retval);
                 }
 		syslog(LOG_ERR, "bogus chdir request: %s (%s)", tmprequest, logstamp());