From 15c6e5cae60a541f109e36efec33f8948b597591 Mon Sep 17 00:00:00 2001 From: "Andrew V. Samoilov" Date: Thu, 2 Sep 2004 00:09:02 +0000 Subject: [PATCH] * direntry.c (vfs_s_readlink): Use g_strlcpy instead strncpy. * ftpfs.c (ftpfs_get_reply): Use g_strlcpy instead strncpy. * extfs.c (extfs_readlink): Fix return value. Use g_strlcpy instead strncpy. * mcfs.c (mcfs_readlink): Fix return value. Use g_strlcpy instead strncpy. * tar.c (tar_read_header): Add additional check for consistency. Fix memory leak. * cpio.c (cpio_defer_find): Remove recursion. (cpio_free_archive): Fix memory leak. (cpio_read_bin_head): Use union for simplicity using of header structure. Add additional check for consistency. (cpio_read_oldc_head): Add additional check for consistency. --- vfs/ChangeLog | 18 +++++++++++++ vfs/cpio.c | 72 +++++++++++++++++++++++++++++++++++--------------- vfs/direntry.c | 3 +-- vfs/extfs.c | 10 +++---- vfs/ftpfs.c | 12 +++------ vfs/mcfs.c | 10 +++---- vfs/tar.c | 21 +++++++++++---- 7 files changed, 99 insertions(+), 47 deletions(-) diff --git a/vfs/ChangeLog b/vfs/ChangeLog index 89a61fae8..5de7b3792 100644 --- a/vfs/ChangeLog +++ b/vfs/ChangeLog @@ -1,3 +1,21 @@ +2004-09-02 Pavel S. Shirshov + + * direntry.c (vfs_s_readlink): Use g_strlcpy instead strncpy. + * ftpfs.c (ftpfs_get_reply): Use g_strlcpy instead strncpy. + * extfs.c (extfs_readlink): Fix return value. Use g_strlcpy + instead strncpy. + * mcfs.c (mcfs_readlink): Fix return value. Use g_strlcpy + instead strncpy. + * tar.c (tar_read_header): Add additional check for consistency. + Fix memory leak. + * cpio.c (cpio_defer_find): Remove recursion. + (cpio_free_archive): Fix memory leak. + (cpio_read_bin_head): Use union for simplicity using of header + structure. Add additional check for consistency. + (cpio_read_oldc_head): Add additional check for consistency. + + Based on patches from Jakub Jelinek + 2004-09-01 Pavel S. Shirshov * fish.c (fish_get_reply): Use g_strlcpy instead diff --git a/vfs/cpio.c b/vfs/cpio.c index 6537e2e2c..785e3a526 100644 --- a/vfs/cpio.c +++ b/vfs/cpio.c @@ -102,10 +102,9 @@ static int cpio_read(void *fh, char *buffer, int count); static struct defer_inode * cpio_defer_find (struct defer_inode *l, struct defer_inode *i) { - if (!l) - return NULL; - return l->inumber == i->inumber - && l->device == i->device ? l : cpio_defer_find (l->next, i); + while (l && (l->inumber != i->inumber || l->device != i->device)) + l = l->next; + return l; } static int cpio_skip_padding(struct vfs_s_super *super) @@ -127,8 +126,15 @@ static int cpio_skip_padding(struct vfs_s_super *super) static void cpio_free_archive(struct vfs_class *me, struct vfs_s_super *super) { + struct defer_inode *l, *lnext; if(super->u.arch.fd != -1) mc_close(super->u.arch.fd); + super->u.arch.fd = -1; + for (l = super->u.arch.deferred; l; l = lnext) { + lnext = l->next; + g_free (l); + } + super->u.arch.deferred = NULL; } static int @@ -247,26 +253,35 @@ static int cpio_find_head(struct vfs_class *me, struct vfs_s_super *super) #define HEAD_LENGTH (26) static int cpio_read_bin_head(struct vfs_class *me, struct vfs_s_super *super) { - struct old_cpio_header buf; + union { + struct old_cpio_header buf; + short shorts[HEAD_LENGTH >> 1]; + } u; int len; char *name; struct stat st; - if((len = mc_read(super->u.arch.fd, (char *)&buf, HEAD_LENGTH)) < HEAD_LENGTH) + if((len = mc_read(super->u.arch.fd, (char *)&u.buf, HEAD_LENGTH)) < HEAD_LENGTH) return STATUS_EOF; CPIO_POS(super) += len; if(super->u.arch.type == CPIO_BINRE) { int i; for(i = 0; i < (HEAD_LENGTH >> 1); i++) - ((short *)&buf)[i] = GUINT16_SWAP_LE_BE(((short *)&buf)[i]); + u.shorts[i] = GUINT16_SWAP_LE_BE(u.shorts[i]); } - g_assert(buf.c_magic == 070707); + g_assert(u.buf.c_magic == 070707); - name = g_malloc(buf.c_namesize); - if((len = mc_read(super->u.arch.fd, name, buf.c_namesize)) < buf.c_namesize) { + if (u.buf.c_namesize == 0 || u.buf.c_namesize > MC_MAXPATHLEN) { + message (1, MSG_ERROR, _("Corrupted cpio header encountered in\n%s"), + super->name); + return STATUS_FAIL; + } + name = g_malloc(u.buf.c_namesize); + if((len = mc_read(super->u.arch.fd, name, u.buf.c_namesize)) < u.buf.c_namesize) { g_free(name); return STATUS_EOF; } + name[u.buf.c_namesize - 1] = '\0'; CPIO_POS(super) += len; cpio_skip_padding(super); @@ -275,15 +290,15 @@ static int cpio_read_bin_head(struct vfs_class *me, struct vfs_s_super *super) return STATUS_TRAIL; } - st.st_dev = buf.c_dev; - st.st_ino = buf.c_ino; - st.st_mode = buf.c_mode; - st.st_nlink = buf.c_nlink; - st.st_uid = buf.c_uid; - st.st_gid = buf.c_gid; - st.st_rdev = buf.c_rdev; - st.st_size = (buf.c_filesizes[0] << 16) | buf.c_filesizes[1]; - st.st_atime = st.st_mtime = st.st_ctime = (buf.c_mtimes[0] << 16) | buf.c_mtimes[1]; + st.st_dev = u.buf.c_dev; + st.st_ino = u.buf.c_ino; + st.st_mode = u.buf.c_mode; + st.st_nlink = u.buf.c_nlink; + st.st_uid = u.buf.c_uid; + st.st_gid = u.buf.c_gid; + st.st_rdev = u.buf.c_rdev; + st.st_size = (u.buf.c_filesizes[0] << 16) | u.buf.c_filesizes[1]; + st.st_atime = st.st_mtime = st.st_ctime = (u.buf.c_mtimes[0] << 16) | u.buf.c_mtimes[1]; return cpio_create_entry(me, super, &st, name); } @@ -307,16 +322,23 @@ static int cpio_read_oldc_head(struct vfs_class *me, struct vfs_s_super *super) &hd.c_dev, &hd.c_ino, &hd.c_mode, &hd.c_uid, &hd.c_gid, &hd.c_nlink, &hd.c_rdev, &hd.c_mtime, &hd.c_namesize, &hd.c_filesize) < 10) { - message (1, MSG_ERROR, _("Corrupted cpio header encountered in\n%s"), super->name); + message (1, MSG_ERROR, _("Corrupted cpio header encountered in\n%s"), + super->name); return STATUS_FAIL; } + if (hd.c_namesize == 0 || hd.c_namesize > MC_MAXPATHLEN) { + message (1, MSG_ERROR, _("Corrupted cpio header encountered in\n%s"), + super->name); + return STATUS_FAIL; + } name = g_malloc(hd.c_namesize); if((len = mc_read(super->u.arch.fd, name, hd.c_namesize)) == -1 || (unsigned long) len < hd.c_namesize) { g_free (name); return STATUS_EOF; } + name[hd.c_namesize - 1] = '\0'; CPIO_POS(super) += len; cpio_skip_padding(super); @@ -367,12 +389,19 @@ static int cpio_read_crc_head(struct vfs_class *me, struct vfs_s_super *super) (super->u.arch.type == CPIO_CRC && hd.c_magic != 070702)) return STATUS_FAIL; + if (hd.c_namesize == 0 || hd.c_namesize > MC_MAXPATHLEN) { + message (1, MSG_ERROR, _("Corrupted cpio header encountered in\n%s"), + super->name); + return STATUS_FAIL; + } + name = g_malloc(hd.c_namesize); if((len = mc_read(super->u.arch.fd, name, hd.c_namesize)) != -1 && (unsigned long) len < hd.c_namesize) { g_free (name); return STATUS_EOF; } + name[hd.c_namesize - 1] = '\0'; CPIO_POS(super) += len; cpio_skip_padding(super); @@ -435,7 +464,8 @@ cpio_create_entry (struct vfs_class *me, struct vfs_s_super *super, ("Inconsistent hardlinks of\n%s\nin cpio archive\n%s"), name, super->name); inode = NULL; - } + } else if (!inode->st.st_size) + inode->st.st_size = st->st_size; } } diff --git a/vfs/direntry.c b/vfs/direntry.c index aab8e0e18..00549cbf8 100644 --- a/vfs/direntry.c +++ b/vfs/direntry.c @@ -686,8 +686,7 @@ vfs_s_readlink (struct vfs_class *me, const char *path, char *buf, int size) if (ino->linkname == NULL) ERRNOR (EFAULT, -1); - strncpy (buf, ino->linkname, size); - *(buf+size-1) = 0; + g_strlcpy (buf, ino->linkname, size); return strlen (buf); } diff --git a/vfs/extfs.c b/vfs/extfs.c index 98924e2e1..29d324979 100644 --- a/vfs/extfs.c +++ b/vfs/extfs.c @@ -978,7 +978,7 @@ extfs_readlink (struct vfs_class *me, const char *path, char *buf, int size) { struct archive *archive; char *q; - int i; + size_t len; struct entry *entry; char *mpath = g_strdup(path); int result = -1; @@ -992,11 +992,9 @@ extfs_readlink (struct vfs_class *me, const char *path, char *buf, int size) me->verrno = EINVAL; goto cleanup; } - if (size < (i = strlen (entry->inode->linkname))) { - i = size; - } - strncpy (buf, entry->inode->linkname, i); - result = i; + len = strlen (entry->inode->linkname); + result = len > (size - 1) ? size - 1 : len; + g_strlcpy (buf, entry->inode->linkname, result + 1); cleanup: g_free (mpath); return result; diff --git a/vfs/ftpfs.c b/vfs/ftpfs.c index a84905b47..a255c7aa5 100644 --- a/vfs/ftpfs.c +++ b/vfs/ftpfs.c @@ -262,10 +262,8 @@ ftpfs_get_reply (struct vfs_class *me, int sock, char *string_buf, int string_le } switch (sscanf(answer, "%d", &code)){ case 0: - if (string_buf) { - strncpy (string_buf, answer, string_len - 1); - *(string_buf + string_len - 1) = 0; - } + if (string_buf) + g_strlcpy (string_buf, answer, string_len); code = 500; return 5; case 1: @@ -282,10 +280,8 @@ ftpfs_get_reply (struct vfs_class *me, int sock, char *string_buf, int string_le break; } } - if (string_buf){ - strncpy (string_buf, answer, string_len - 1); - *(string_buf + string_len - 1) = 0; - } + if (string_buf) + g_strlcpy (string_buf, answer, string_len); return code / 100; } } diff --git a/vfs/mcfs.c b/vfs/mcfs.c index 9805fcb23..acd53ade7 100644 --- a/vfs/mcfs.c +++ b/vfs/mcfs.c @@ -966,7 +966,8 @@ static int mcfs_readlink (struct vfs_class *me, const char *path, char *buf, int size) { char *remote_file, *stat_str; - int status, error; + int error; + size_t len; mcfs_connection *mc; if (!(remote_file = mcfs_get_path (&mc, path))) @@ -984,10 +985,9 @@ mcfs_readlink (struct vfs_class *me, const char *path, char *buf, int size) if (!rpc_get (mc->sock, RPC_STRING, &stat_str, RPC_END)) return mcfs_set_error (-1, EIO); - status = strlen (stat_str); - if (status < size) - size = status; - strncpy (buf, stat_str, size); + len = strlen (stat_str); + size = len > (size - 1) ? size - 1 : len; + g_strlcpy (buf, stat_str, size + 1); g_free (stat_str); return size; } diff --git a/vfs/tar.c b/vfs/tar.c index 6805b0d3f..86d91b4d6 100644 --- a/vfs/tar.c +++ b/vfs/tar.c @@ -410,16 +410,23 @@ tar_read_header (struct vfs_class *me, struct vfs_s_super *archive, char *bp, *data; int size, written; + if (h_size > MC_MAXPATHLEN) { + message (1, MSG_ERROR, _("Inconsistent tar archive")); + return STATUS_BADCHECKSUM; + } + longp = ((header->header.linkflag == LF_LONGNAME) ? &next_long_name : &next_long_link); if (*longp) g_free (*longp); - bp = *longp = g_malloc (*h_size); + bp = *longp = g_malloc (*h_size + 1); for (size = *h_size; size > 0; size -= written) { data = tar_get_next_record (archive, tard)->charptr; if (data == NULL) { + g_free (*longp); + *longp = NULL; message (1, MSG_ERROR, _("Unexpected EOF on archive file")); return STATUS_BADCHECKSUM; @@ -431,10 +438,14 @@ tar_read_header (struct vfs_class *me, struct vfs_s_super *archive, memcpy (bp, data, written); bp += written; } -#if 0 - if (*h_size > 1) - bp[*h_size - 1] = 0; /* just to make sure */ -#endif + + if (bp - *longp == MC_MAXPATHLEN && bp[-1] != '\0') { + g_free (*longp); + *longp = NULL; + message (1, MSG_ERROR, _("Inconsistent tar archive")); + return STATUS_BADCHECKSUM; + } + *bp = 0; goto recurse; } else { struct stat st;