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