From e3af2353c4be4d158a6e21613b28d7e9fad1528c Mon Sep 17 00:00:00 2001 From: kientzle Date: Sun, 2 May 2004 00:43:02 +0000 Subject: A security issue: An archive containing a symlink to another directory, then a file with that symlink as a prefix can drop a file outside of the current directory, which can be a security hole. Plug this hole by refusing to extract files if a prefix of the pathname is a symlink. The -P option disables this check. --- usr.bin/tar/bsdtar.h | 1 + usr.bin/tar/read.c | 83 ++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 71 insertions(+), 13 deletions(-) (limited to 'usr.bin') diff --git a/usr.bin/tar/bsdtar.h b/usr.bin/tar/bsdtar.h index 4b1ef32..cecaae7 100644 --- a/usr.bin/tar/bsdtar.h +++ b/usr.bin/tar/bsdtar.h @@ -79,6 +79,7 @@ struct bsdtar { struct gname_cache *gname_cache; /* for write.c */ struct links_cache *links_cache; /* for write.c */ struct matching *matching; /* for matching.c */ + struct security *security; /* for read.c */ struct uname_cache *uname_cache; /* for write.c */ }; diff --git a/usr.bin/tar/read.c b/usr.bin/tar/read.c index 3023ddb..906040c 100644 --- a/usr.bin/tar/read.c +++ b/usr.bin/tar/read.c @@ -27,6 +27,7 @@ #include "bsdtar_platform.h" __FBSDID("$FreeBSD$"); +#include #include #include @@ -44,6 +45,7 @@ __FBSDID("$FreeBSD$"); #include "bsdtar.h" +static void cleanup_security(struct bsdtar *); static void list_item_verbose(struct bsdtar *, struct archive_entry *); static void read_archive(struct bsdtar *bsdtar, char mode); static int security_problem(struct bsdtar *, struct archive_entry *); @@ -184,6 +186,7 @@ read_archive(struct bsdtar *bsdtar, char mode) } } archive_read_finish(a); + cleanup_security(bsdtar); } @@ -285,6 +288,11 @@ list_item_verbose(struct bsdtar *bsdtar, struct archive_entry *entry) safe_fprintf(out, " -> %s", archive_entry_symlink(entry)); } +struct security { + char *path; + size_t path_size; +}; + /* * Check for a variety of security issues. Fix what we can here, * generate warnings as appropriate, return non-zero to prevent @@ -293,30 +301,79 @@ list_item_verbose(struct bsdtar *bsdtar, struct archive_entry *entry) int security_problem(struct bsdtar *bsdtar, struct archive_entry *entry) { - const char *name, *p; + struct stat st; + const char *name, *pn; + char *p; + int r; + + /* -P option forces us to just accept all pathnames. */ + if (bsdtar->option_absolute_paths) + return (0); - /* Strip leading '/' unless -P is specified. */ + /* Strip leading '/'. */ name = archive_entry_pathname(entry); - if (name[0] == '/' && !bsdtar->option_absolute_paths) { + if (name[0] == '/') { /* XXX gtar generates a warning the first time this happens. */ name++; archive_entry_set_pathname(entry, name); } /* Reject any archive entry with '..' as a path element. */ - p = name; - while (p != NULL && p[0] != '\0') { - if (p[0] == '.' && p[1] == '.' && - (p[2] == '\0' || p[2] == '/')) { - bsdtar_warnc(0,"pathname contains ..; skipping %s", - name); + pn = name; + while (pn != NULL && pn[0] != '\0') { + if (pn[0] == '.' && pn[1] == '.' && + (pn[2] == '\0' || pn[2] == '/')) { + bsdtar_warnc(0,"Skipping pathname containing .."); + return (1); + } + pn = strchr(pn, '/'); + if (pn != NULL) + pn++; + } + + /* + * Gaurd against symlink tricks. Reject any archive entry whose + * destination would be altered by a symlink. + */ + /* XXX TODO: Make this faster!!! XXX */ + pn = name; + if (bsdtar->security == NULL) { + bsdtar->security = malloc(sizeof(*bsdtar->security)); + bsdtar->security->path_size = MAXPATHLEN + 1; + bsdtar->security->path = malloc(bsdtar->security->path_size); + } + if (strlen(name) >= bsdtar->security->path_size) { + free(bsdtar->security->path); + while (strlen(name) >= bsdtar->security->path_size) + bsdtar->security->path_size *= 2; + bsdtar->security->path = malloc(bsdtar->security->path_size); + } + p = bsdtar->security->path; + while (pn != NULL && pn[0] != '\0') { + *p++ = *pn++; + while (*pn != '\0' && *pn != '/') + *p++ = *pn++; + p[0] = '\0'; + r = lstat(bsdtar->security->path, &st); + if (r != 0) { + if (errno == ENOENT) + break; + } else if (S_ISLNK(st.st_mode)) { + bsdtar_warnc(0,"Cannot extract %s through symlink %s", + name, bsdtar->security->path); return (1); } - while (*p != '/' && *p != '\0') - p++; - if (*p != '\0') - p++; } return (0); } + +static void +cleanup_security(struct bsdtar *bsdtar) +{ + if (bsdtar->security != NULL) { + if (bsdtar->security->path != NULL) + free(bsdtar->security->path); + free(bsdtar->security); + } +} -- cgit v1.1