Improving name-truncation detection

Wayne Davison wayned at samba.org
Tue Jan 20 18:27:15 GMT 2004


On Mon, Jan 19, 2004 at 02:39:56PM -0800, jw schultz wrote:
> If we are going to vet the path name for overflow (a good idea) lets
> do it once, explicitly, as we receive it instead of having tests
> scattered throughout the code.

Fortunately the receive_file_entry() call was already checking this.
The sender code could have possibly created an entry that would overflow
once basedir was prefixed, so I've added code to make_file() that makes
sure that we don't add an entry to the list where that would happen.
Then we can simplify f_name_to() to not take a size limit (since all
flist entries will not overflow MAXPATHLEN).

..wayne..
-------------- next part --------------
Index: flist.c
--- flist.c	20 Jan 2004 17:46:30 -0000	1.167
+++ flist.c	20 Jan 2004 18:24:45 -0000
@@ -315,6 +315,7 @@ static mode_t from_wire_mode(int mode)
 static void send_directory(int f, struct file_list *flist, char *dir);
 
 static char *flist_dir;
+static int flist_dir_len;
 
 
 /**
@@ -381,7 +382,7 @@ void send_file_entry(struct file_struct 
 
 	io_write_phase = "send_file_entry";
 
-	fname = f_name_to(file, fbuf, sizeof fbuf);
+	fname = f_name_to(file, fbuf);
 
 	flags = base_flags;
 
@@ -522,7 +523,6 @@ void send_file_entry(struct file_struct 
 	}
 
 	strlcpy(lastname, fname, MAXPATHLEN);
-	lastname[MAXPATHLEN - 1] = 0;
 
 	io_write_phase = "unknown";
 }
@@ -750,8 +750,11 @@ struct file_struct *make_file(char *fnam
 	char cleaned_name[MAXPATHLEN];
 	char linkbuf[MAXPATHLEN];
 
-	strlcpy(cleaned_name, fname, MAXPATHLEN);
-	cleaned_name[MAXPATHLEN - 1] = 0;
+	if (strlcpy(cleaned_name, fname, sizeof cleaned_name)
+	    >= sizeof cleaned_name - flist_dir_len) {
+		rprintf(FINFO, "skipping overly long name: %s\n", fname);
+		return NULL;
+	}
 	clean_fname(cleaned_name);
 	if (sanitize_paths)
 		sanitize_path(cleaned_name, NULL);
@@ -852,16 +855,7 @@ struct file_struct *make_file(char *fnam
 		file_checksum(fname, file->u.sum, st.st_size);
 	}
 
-	if (flist_dir) {
-		static char *lastdir;
-		if (lastdir && strcmp(lastdir, flist_dir) == 0)
-			file->basedir = lastdir;
-		else {
-			file->basedir = strdup(flist_dir);
-			lastdir = file->basedir;
-		}
-	} else
-		file->basedir = NULL;
+	file->basedir = flist_dir;
 
 	if (!S_ISDIR(st.st_mode))
 		stats.total_size += st.st_size;
@@ -900,7 +894,7 @@ void send_file_name(int f, struct file_l
 	if (S_ISDIR(file->mode) && recursive) {
 		struct exclude_struct **last_exclude_list =
 		    local_exclude_list;
-		send_directory(f, flist, f_name_to(file, fbuf, sizeof fbuf));
+		send_directory(f, flist, f_name_to(file, fbuf));
 		local_exclude_list = last_exclude_list;
 		return;
 	}
@@ -1106,6 +1100,9 @@ struct file_list *send_file_list(int f, 
 			fname = ".";
 
 		if (dir && *dir) {
+			static char *lastdir;
+			static int lastdir_len;
+
 			strcpy(olddir, curr_dir); /* can't overflow */
 
 			if (!push_dir(dir)) {
@@ -1115,7 +1112,15 @@ struct file_list *send_file_list(int f, 
 				continue;
 			}
 
-			flist_dir = dir;
+			if (lastdir && strcmp(lastdir, dir) == 0) {
+				flist_dir = lastdir;
+				flist_dir_len = lastdir_len;
+			} else {
+				if (lastdir)
+					free(lastdir);
+				flist_dir = lastdir = strdup(dir);
+				flist_dir_len = lastdir_len = strlen(dir);
+			}
 		}
 
 		if (one_file_system)
@@ -1125,6 +1130,7 @@ struct file_list *send_file_list(int f, 
 
 		if (olddir[0]) {
 			flist_dir = NULL;
+			flist_dir_len = 0;
 			if (!pop_dir(olddir)) {
 				rprintf(FERROR, "pop_dir %s failed: %s\n",
 					full_fname(dir), strerror(errno));
@@ -1510,19 +1516,21 @@ int f_name_cmp(struct file_struct *f1, s
 
 
 /* Return a copy of the full filename of a flist entry, using the indicated
- * buffer.
+ * buffer.  No size-checking is done because we checked the size when creating
+ * the file_struct entry.
  */
-char *f_name_to(struct file_struct *f, char *fbuf, int bsize)
+char *f_name_to(struct file_struct *f, char *fbuf)
 {
 	if (!f || !f->basename)
 		return NULL;
 
 	if (f->dirname) {
-		int off = strlcpy(fbuf, f->dirname, bsize);
-		off += strlcpy(fbuf + off, "/", bsize - off);
-		strlcpy(fbuf + off, f->basename, bsize - off);
+		int len = strlen(f->dirname);
+		memcpy(fbuf, f->dirname, len);
+		fbuf[len] = '/';
+		strcpy(fbuf + len + 1, f->basename);
 	} else
-		strlcpy(fbuf, f->basename, bsize);
+		strcpy(fbuf, f->basename);
 	return fbuf;
 }
 
@@ -1536,5 +1544,5 @@ char *f_name(struct file_struct *f)
 
 	n = (n + 1) % (sizeof names / sizeof names[0]);
 
-	return f_name_to(f, names[n], sizeof names[0]);
+	return f_name_to(f, names[n]);
 }
Index: generator.c
--- generator.c	20 Jan 2004 05:09:36 -0000	1.71
+++ generator.c	20 Jan 2004 17:54:55 -0000
@@ -561,8 +561,8 @@ void generate_files(int f, struct file_l
 			file = ©
 		}
 
-		recv_generator(local_name? local_name
-			     : f_name_to(file,fbuf,sizeof fbuf), file, i, f);
+		recv_generator(local_name ? local_name : f_name_to(file, fbuf),
+			       file, i, f);
 	}
 
 	phase++;
@@ -578,8 +578,8 @@ void generate_files(int f, struct file_l
 	 * to catch initial checksum errors */
 	while ((i = get_redo_num()) != -1) {
 		struct file_struct *file = flist->files[i];
-		recv_generator(local_name? local_name
-			     : f_name_to(file,fbuf,sizeof fbuf), file, i, f);
+		recv_generator(local_name ? local_name : f_name_to(file, fbuf),
+			       file, i, f);
 	}
 
 	phase++;
Index: hlink.c
--- hlink.c	12 Jan 2004 03:49:47 -0000	1.29
+++ hlink.c	20 Jan 2004 18:00:23 -0000
@@ -137,8 +137,8 @@ void do_hard_links(void)
 {
 #if SUPPORT_HARD_LINKS
 	struct file_struct *file;
-	char fbuf[MAXPATHLEN];
-	char *hlink1, *hlink2;
+	char hlink1[MAXPATHLEN];
+	char *hlink2;
 	STRUCT_STAT st1, st2;
 	int i;
 
@@ -147,8 +147,7 @@ void do_hard_links(void)
 
 	for (i = 0; i < hlink_count; i++) {
 		file = hlink_list[i];
-		hlink1 = f_name_to(file, fbuf, sizeof fbuf);
-		if (link_stat(hlink1, &st1) != 0)
+		if (link_stat(f_name_to(file, hlink1), &st1) != 0)
 			continue;
 		while ((file = file->F_NEXT) != NULL) {
 			hlink2 = f_name(file);
Index: proto.h
--- proto.h	20 Jan 2004 17:46:31 -0000	1.174
+++ proto.h	20 Jan 2004 17:53:28 -0000
@@ -85,7 +85,7 @@ void free_file(struct file_struct *file)
 struct file_list *flist_new(void);
 void flist_free(struct file_list *flist);
 int f_name_cmp(struct file_struct *f1, struct file_struct *f2);
-char *f_name_to(struct file_struct *f, char *fbuf, int bsize);
+char *f_name_to(struct file_struct *f, char *fbuf);
 char *f_name(struct file_struct *f);
 void write_sum_head(int f, struct sum_struct *sum);
 void recv_generator(char *fname, struct file_struct *file, int i, int f_out);
Index: receiver.c
--- receiver.c	20 Jan 2004 03:37:04 -0000	1.64
+++ receiver.c	20 Jan 2004 18:04:51 -0000
@@ -94,7 +94,7 @@ void delete_files(struct file_list *flis
 		if (!S_ISDIR(flist->files[j]->mode) ||
 		    !(flist->files[j]->flags & FLAG_DELETE)) continue;
 
-		name = f_name_to(flist->files[j], fbuf, sizeof fbuf);
+		name = f_name_to(flist->files[j], fbuf);
 
 		if (!(local_file_list = send_file_list(-1,1,&name)))
 			continue;
@@ -335,7 +335,7 @@ int recv_files(int f_in,struct file_list
 		if (local_name)
 			fname = local_name;
 		else
-			fname = f_name_to(file, fbuf, sizeof fbuf);
+			fname = f_name_to(file, fbuf);
 
 		if (dry_run) {
 			if (!am_server && verbose) {	/* log transfer */
@@ -486,8 +486,8 @@ int recv_files(int f_in,struct file_list
 	for (i = 0; i < flist->count; i++) {
 		file = flist->files[i];
 		if (!file->basename || !S_ISDIR(file->mode)) continue;
-		recv_generator(local_name? local_name
-			     : f_name_to(file,fbuf,sizeof fbuf), file, i, -1);
+		recv_generator(local_name ? local_name : f_name_to(file, fbuf),
+			       file, i, -1);
 	}
 
 	if (verbose > 2)
Index: sender.c
--- sender.c	20 Jan 2004 05:35:57 -0000	1.35
+++ sender.c	20 Jan 2004 17:57:53 -0000
@@ -130,7 +130,7 @@ void send_files(struct file_list *flist,
 		rprintf(FINFO, "send_files starting\n");
 
 	while (1) {
-		int offset = 0;
+		unsigned int offset;
 
 		i = read_int(f_in);
 		if (i == -1) {
@@ -167,8 +167,9 @@ void send_files(struct file_list *flist,
 					full_fname(fname));
 				return;
 			}
-		}
-		f_name_to(file, fname + offset, MAXPATHLEN - offset);
+		} else
+			offset = 0;
+		f_name_to(file, fname + offset);
 
 		if (verbose > 2)
 			rprintf(FINFO, "send_files(%d, %s)\n", i, fname);


More information about the rsync mailing list