gnu: libarchive: Fix several security issues.
* gnu/packages/backup.scm (libarchive)[replacement]: New field. (libarchive/fixed): New variable. * gnu/packages/patches/libarchive-7zip-heap-overflow.patch, gnu/packages/patches/libarchive-fix-symlink-check.patch, gnu/packages/patches/libarchive-fix-filesystem-attacks.patch, gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files. * gnu/local.mk (dist_patch_DATA): Add them.
This commit is contained in:
Leo Famulari
parent
85358aef8e
commit
b38e97e03b
6 changed files with 642 additions and 0 deletions
|
|
@ -624,6 +624,10 @@ dist_patch_DATA = \
|
||||||
%D%/packages/patches/liba52-link-with-libm.patch \
|
%D%/packages/patches/liba52-link-with-libm.patch \
|
||||||
%D%/packages/patches/liba52-set-soname.patch \
|
%D%/packages/patches/liba52-set-soname.patch \
|
||||||
%D%/packages/patches/liba52-use-mtune-not-mcpu.patch \
|
%D%/packages/patches/liba52-use-mtune-not-mcpu.patch \
|
||||||
|
%D%/packages/patches/libarchive-7zip-heap-overflow.patch \
|
||||||
|
%D%/packages/patches/libarchive-fix-symlink-check.patch \
|
||||||
|
%D%/packages/patches/libarchive-fix-filesystem-attacks.patch \
|
||||||
|
%D%/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch \
|
||||||
%D%/packages/patches/libbonobo-activation-test-race.patch \
|
%D%/packages/patches/libbonobo-activation-test-race.patch \
|
||||||
%D%/packages/patches/libcanberra-sound-theme-freedesktop.patch \
|
%D%/packages/patches/libcanberra-sound-theme-freedesktop.patch \
|
||||||
%D%/packages/patches/libcmis-fix-test-onedrive.patch \
|
%D%/packages/patches/libcmis-fix-test-onedrive.patch \
|
||||||
|
|
|
||||||
|
|
@ -172,6 +172,7 @@ backups (called chunks) to allow easy burning to CD/DVD.")
|
||||||
(define-public libarchive
|
(define-public libarchive
|
||||||
(package
|
(package
|
||||||
(name "libarchive")
|
(name "libarchive")
|
||||||
|
(replacement libarchive/fixed)
|
||||||
(version "3.2.1")
|
(version "3.2.1")
|
||||||
(source
|
(source
|
||||||
(origin
|
(origin
|
||||||
|
|
@ -227,6 +228,17 @@ archive. In particular, note that there is currently no built-in support for
|
||||||
random access nor for in-place modification.")
|
random access nor for in-place modification.")
|
||||||
(license license:bsd-2)))
|
(license license:bsd-2)))
|
||||||
|
|
||||||
|
(define libarchive/fixed
|
||||||
|
(package
|
||||||
|
(inherit libarchive)
|
||||||
|
(source (origin
|
||||||
|
(inherit (package-source libarchive))
|
||||||
|
(patches (search-patches
|
||||||
|
"libarchive-7zip-heap-overflow.patch"
|
||||||
|
"libarchive-fix-symlink-check.patch"
|
||||||
|
"libarchive-fix-filesystem-attacks.patch"
|
||||||
|
"libarchive-safe_fprintf-buffer-overflow.patch"))))))
|
||||||
|
|
||||||
(define-public rdup
|
(define-public rdup
|
||||||
(package
|
(package
|
||||||
(name "rdup")
|
(name "rdup")
|
||||||
|
|
|
||||||
77
gnu/packages/patches/libarchive-7zip-heap-overflow.patch
Normal file
77
gnu/packages/patches/libarchive-7zip-heap-overflow.patch
Normal file
|
|
@ -0,0 +1,77 @@
|
||||||
|
Fix buffer overflow reading 7Zip files:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/issues/761
|
||||||
|
|
||||||
|
Patch copied from upstream repository:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/commit/7f17c791dcfd8c0416e2cd2485b19410e47ef126
|
||||||
|
|
||||||
|
From 7f17c791dcfd8c0416e2cd2485b19410e47ef126 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tim Kientzle <kientzle@acm.org>
|
||||||
|
Date: Sun, 18 Sep 2016 18:14:58 -0700
|
||||||
|
Subject: [PATCH] Issue 761: Heap overflow reading corrupted 7Zip files
|
||||||
|
|
||||||
|
The sample file that demonstrated this had multiple 'EmptyStream'
|
||||||
|
attributes. The first one ended up being used to calculate
|
||||||
|
certain statistics, then was overwritten by the second which
|
||||||
|
was incompatible with those statistics.
|
||||||
|
|
||||||
|
The fix here is to reject any header with multiple EmptyStream
|
||||||
|
attributes. While here, also reject headers with multiple
|
||||||
|
EmptyFile, AntiFile, Name, or Attributes markers.
|
||||||
|
---
|
||||||
|
libarchive/archive_read_support_format_7zip.c | 10 ++++++++++
|
||||||
|
1 file changed, 10 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/libarchive/archive_read_support_format_7zip.c b/libarchive/archive_read_support_format_7zip.c
|
||||||
|
index 1dfe52b..c0a536c 100644
|
||||||
|
--- a/libarchive/archive_read_support_format_7zip.c
|
||||||
|
+++ b/libarchive/archive_read_support_format_7zip.c
|
||||||
|
@@ -2431,6 +2431,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
|
||||||
|
|
||||||
|
switch (type) {
|
||||||
|
case kEmptyStream:
|
||||||
|
+ if (h->emptyStreamBools != NULL)
|
||||||
|
+ return (-1);
|
||||||
|
h->emptyStreamBools = calloc((size_t)zip->numFiles,
|
||||||
|
sizeof(*h->emptyStreamBools));
|
||||||
|
if (h->emptyStreamBools == NULL)
|
||||||
|
@@ -2451,6 +2453,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
|
||||||
|
return (-1);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
+ if (h->emptyFileBools != NULL)
|
||||||
|
+ return (-1);
|
||||||
|
h->emptyFileBools = calloc(empty_streams,
|
||||||
|
sizeof(*h->emptyFileBools));
|
||||||
|
if (h->emptyFileBools == NULL)
|
||||||
|
@@ -2465,6 +2469,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
|
||||||
|
return (-1);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
+ if (h->antiBools != NULL)
|
||||||
|
+ return (-1);
|
||||||
|
h->antiBools = calloc(empty_streams,
|
||||||
|
sizeof(*h->antiBools));
|
||||||
|
if (h->antiBools == NULL)
|
||||||
|
@@ -2491,6 +2497,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
|
||||||
|
if ((ll & 1) || ll < zip->numFiles * 4)
|
||||||
|
return (-1);
|
||||||
|
|
||||||
|
+ if (zip->entry_names != NULL)
|
||||||
|
+ return (-1);
|
||||||
|
zip->entry_names = malloc(ll);
|
||||||
|
if (zip->entry_names == NULL)
|
||||||
|
return (-1);
|
||||||
|
@@ -2543,6 +2551,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
|
||||||
|
if ((p = header_bytes(a, 2)) == NULL)
|
||||||
|
return (-1);
|
||||||
|
allAreDefined = *p;
|
||||||
|
+ if (h->attrBools != NULL)
|
||||||
|
+ return (-1);
|
||||||
|
h->attrBools = calloc((size_t)zip->numFiles,
|
||||||
|
sizeof(*h->attrBools));
|
||||||
|
if (h->attrBools == NULL)
|
||||||
|
--
|
||||||
|
2.10.0
|
||||||
|
|
||||||
445
gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
Normal file
445
gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
Normal file
|
|
@ -0,0 +1,445 @@
|
||||||
|
This patch fixes two bugs that allow attackers to overwrite or change
|
||||||
|
the permissions of arbitrary files:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/issues/745
|
||||||
|
https://github.com/libarchive/libarchive/issues/746
|
||||||
|
|
||||||
|
Patch copied from upstream repository:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/commit/dfd6b54ce33960e420fb206d8872fb759b577ad9
|
||||||
|
|
||||||
|
From dfd6b54ce33960e420fb206d8872fb759b577ad9 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tim Kientzle <kientzle@acm.org>
|
||||||
|
Date: Sun, 11 Sep 2016 13:21:57 -0700
|
||||||
|
Subject: [PATCH] Fixes for Issue #745 and Issue #746 from Doran Moppert.
|
||||||
|
|
||||||
|
---
|
||||||
|
libarchive/archive_write_disk_posix.c | 294 ++++++++++++++++++++++++++--------
|
||||||
|
1 file changed, 227 insertions(+), 67 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
|
||||||
|
index 8f0421e..abe1a86 100644
|
||||||
|
--- a/libarchive/archive_write_disk_posix.c
|
||||||
|
+++ b/libarchive/archive_write_disk_posix.c
|
||||||
|
@@ -326,12 +326,14 @@ struct archive_write_disk {
|
||||||
|
|
||||||
|
#define HFS_BLOCKS(s) ((s) >> 12)
|
||||||
|
|
||||||
|
+static int check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
|
||||||
|
static int check_symlinks(struct archive_write_disk *);
|
||||||
|
static int create_filesystem_object(struct archive_write_disk *);
|
||||||
|
static struct fixup_entry *current_fixup(struct archive_write_disk *, const char *pathname);
|
||||||
|
#if defined(HAVE_FCHDIR) && defined(PATH_MAX)
|
||||||
|
static void edit_deep_directories(struct archive_write_disk *ad);
|
||||||
|
#endif
|
||||||
|
+static int cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
|
||||||
|
static int cleanup_pathname(struct archive_write_disk *);
|
||||||
|
static int create_dir(struct archive_write_disk *, char *);
|
||||||
|
static int create_parent_dir(struct archive_write_disk *, char *);
|
||||||
|
@@ -2014,6 +2016,10 @@ create_filesystem_object(struct archive_write_disk *a)
|
||||||
|
const char *linkname;
|
||||||
|
mode_t final_mode, mode;
|
||||||
|
int r;
|
||||||
|
+ /* these for check_symlinks_fsobj */
|
||||||
|
+ char *linkname_copy; /* non-const copy of linkname */
|
||||||
|
+ struct archive_string error_string;
|
||||||
|
+ int error_number;
|
||||||
|
|
||||||
|
/* We identify hard/symlinks according to the link names. */
|
||||||
|
/* Since link(2) and symlink(2) don't handle modes, we're done here. */
|
||||||
|
@@ -2022,6 +2028,27 @@ create_filesystem_object(struct archive_write_disk *a)
|
||||||
|
#if !HAVE_LINK
|
||||||
|
return (EPERM);
|
||||||
|
#else
|
||||||
|
+ archive_string_init(&error_string);
|
||||||
|
+ linkname_copy = strdup(linkname);
|
||||||
|
+ if (linkname_copy == NULL) {
|
||||||
|
+ return (EPERM);
|
||||||
|
+ }
|
||||||
|
+ /* TODO: consider using the cleaned-up path as the link target? */
|
||||||
|
+ r = cleanup_pathname_fsobj(linkname_copy, &error_number, &error_string, a->flags);
|
||||||
|
+ if (r != ARCHIVE_OK) {
|
||||||
|
+ archive_set_error(&a->archive, error_number, "%s", error_string.s);
|
||||||
|
+ free(linkname_copy);
|
||||||
|
+ /* EPERM is more appropriate than error_number for our callers */
|
||||||
|
+ return (EPERM);
|
||||||
|
+ }
|
||||||
|
+ r = check_symlinks_fsobj(linkname_copy, &error_number, &error_string, a->flags);
|
||||||
|
+ if (r != ARCHIVE_OK) {
|
||||||
|
+ archive_set_error(&a->archive, error_number, "%s", error_string.s);
|
||||||
|
+ free(linkname_copy);
|
||||||
|
+ /* EPERM is more appropriate than error_number for our callers */
|
||||||
|
+ return (EPERM);
|
||||||
|
+ }
|
||||||
|
+ free(linkname_copy);
|
||||||
|
r = link(linkname, a->name) ? errno : 0;
|
||||||
|
/*
|
||||||
|
* New cpio and pax formats allow hardlink entries
|
||||||
|
@@ -2362,115 +2389,228 @@ current_fixup(struct archive_write_disk *a, const char *pathname)
|
||||||
|
* recent paths.
|
||||||
|
*/
|
||||||
|
/* TODO: Extend this to support symlinks on Windows Vista and later. */
|
||||||
|
+
|
||||||
|
+/*
|
||||||
|
+ * Checks the given path to see if any elements along it are symlinks. Returns
|
||||||
|
+ * ARCHIVE_OK if there are none, otherwise puts an error in errmsg.
|
||||||
|
+ */
|
||||||
|
static int
|
||||||
|
-check_symlinks(struct archive_write_disk *a)
|
||||||
|
+check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
|
||||||
|
{
|
||||||
|
#if !defined(HAVE_LSTAT)
|
||||||
|
/* Platform doesn't have lstat, so we can't look for symlinks. */
|
||||||
|
(void)a; /* UNUSED */
|
||||||
|
+ (void)path; /* UNUSED */
|
||||||
|
+ (void)error_number; /* UNUSED */
|
||||||
|
+ (void)error_string; /* UNUSED */
|
||||||
|
+ (void)flags; /* UNUSED */
|
||||||
|
return (ARCHIVE_OK);
|
||||||
|
#else
|
||||||
|
- char *pn;
|
||||||
|
+ int res = ARCHIVE_OK;
|
||||||
|
+ char *tail;
|
||||||
|
+ char *head;
|
||||||
|
+ int last;
|
||||||
|
char c;
|
||||||
|
int r;
|
||||||
|
struct stat st;
|
||||||
|
+ int restore_pwd;
|
||||||
|
+
|
||||||
|
+ /* Nothing to do here if name is empty */
|
||||||
|
+ if(path[0] == '\0')
|
||||||
|
+ return (ARCHIVE_OK);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Guard against symlink tricks. Reject any archive entry whose
|
||||||
|
* destination would be altered by a symlink.
|
||||||
|
+ *
|
||||||
|
+ * Walk the filename in chunks separated by '/'. For each segment:
|
||||||
|
+ * - if it doesn't exist, continue
|
||||||
|
+ * - if it's symlink, abort or remove it
|
||||||
|
+ * - if it's a directory and it's not the last chunk, cd into it
|
||||||
|
+ * As we go:
|
||||||
|
+ * head points to the current (relative) path
|
||||||
|
+ * tail points to the temporary \0 terminating the segment we're currently examining
|
||||||
|
+ * c holds what used to be in *tail
|
||||||
|
+ * last is 1 if this is the last tail
|
||||||
|
*/
|
||||||
|
- /* Whatever we checked last time doesn't need to be re-checked. */
|
||||||
|
- pn = a->name;
|
||||||
|
- if (archive_strlen(&(a->path_safe)) > 0) {
|
||||||
|
- char *p = a->path_safe.s;
|
||||||
|
- while ((*pn != '\0') && (*p == *pn))
|
||||||
|
- ++p, ++pn;
|
||||||
|
- }
|
||||||
|
+ restore_pwd = open(".", O_RDONLY | O_BINARY | O_CLOEXEC);
|
||||||
|
+ __archive_ensure_cloexec_flag(restore_pwd);
|
||||||
|
+ if (restore_pwd < 0)
|
||||||
|
+ return (ARCHIVE_FATAL);
|
||||||
|
+ head = path;
|
||||||
|
+ tail = path;
|
||||||
|
+ last = 0;
|
||||||
|
+ /* TODO: reintroduce a safe cache here? */
|
||||||
|
/* Skip the root directory if the path is absolute. */
|
||||||
|
- if(pn == a->name && pn[0] == '/')
|
||||||
|
- ++pn;
|
||||||
|
- c = pn[0];
|
||||||
|
- /* Keep going until we've checked the entire name. */
|
||||||
|
- while (pn[0] != '\0' && (pn[0] != '/' || pn[1] != '\0')) {
|
||||||
|
+ if(tail == path && tail[0] == '/')
|
||||||
|
+ ++tail;
|
||||||
|
+ /* Keep going until we've checked the entire name.
|
||||||
|
+ * head, tail, path all alias the same string, which is
|
||||||
|
+ * temporarily zeroed at tail, so be careful restoring the
|
||||||
|
+ * stashed (c=tail[0]) for error messages.
|
||||||
|
+ * Exiting the loop with break is okay; continue is not.
|
||||||
|
+ */
|
||||||
|
+ while (!last) {
|
||||||
|
+ /* Skip the separator we just consumed, plus any adjacent ones */
|
||||||
|
+ while (*tail == '/')
|
||||||
|
+ ++tail;
|
||||||
|
/* Skip the next path element. */
|
||||||
|
- while (*pn != '\0' && *pn != '/')
|
||||||
|
- ++pn;
|
||||||
|
- c = pn[0];
|
||||||
|
- pn[0] = '\0';
|
||||||
|
+ while (*tail != '\0' && *tail != '/')
|
||||||
|
+ ++tail;
|
||||||
|
+ /* is this the last path component? */
|
||||||
|
+ last = (tail[0] == '\0') || (tail[0] == '/' && tail[1] == '\0');
|
||||||
|
+ /* temporarily truncate the string here */
|
||||||
|
+ c = tail[0];
|
||||||
|
+ tail[0] = '\0';
|
||||||
|
/* Check that we haven't hit a symlink. */
|
||||||
|
- r = lstat(a->name, &st);
|
||||||
|
+ r = lstat(head, &st);
|
||||||
|
if (r != 0) {
|
||||||
|
+ tail[0] = c;
|
||||||
|
/* We've hit a dir that doesn't exist; stop now. */
|
||||||
|
if (errno == ENOENT) {
|
||||||
|
break;
|
||||||
|
} else {
|
||||||
|
- /* Note: This effectively disables deep directory
|
||||||
|
+ /* Treat any other error as fatal - best to be paranoid here
|
||||||
|
+ * Note: This effectively disables deep directory
|
||||||
|
* support when security checks are enabled.
|
||||||
|
* Otherwise, very long pathnames that trigger
|
||||||
|
* an error here could evade the sandbox.
|
||||||
|
* TODO: We could do better, but it would probably
|
||||||
|
* require merging the symlink checks with the
|
||||||
|
* deep-directory editing. */
|
||||||
|
- return (ARCHIVE_FAILED);
|
||||||
|
+ if (error_number) *error_number = errno;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Could not stat %s",
|
||||||
|
+ path);
|
||||||
|
+ res = ARCHIVE_FAILED;
|
||||||
|
+ break;
|
||||||
|
+ }
|
||||||
|
+ } else if (S_ISDIR(st.st_mode)) {
|
||||||
|
+ if (!last) {
|
||||||
|
+ if (chdir(head) != 0) {
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ if (error_number) *error_number = errno;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Could not chdir %s",
|
||||||
|
+ path);
|
||||||
|
+ res = (ARCHIVE_FATAL);
|
||||||
|
+ break;
|
||||||
|
+ }
|
||||||
|
+ /* Our view is now from inside this dir: */
|
||||||
|
+ head = tail + 1;
|
||||||
|
}
|
||||||
|
} else if (S_ISLNK(st.st_mode)) {
|
||||||
|
- if (c == '\0') {
|
||||||
|
+ if (last) {
|
||||||
|
/*
|
||||||
|
* Last element is symlink; remove it
|
||||||
|
* so we can overwrite it with the
|
||||||
|
* item being extracted.
|
||||||
|
*/
|
||||||
|
- if (unlink(a->name)) {
|
||||||
|
- archive_set_error(&a->archive, errno,
|
||||||
|
- "Could not remove symlink %s",
|
||||||
|
- a->name);
|
||||||
|
- pn[0] = c;
|
||||||
|
- return (ARCHIVE_FAILED);
|
||||||
|
+ if (unlink(head)) {
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ if (error_number) *error_number = errno;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Could not remove symlink %s",
|
||||||
|
+ path);
|
||||||
|
+ res = ARCHIVE_FAILED;
|
||||||
|
+ break;
|
||||||
|
}
|
||||||
|
- a->pst = NULL;
|
||||||
|
/*
|
||||||
|
* Even if we did remove it, a warning
|
||||||
|
* is in order. The warning is silly,
|
||||||
|
* though, if we're just replacing one
|
||||||
|
* symlink with another symlink.
|
||||||
|
*/
|
||||||
|
- if (!S_ISLNK(a->mode)) {
|
||||||
|
- archive_set_error(&a->archive, 0,
|
||||||
|
- "Removing symlink %s",
|
||||||
|
- a->name);
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ /* FIXME: not sure how important this is to restore
|
||||||
|
+ if (!S_ISLNK(path)) {
|
||||||
|
+ if (error_number) *error_number = 0;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Removing symlink %s",
|
||||||
|
+ path);
|
||||||
|
}
|
||||||
|
+ */
|
||||||
|
/* Symlink gone. No more problem! */
|
||||||
|
- pn[0] = c;
|
||||||
|
- return (0);
|
||||||
|
- } else if (a->flags & ARCHIVE_EXTRACT_UNLINK) {
|
||||||
|
+ res = ARCHIVE_OK;
|
||||||
|
+ break;
|
||||||
|
+ } else if (flags & ARCHIVE_EXTRACT_UNLINK) {
|
||||||
|
/* User asked us to remove problems. */
|
||||||
|
- if (unlink(a->name) != 0) {
|
||||||
|
- archive_set_error(&a->archive, 0,
|
||||||
|
- "Cannot remove intervening symlink %s",
|
||||||
|
- a->name);
|
||||||
|
- pn[0] = c;
|
||||||
|
- return (ARCHIVE_FAILED);
|
||||||
|
+ if (unlink(head) != 0) {
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ if (error_number) *error_number = 0;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Cannot remove intervening symlink %s",
|
||||||
|
+ path);
|
||||||
|
+ res = ARCHIVE_FAILED;
|
||||||
|
+ break;
|
||||||
|
}
|
||||||
|
- a->pst = NULL;
|
||||||
|
+ tail[0] = c;
|
||||||
|
} else {
|
||||||
|
- archive_set_error(&a->archive, 0,
|
||||||
|
- "Cannot extract through symlink %s",
|
||||||
|
- a->name);
|
||||||
|
- pn[0] = c;
|
||||||
|
- return (ARCHIVE_FAILED);
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ if (error_number) *error_number = 0;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Cannot extract through symlink %s",
|
||||||
|
+ path);
|
||||||
|
+ res = ARCHIVE_FAILED;
|
||||||
|
+ break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
- pn[0] = c;
|
||||||
|
- if (pn[0] != '\0')
|
||||||
|
- pn++; /* Advance to the next segment. */
|
||||||
|
+ /* be sure to always maintain this */
|
||||||
|
+ tail[0] = c;
|
||||||
|
+ if (tail[0] != '\0')
|
||||||
|
+ tail++; /* Advance to the next segment. */
|
||||||
|
}
|
||||||
|
- pn[0] = c;
|
||||||
|
- /* We've checked and/or cleaned the whole path, so remember it. */
|
||||||
|
- archive_strcpy(&a->path_safe, a->name);
|
||||||
|
- return (ARCHIVE_OK);
|
||||||
|
+ /* Catches loop exits via break */
|
||||||
|
+ tail[0] = c;
|
||||||
|
+#ifdef HAVE_FCHDIR
|
||||||
|
+ /* If we changed directory above, restore it here. */
|
||||||
|
+ if (restore_pwd >= 0) {
|
||||||
|
+ r = fchdir(restore_pwd);
|
||||||
|
+ if (r != 0) {
|
||||||
|
+ if(error_number) *error_number = errno;
|
||||||
|
+ if(error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "chdir() failure");
|
||||||
|
+ }
|
||||||
|
+ close(restore_pwd);
|
||||||
|
+ restore_pwd = -1;
|
||||||
|
+ if (r != 0) {
|
||||||
|
+ res = (ARCHIVE_FATAL);
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
+#endif
|
||||||
|
+ /* TODO: reintroduce a safe cache here? */
|
||||||
|
+ return res;
|
||||||
|
#endif
|
||||||
|
}
|
||||||
|
|
||||||
|
+/*
|
||||||
|
+ * Check a->name for symlinks, returning ARCHIVE_OK if its clean, otherwise
|
||||||
|
+ * calls archive_set_error and returns ARCHIVE_{FATAL,FAILED}
|
||||||
|
+ */
|
||||||
|
+static int
|
||||||
|
+check_symlinks(struct archive_write_disk *a)
|
||||||
|
+{
|
||||||
|
+ struct archive_string error_string;
|
||||||
|
+ int error_number;
|
||||||
|
+ int rc;
|
||||||
|
+ archive_string_init(&error_string);
|
||||||
|
+ rc = check_symlinks_fsobj(a->name, &error_number, &error_string, a->flags);
|
||||||
|
+ if (rc != ARCHIVE_OK) {
|
||||||
|
+ archive_set_error(&a->archive, error_number, "%s", error_string.s);
|
||||||
|
+ }
|
||||||
|
+ archive_string_free(&error_string);
|
||||||
|
+ a->pst = NULL; /* to be safe */
|
||||||
|
+ return rc;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+
|
||||||
|
#if defined(__CYGWIN__)
|
||||||
|
/*
|
||||||
|
* 1. Convert a path separator from '\' to '/' .
|
||||||
|
@@ -2544,15 +2684,17 @@ cleanup_pathname_win(struct archive_write_disk *a)
|
||||||
|
* is set) if the path is absolute.
|
||||||
|
*/
|
||||||
|
static int
|
||||||
|
-cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
+cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
|
||||||
|
{
|
||||||
|
char *dest, *src;
|
||||||
|
char separator = '\0';
|
||||||
|
|
||||||
|
- dest = src = a->name;
|
||||||
|
+ dest = src = path;
|
||||||
|
if (*src == '\0') {
|
||||||
|
- archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
|
||||||
|
- "Invalid empty pathname");
|
||||||
|
+ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Invalid empty pathname");
|
||||||
|
return (ARCHIVE_FAILED);
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -2561,9 +2703,11 @@ cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
#endif
|
||||||
|
/* Skip leading '/'. */
|
||||||
|
if (*src == '/') {
|
||||||
|
- if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
|
||||||
|
- archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
|
||||||
|
- "Path is absolute");
|
||||||
|
+ if (flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
|
||||||
|
+ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Path is absolute");
|
||||||
|
return (ARCHIVE_FAILED);
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -2590,10 +2734,11 @@ cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
} else if (src[1] == '.') {
|
||||||
|
if (src[2] == '/' || src[2] == '\0') {
|
||||||
|
/* Conditionally warn about '..' */
|
||||||
|
- if (a->flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
|
||||||
|
- archive_set_error(&a->archive,
|
||||||
|
- ARCHIVE_ERRNO_MISC,
|
||||||
|
- "Path contains '..'");
|
||||||
|
+ if (flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
|
||||||
|
+ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
|
||||||
|
+ if (error_string)
|
||||||
|
+ archive_string_sprintf(error_string,
|
||||||
|
+ "Path contains '..'");
|
||||||
|
return (ARCHIVE_FAILED);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@@ -2624,7 +2769,7 @@ cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
* We've just copied zero or more path elements, not including the
|
||||||
|
* final '/'.
|
||||||
|
*/
|
||||||
|
- if (dest == a->name) {
|
||||||
|
+ if (dest == path) {
|
||||||
|
/*
|
||||||
|
* Nothing got copied. The path must have been something
|
||||||
|
* like '.' or '/' or './' or '/././././/./'.
|
||||||
|
@@ -2639,6 +2784,21 @@ cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
return (ARCHIVE_OK);
|
||||||
|
}
|
||||||
|
|
||||||
|
+static int
|
||||||
|
+cleanup_pathname(struct archive_write_disk *a)
|
||||||
|
+{
|
||||||
|
+ struct archive_string error_string;
|
||||||
|
+ int error_number;
|
||||||
|
+ int rc;
|
||||||
|
+ archive_string_init(&error_string);
|
||||||
|
+ rc = cleanup_pathname_fsobj(a->name, &error_number, &error_string, a->flags);
|
||||||
|
+ if (rc != ARCHIVE_OK) {
|
||||||
|
+ archive_set_error(&a->archive, error_number, "%s", error_string.s);
|
||||||
|
+ }
|
||||||
|
+ archive_string_free(&error_string);
|
||||||
|
+ return rc;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
/*
|
||||||
|
* Create the parent directory of the specified path, assuming path
|
||||||
|
* is already in mutable storage.
|
||||||
60
gnu/packages/patches/libarchive-fix-symlink-check.patch
Normal file
60
gnu/packages/patches/libarchive-fix-symlink-check.patch
Normal file
|
|
@ -0,0 +1,60 @@
|
||||||
|
Make sure to check for symlinks even if the pathname is very long:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/issues/744
|
||||||
|
|
||||||
|
Patch copied from upstream repository:
|
||||||
|
|
||||||
|
https://github.com/libarchive/libarchive/commit/1fa9c7bf90f0862036a99896b0501c381584451a
|
||||||
|
|
||||||
|
From 1fa9c7bf90f0862036a99896b0501c381584451a Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tim Kientzle <kientzle@acm.org>
|
||||||
|
Date: Sun, 21 Aug 2016 17:11:45 -0700
|
||||||
|
Subject: [PATCH] Issue #744 (part of Issue #743): Enforce sandbox with very
|
||||||
|
long pathnames
|
||||||
|
|
||||||
|
Because check_symlinks is handled separately from the deep-directory
|
||||||
|
support, very long pathnames cause problems. Previously, the code
|
||||||
|
ignored most failures to lstat() a path component. In particular,
|
||||||
|
this led to check_symlinks always passing for very long paths, which
|
||||||
|
in turn provides a way to evade the symlink checks in the sandboxing
|
||||||
|
code.
|
||||||
|
|
||||||
|
We now fail on unrecognized lstat() failures, which plugs this
|
||||||
|
hole at the cost of disabling deep directory support when the
|
||||||
|
user requests sandboxing.
|
||||||
|
|
||||||
|
TODO: This probably cannot be completely fixed without
|
||||||
|
entirely reimplementing the deep directory support to
|
||||||
|
integrate the symlink checks. I want to reimplement the
|
||||||
|
deep directory hanlding someday anyway; openat() and
|
||||||
|
related system calls now provide a much cleaner way to
|
||||||
|
handle deep directories than the chdir approach used by this
|
||||||
|
code.
|
||||||
|
---
|
||||||
|
libarchive/archive_write_disk_posix.c | 12 +++++++++++-
|
||||||
|
1 file changed, 11 insertions(+), 1 deletion(-)
|
||||||
|
|
||||||
|
diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
|
||||||
|
index 39ee3b6..8f0421e 100644
|
||||||
|
--- a/libarchive/archive_write_disk_posix.c
|
||||||
|
+++ b/libarchive/archive_write_disk_posix.c
|
||||||
|
@@ -2401,8 +2401,18 @@ check_symlinks(struct archive_write_disk *a)
|
||||||
|
r = lstat(a->name, &st);
|
||||||
|
if (r != 0) {
|
||||||
|
/* We've hit a dir that doesn't exist; stop now. */
|
||||||
|
- if (errno == ENOENT)
|
||||||
|
+ if (errno == ENOENT) {
|
||||||
|
break;
|
||||||
|
+ } else {
|
||||||
|
+ /* Note: This effectively disables deep directory
|
||||||
|
+ * support when security checks are enabled.
|
||||||
|
+ * Otherwise, very long pathnames that trigger
|
||||||
|
+ * an error here could evade the sandbox.
|
||||||
|
+ * TODO: We could do better, but it would probably
|
||||||
|
+ * require merging the symlink checks with the
|
||||||
|
+ * deep-directory editing. */
|
||||||
|
+ return (ARCHIVE_FAILED);
|
||||||
|
+ }
|
||||||
|
} else if (S_ISLNK(st.st_mode)) {
|
||||||
|
if (c == '\0') {
|
||||||
|
/*
|
||||||
|
|
@ -0,0 +1,44 @@
|
||||||
|
Fixes this buffer overflow:
|
||||||
|
https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
|
||||||
|
|
||||||
|
Patch copied from upstream source repository:
|
||||||
|
https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
|
||||||
|
|
||||||
|
From e37b620fe8f14535d737e89a4dcabaed4517bf1a Mon Sep 17 00:00:00 2001
|
||||||
|
From: Tim Kientzle <kientzle@acm.org>
|
||||||
|
Date: Sun, 21 Aug 2016 10:51:43 -0700
|
||||||
|
Subject: [PATCH] Issue #767: Buffer overflow printing a filename
|
||||||
|
|
||||||
|
The safe_fprintf function attempts to ensure clean output for an
|
||||||
|
arbitrary sequence of bytes by doing a trial conversion of the
|
||||||
|
multibyte characters to wide characters -- if the resulting wide
|
||||||
|
character is printable then we pass through the corresponding bytes
|
||||||
|
unaltered, otherwise, we convert them to C-style ASCII escapes.
|
||||||
|
|
||||||
|
The stack trace in Issue #767 suggest that the 20-byte buffer
|
||||||
|
was getting overflowed trying to format a non-printable multibyte
|
||||||
|
character. This should only happen if there is a valid multibyte
|
||||||
|
character of more than 5 bytes that was unprintable. (Each byte
|
||||||
|
would get expanded to a four-charcter octal-style escape of the form
|
||||||
|
"\123" resulting in >20 characters for the >5 byte multibyte character.)
|
||||||
|
|
||||||
|
I've not been able to reproduce this, but have expanded the conversion
|
||||||
|
buffer to 128 bytes on the belief that no multibyte character set
|
||||||
|
has a single character of more than 32 bytes.
|
||||||
|
---
|
||||||
|
tar/util.c | 2 +-
|
||||||
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||||
|
|
||||||
|
diff --git a/tar/util.c b/tar/util.c
|
||||||
|
index 9ff22f2..2b4aebe 100644
|
||||||
|
--- a/tar/util.c
|
||||||
|
+++ b/tar/util.c
|
||||||
|
@@ -182,7 +182,7 @@ safe_fprintf(FILE *f, const char *fmt, ...)
|
||||||
|
}
|
||||||
|
|
||||||
|
/* If our output buffer is full, dump it and keep going. */
|
||||||
|
- if (i > (sizeof(outbuff) - 20)) {
|
||||||
|
+ if (i > (sizeof(outbuff) - 128)) {
|
||||||
|
outbuff[i] = '\0';
|
||||||
|
fprintf(f, "%s", outbuff);
|
||||||
|
i = 0;
|
||||||
Reference in a new issue