Source: libtar Version: 1.2.11-8 Severity: important Tags: patch User: debian-h...@lists.debian.org Usertags: hurd
Hi, This patch solves the build problems for GNU/Hurd due to MAXPATHLEN issues. The solution is to make dynamic string allocations instead of using fixed length buffers. The functions mkdirhier, basename and dirname have all been checked with separate test programs on Linux and Hurd and with valgrind on Linux without problems. Furthermore, parts of the the code has been reviewed by GN/Hurd developers and Debian GNU/Hurd developers and maintainers. Build and run tested on both Linux and Hurd without problems. Also heap and leak tested on Linux with valgrind. No heap errors, but still memory leaks (coming from the original code). Removing the memory leaks will need another deep investigation of the original code, which time does not permit right now. Since there is no upstream, the wish is that the Debian Maintainer takes a look at this patch and checks if it can be applied. Test programs are available on request. This patch will affect all architectures, completely removing the MAXPATHLEN dependencies (except for a few unused compat functions). There might be some remaining unneeded header file inclusions, they are remnants of debugging methods used, both gdb and printf. Thanks!
diff -ur libtar-1.2.11/compat/basename.c libtar-1.2.11.modified/compat/basename.c --- libtar-1.2.11/compat/basename.c 2000-10-27 18:16:34.000000000 +0200 +++ libtar-1.2.11.modified/compat/basename.c 2012-01-19 14:20:20.000000000 +0100 @@ -34,17 +34,22 @@ #include <errno.h> #include <string.h> #include <sys/param.h> +#include <stdlib.h> char * openbsd_basename(path) const char *path; { - static char bname[MAXPATHLEN]; + static char *bname = NULL; register const char *endp, *startp; + int len = 0; + + if (bname != NULL) + free(bname); /* Empty or NULL string gets treated as "." */ if (path == NULL || *path == '\0') { - (void)strcpy(bname, "."); + bname = strdup("."); return(bname); } @@ -55,7 +60,7 @@ /* All slashes becomes "/" */ if (endp == path && *endp == '/') { - (void)strcpy(bname, "/"); + bname = strdup("/"); return(bname); } @@ -64,11 +69,9 @@ while (startp > path && *(startp - 1) != '/') startp--; - if (endp - startp + 1 > sizeof(bname)) { - errno = ENAMETOOLONG; - return(NULL); - } - (void)strncpy(bname, startp, endp - startp + 1); - bname[endp - startp + 1] = '\0'; + len = endp - startp + 1; + bname = malloc(len + 1); + (void)strncpy(bname, startp, len); + bname[len] = '\0'; return(bname); } diff -ur libtar-1.2.11/compat/dirname.c libtar-1.2.11.modified/compat/dirname.c --- libtar-1.2.11/compat/dirname.c 2000-10-27 18:16:34.000000000 +0200 +++ libtar-1.2.11.modified/compat/dirname.c 2012-01-18 23:06:20.000000000 +0100 @@ -34,17 +34,30 @@ #include <errno.h> #include <string.h> #include <sys/param.h> +#include <stdio.h> +#include <stdlib.h> char * openbsd_dirname(path) const char *path; { - static char bname[MAXPATHLEN]; + static char *bname = NULL; + static unsigned allocated = 0; register const char *endp; + int len; + + if (!allocated) { + allocated = 64; + bname = malloc(allocated); + if (!bname) { + allocated = 0; + return NULL; + } + } /* Empty or NULL string gets treated as "." */ if (path == NULL || *path == '\0') { - (void)strcpy(bname, "."); + strcpy(bname, "."); return(bname); } @@ -67,11 +80,18 @@ } while (endp > path && *endp == '/'); } - if (endp - path + 1 > sizeof(bname)) { - errno = ENAMETOOLONG; - return(NULL); + len = endp - path + 1; + if (len + 1 > allocated) { + unsigned new_allocated = 2*(len+1); + void *new_bname = malloc(new_allocated); + if (!new_bname) + return NULL; + allocated = new_allocated; + free(bname); + bname = new_bname; } - (void)strncpy(bname, path, endp - path + 1); - bname[endp - path + 1] = '\0'; + + (void)strncpy(bname, path, len); + bname[len] = '\0'; return(bname); } diff -ur libtar-1.2.11/lib/append.c libtar-1.2.11.modified/lib/append.c --- libtar-1.2.11/lib/append.c 2003-01-07 02:40:59.000000000 +0100 +++ libtar-1.2.11.modified/lib/append.c 2012-01-20 14:28:57.000000000 +0100 @@ -17,6 +17,7 @@ #include <fcntl.h> #include <sys/param.h> #include <sys/types.h> +#include <sys/stat.h> #ifdef STDC_HEADERS # include <stdlib.h> @@ -38,7 +39,7 @@ struct tar_ino { ino_t ti_ino; - char ti_name[MAXPATHLEN]; + char *ti_name; }; typedef struct tar_ino tar_ino_t; @@ -61,7 +62,7 @@ libtar_hashptr_t hp; tar_dev_t *td = NULL; tar_ino_t *ti = NULL; - char path[MAXPATHLEN]; + char *path = NULL, *name = NULL; #ifdef DEBUG printf("==> tar_append_file(TAR=0x%lx (\"%s\"), realname=\"%s\", " @@ -135,25 +136,33 @@ if (ti == NULL) return -1; ti->ti_ino = s.st_ino; - snprintf(ti->ti_name, sizeof(ti->ti_name), "%s", - savename ? savename : realname); + name = savename ? savename : realname; + if ((ti->ti_name = malloc(strlen(name) + 1)) == NULL) + return -1; + snprintf(ti->ti_name, strlen(name) + 1, "%s", name); libtar_hash_add(td->td_h, ti); + if (ti->ti_name != NULL) + free(ti->ti_name); } /* check if it's a symlink */ if (TH_ISSYM(t)) { - i = readlink(realname, path, sizeof(path)); + if ((path = malloc(s.st_size + 1)) == NULL) + return -1; + i = readlink(realname, path, s.st_size); if (i == -1) + { + free(path); return -1; - if (i >= MAXPATHLEN) - i = MAXPATHLEN - 1; + } path[i] = '\0'; #ifdef DEBUG printf(" tar_append_file(): encoding symlink \"%s\" -> " "\"%s\"...\n", realname, path); #endif th_set_link(t, path); + free(path); } /* print file info */ diff -ur libtar-1.2.11/lib/decode.c libtar-1.2.11.modified/lib/decode.c --- libtar-1.2.11/lib/decode.c 2012-01-14 17:16:35.000000000 +0100 +++ libtar-1.2.11.modified/lib/decode.c 2012-01-21 13:06:29.000000000 +0100 @@ -26,19 +26,26 @@ char * th_get_pathname(TAR *t) { - static char filename[MAXPATHLEN]; + static char *filename = NULL; + int len; if (t->th_buf.gnu_longname) return t->th_buf.gnu_longname; if (t->th_buf.prefix[0] != '\0') { - snprintf(filename, sizeof(filename), "%.155s/%.100s", + len = strlen(t->th_buf.prefix) + 1 + strlen(t->th_buf.name) + 1; + if ((filename = malloc(len + 1)) == NULL) + return NULL; + snprintf(filename, len + 1, "%.155s/%.100s", t->th_buf.prefix, t->th_buf.name); return filename; } - snprintf(filename, sizeof(filename), "%.100s", t->th_buf.name); + len = strlen(t->th_buf.name); + if ((filename = malloc(len + 1)) == NULL) + return NULL; + snprintf(filename, len + 1, "%.100s", t->th_buf.name); return filename; } diff -ur libtar-1.2.11/lib/extract.c libtar-1.2.11.modified/lib/extract.c --- libtar-1.2.11/lib/extract.c 2003-03-03 00:58:07.000000000 +0100 +++ libtar-1.2.11.modified/lib/extract.c 2012-01-20 18:16:19.000000000 +0100 @@ -18,6 +18,7 @@ #include <fcntl.h> #include <errno.h> #include <utime.h> +#include <string.h> #ifdef STDC_HEADERS # include <stdlib.h> @@ -30,8 +31,8 @@ struct linkname { - char ln_save[MAXPATHLEN]; - char ln_real[MAXPATHLEN]; + char *ln_save; + char *ln_real; }; typedef struct linkname linkname_t; @@ -97,8 +98,9 @@ int tar_extract_file(TAR *t, char *realname) { - int i; + int i, len; linkname_t *lnp; + char *pathname = NULL; if (t->options & TAR_NOOVERWRITE) { @@ -140,15 +142,32 @@ lnp = (linkname_t *)calloc(1, sizeof(linkname_t)); if (lnp == NULL) return -1; - strlcpy(lnp->ln_save, th_get_pathname(t), sizeof(lnp->ln_save)); - strlcpy(lnp->ln_real, realname, sizeof(lnp->ln_real)); + pathname = th_get_pathname(t); + len = strlen(pathname); + if ((lnp->ln_save = malloc(len + 1)) == NULL) + return -1; + strlcpy(lnp->ln_save, pathname, len + 1); + len = strlen(realname); + if ((lnp->ln_real = malloc(len + 1)) == NULL) + { + free(lnp->ln_save); + return -1; + } + strlcpy(lnp->ln_real, realname, len + 1); #ifdef DEBUG printf("tar_extract_file(): calling libtar_hash_add(): key=\"%s\", " - "value=\"%s\"\n", th_get_pathname(t), realname); + "value=\"%s\"\n", pathname, realname); #endif if (libtar_hash_add(t->h, lnp) != 0) + { + free(lnp->ln_save); + free(lnp->ln_real); return -1; + } + /* FIXME: check if they can be freed here */ + free(lnp->ln_save); + free(lnp->ln_real); return 0; } diff -ur libtar-1.2.11/lib/libtar.h libtar-1.2.11.modified/lib/libtar.h --- libtar-1.2.11/lib/libtar.h 2003-01-07 02:40:59.000000000 +0100 +++ libtar-1.2.11.modified/lib/libtar.h 2012-01-17 17:49:39.000000000 +0100 @@ -260,7 +260,7 @@ int ino_hash(ino_t *inode); /* create any necessary dirs */ -int mkdirhier(char *path); +int mkdirhier(const char *path); /* calculate header checksum */ int th_crc_calc(TAR *t); diff -ur libtar-1.2.11/lib/util.c libtar-1.2.11.modified/lib/util.c --- libtar-1.2.11/lib/util.c 2003-01-07 02:41:00.000000000 +0100 +++ libtar-1.2.11.modified/lib/util.c 2012-01-18 23:43:01.000000000 +0100 @@ -15,6 +15,7 @@ #include <stdio.h> #include <sys/param.h> #include <errno.h> +#include <stdlib.h> #ifdef STDC_HEADERS # include <string.h> @@ -25,13 +26,15 @@ int path_hashfunc(char *key, int numbuckets) { - char buf[MAXPATHLEN]; + char *buf; char *p; + int i; - strcpy(buf, key); + buf = strdup(key); p = basename(buf); - - return (((unsigned int)p[0]) % numbuckets); + i = ((unsigned int)p[0]) % numbuckets; + free(buf); + return (i); } @@ -66,7 +69,6 @@ return *inode % 256; } - /* ** mkdirhier() - create all directories in a given path ** returns: @@ -75,17 +77,28 @@ ** -1 (and sets errno) error */ int -mkdirhier(char *path) +mkdirhier(const char *path) { - char src[MAXPATHLEN], dst[MAXPATHLEN] = ""; - char *dirp, *nextp = src; - int retval = 1; + char *src, *dst = NULL; + char *dirp, *nextp = NULL; + int retval = 1, len; + + len = strlen(path); + if ((src = strdup(path)) == NULL) + { + errno = ENOMEM; + return -1; + } + nextp = src; - if (strlcpy(src, path, sizeof(src)) > sizeof(src)) + /* Make room for // with absolute paths */ + if ((dst = malloc(len + 2)) == NULL) { - errno = ENAMETOOLONG; + free(src); + errno = ENOMEM; return -1; } + dst[0] = '\0'; if (path[0] == '/') strcpy(dst, "/"); @@ -102,12 +115,18 @@ if (mkdir(dst, 0777) == -1) { if (errno != EEXIST) + { + free(src); + free(dst); return -1; + } } else retval = 0; } + free(src); + free(dst); return retval; } diff -ur libtar-1.2.11/lib/wrapper.c libtar-1.2.11.modified/lib/wrapper.c --- libtar-1.2.11/lib/wrapper.c 2003-01-07 02:41:00.000000000 +0100 +++ libtar-1.2.11.modified/lib/wrapper.c 2012-01-21 13:29:36.000000000 +0100 @@ -16,6 +16,7 @@ #include <sys/param.h> #include <dirent.h> #include <errno.h> +#include <stdlib.h> #ifdef STDC_HEADERS # include <string.h> @@ -26,8 +27,8 @@ tar_extract_glob(TAR *t, char *globname, char *prefix) { char *filename; - char buf[MAXPATHLEN]; - int i; + char *buf = NULL; + int i, len; while ((i = th_read(t)) == 0) { @@ -41,13 +42,27 @@ if (t->options & TAR_VERBOSE) th_print_long_ls(t); if (prefix != NULL) - snprintf(buf, sizeof(buf), "%s/%s", prefix, filename); + { + len = strlen(prefix) + 1 + strlen(filename); + if ((buf = malloc(len + 1)) == NULL) + return -1; + snprintf(buf, len + 1, "%s/%s", prefix, filename); + } else - strlcpy(buf, filename, sizeof(buf)); - if (tar_extract_file(t, filename) != 0) + { + len = strlen(filename); + if ((buf = malloc(len + 1)) == NULL) + return -1; + strlcpy(buf, filename, len + 1); + } + if (tar_extract_file(t, buf) != 0) + { + free(buf); return -1; + } } + free(buf); return (i == 1 ? 0 : -1); } @@ -56,8 +71,8 @@ tar_extract_all(TAR *t, char *prefix) { char *filename; - char buf[MAXPATHLEN]; - int i; + char *buf = NULL; + int i, len; #ifdef DEBUG printf("==> tar_extract_all(TAR *t, \"%s\")\n", @@ -69,21 +84,36 @@ #ifdef DEBUG puts(" tar_extract_all(): calling th_get_pathname()"); #endif + filename = th_get_pathname(t); if (t->options & TAR_VERBOSE) th_print_long_ls(t); if (prefix != NULL) - snprintf(buf, sizeof(buf), "%s/%s", prefix, filename); + { + len = strlen(prefix) + 1 + strlen(filename); + if ((buf = malloc(len + 1)) == NULL) + return -1; + snprintf(buf, len + 1, "%s/%s", prefix, filename); + } else - strlcpy(buf, filename, sizeof(buf)); + { + len = strlen(filename); + if ((buf = malloc(len + 1)) == NULL) + return -1; + strlcpy(buf, filename, len + 1); + } #ifdef DEBUG printf(" tar_extract_all(): calling tar_extract_file(t, " "\"%s\")\n", buf); #endif if (tar_extract_file(t, buf) != 0) + { + free(buf); return -1; + } } + free(buf); return (i == 1 ? 0 : -1); } @@ -91,11 +121,12 @@ int tar_append_tree(TAR *t, char *realdir, char *savedir) { - char realpath[MAXPATHLEN]; - char savepath[MAXPATHLEN]; + char *realpath = NULL; + char *savepath = NULL; struct dirent *dent; DIR *dp; struct stat s; + int len; #ifdef DEBUG printf("==> tar_append_tree(0x%lx, \"%s\", \"%s\")\n", @@ -122,11 +153,19 @@ strcmp(dent->d_name, "..") == 0) continue; - snprintf(realpath, MAXPATHLEN, "%s/%s", realdir, + len = strlen(realdir) + 1 + strlen(dent->d_name); + if ((realpath = malloc(len + 1)) == NULL) + return -1; + snprintf(realpath, len + 1, "%s/%s", realdir, dent->d_name); if (savedir) - snprintf(savepath, MAXPATHLEN, "%s/%s", savedir, + { + len = strlen(savepath) + 1 + strlen(dent->d_name); + if ((realpath = malloc(len + 1)) == NULL) + return -1; + snprintf(savepath, len + 1, "%s/%s", savedir, dent->d_name); + } if (lstat(realpath, &s) != 0) return -1; @@ -135,15 +174,25 @@ { if (tar_append_tree(t, realpath, (savedir ? savepath : NULL)) != 0) + { + free(realpath); + free(savepath); return -1; + } continue; } if (tar_append_file(t, realpath, (savedir ? savepath : NULL)) != 0) - return -1; + { + free(realpath); + free(savepath); + return -1; + } } + free(realpath); + free(savepath); closedir(dp); return 0; diff -ur libtar-1.2.11/libtar/libtar.c libtar-1.2.11.modified/libtar/libtar.c --- libtar-1.2.11/libtar/libtar.c 2012-01-14 17:16:35.000000000 +0100 +++ libtar-1.2.11.modified/libtar/libtar.c 2012-01-21 15:04:14.000000000 +0100 @@ -111,8 +111,9 @@ { TAR *t; char *pathname; - char buf[MAXPATHLEN]; + char *buf = NULL; libtar_listptr_t lp; + int len; if (tar_open(&t, tarfile, #ifdef HAVE_LIBZ @@ -133,15 +134,26 @@ { pathname = (char *)libtar_listptr_data(&lp); if (pathname[0] != '/' && rootdir != NULL) - snprintf(buf, sizeof(buf), "%s/%s", rootdir, pathname); + { + len = strlen(rootdir) + 1 + strlen(pathname); + if ((buf = malloc(len + 1)) == NULL) + return -1; + snprintf(buf, len + 1, "%s/%s", rootdir, pathname); + } else - strlcpy(buf, pathname, sizeof(buf)); + { + len = strlen(pathname); + if ((buf = malloc(len + 1)) == NULL) + return -1; + strlcpy(buf, pathname, len + 1); + } if (tar_append_tree(t, buf, pathname) != 0) { fprintf(stderr, "tar_append_tree(\"%s\", \"%s\"): %s\n", buf, pathname, strerror(errno)); tar_close(t); + free(buf); return -1; } } @@ -150,15 +162,18 @@ { fprintf(stderr, "tar_append_eof(): %s\n", strerror(errno)); tar_close(t); + free(buf); return -1; } if (tar_close(t) != 0) { fprintf(stderr, "tar_close(): %s\n", strerror(errno)); + free(buf); return -1; } + free(buf); return 0; } @@ -286,7 +301,7 @@ { char *tarfile = NULL; char *rootdir = NULL; - int c; + int c, i; int mode = 0; libtar_list_t *l; @@ -348,15 +363,22 @@ switch (mode) { case MODE_EXTRACT: - return extract(argv[optind], rootdir); + i = extract(argv[optind], rootdir); + free(rootdir); + return i; case MODE_CREATE: tarfile = argv[optind]; l = libtar_list_new(LIST_QUEUE, NULL); for (c = optind + 1; c < argc; c++) libtar_list_add(l, argv[c]); - return create(tarfile, rootdir, l); + i = create(tarfile, rootdir, l); + free(l); + free(rootdir); + return i; case MODE_LIST: - return list(argv[optind]); + free(rootdir); + i = list(argv[optind]); + return i;; default: break; }