Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Sep 2016 21:15:01 +0000 (UTC)
From:      Martin Matuska <mm@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r305819 - in head: contrib/libarchive/libarchive contrib/libarchive/libarchive/test lib/libarchive/tests
Message-ID:  <201609142115.u8ELF1t1019804@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mm
Date: Wed Sep 14 21:15:01 2016
New Revision: 305819
URL: https://svnweb.freebsd.org/changeset/base/305819

Log:
  MFV r305816:
  Sync libarchive with vendor including important security fixes.
  
  Issues fixed (FreeBSD):
  PR #778: ACL error handling
  Issue #745: Symlink check prefix optimization is too aggressive
  Issue #746: Hard links with data can evade sandboxing restrictions
  
  This update fixes the vulnerability #3 and vulnerability #4 as reported in
  "non-cryptanalytic attacks against FreeBSD update components".
  https://gist.github.com/anonymous/e48209b03f1dd9625a992717e7b89c4f
  
  Fix for vulnerability #2 has already been merged in r304989.
  
  MFC after:	1 week
  Security: http://gist.github.com/anonymous/e48209b03f1dd9625a992717e7b89c4f

Modified:
  head/contrib/libarchive/libarchive/archive_platform.h
  head/contrib/libarchive/libarchive/archive_read_disk_entry_from_file.c
  head/contrib/libarchive/libarchive/archive_read_disk_posix.c
  head/contrib/libarchive/libarchive/archive_read_support_format_tar.c
  head/contrib/libarchive/libarchive/archive_write_disk_acl.c
  head/contrib/libarchive/libarchive/archive_write_disk_posix.c
  head/contrib/libarchive/libarchive/test/test_write_disk_secure745.c
  head/contrib/libarchive/libarchive/test/test_write_disk_secure746.c
  head/contrib/libarchive/libarchive/test/test_write_format_gnutar_filenames.c
  head/lib/libarchive/tests/Makefile
Directory Properties:
  head/contrib/libarchive/   (props changed)

Modified: head/contrib/libarchive/libarchive/archive_platform.h
==============================================================================
--- head/contrib/libarchive/libarchive/archive_platform.h	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_platform.h	Wed Sep 14 21:15:01 2016	(r305819)
@@ -159,6 +159,15 @@
 #define	CAN_RESTORE_METADATA_FD
 #endif
 
+/*
+ * glibc 2.24 deprecates readdir_r
+ */
+#if defined(HAVE_READDIR_R) && (!defined(__GLIBC__) || !defined(__GLIBC_MINOR__) || __GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 24))
+#define	USE_READDIR_R	1
+#else
+#undef	USE_READDIR_R
+#endif
+
 /* Set up defaults for internal error codes. */
 #ifndef ARCHIVE_ERRNO_FILE_FORMAT
 #if HAVE_EFTYPE

Modified: head/contrib/libarchive/libarchive/archive_read_disk_entry_from_file.c
==============================================================================
--- head/contrib/libarchive/libarchive/archive_read_disk_entry_from_file.c	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_read_disk_entry_from_file.c	Wed Sep 14 21:15:01 2016	(r305819)
@@ -411,9 +411,7 @@ setup_acls(struct archive_read_disk *a,
 {
 	const char	*accpath;
 	acl_t		 acl;
-#if HAVE_ACL_IS_TRIVIAL_NP
 	int		r;
-#endif
 
 	accpath = archive_entry_sourcepath(entry);
 	if (accpath == NULL)
@@ -473,9 +471,13 @@ setup_acls(struct archive_read_disk *a,
 	}
 #endif
 	if (acl != NULL) {
-		translate_acl(a, entry, acl, ARCHIVE_ENTRY_ACL_TYPE_NFS4);
+		r = translate_acl(a, entry, acl, ARCHIVE_ENTRY_ACL_TYPE_NFS4);
 		acl_free(acl);
-		return (ARCHIVE_OK);
+		if (r != ARCHIVE_OK) {
+			archive_set_error(&a->archive, errno,
+			    "Couldn't translate NFSv4 ACLs: %s", accpath);
+		}
+		return (r);
 	}
 #endif	/* ACL_TYPE_NFS4 */
 
@@ -506,19 +508,30 @@ setup_acls(struct archive_read_disk *a,
 #endif
 
 	if (acl != NULL) {
-		translate_acl(a, entry, acl,
+		r = translate_acl(a, entry, acl,
 		    ARCHIVE_ENTRY_ACL_TYPE_ACCESS);
 		acl_free(acl);
 		acl = NULL;
+		if (r != ARCHIVE_OK) {
+			archive_set_error(&a->archive, errno,
+			    "Couldn't translate access ACLs: %s", accpath);
+			return (r);
+		}
 	}
 
 	/* Only directories can have default ACLs. */
 	if (S_ISDIR(archive_entry_mode(entry))) {
 		acl = acl_get_file(accpath, ACL_TYPE_DEFAULT);
 		if (acl != NULL) {
-			translate_acl(a, entry, acl,
+			r = translate_acl(a, entry, acl,
 			    ARCHIVE_ENTRY_ACL_TYPE_DEFAULT);
 			acl_free(acl);
+			if (r != ARCHIVE_OK) {
+				archive_set_error(&a->archive, errno,
+				    "Couldn't translate default ACLs: %s",
+				    accpath);
+				return (r);
+			}
 		}
 	}
 	return (ARCHIVE_OK);
@@ -574,19 +587,23 @@ translate_acl(struct archive_read_disk *
 #ifdef ACL_TYPE_NFS4
 	acl_entry_type_t acl_type;
 	acl_flagset_t	 acl_flagset;
-	int brand, r;
+	int brand;
 #endif
 	acl_entry_t	 acl_entry;
 	acl_permset_t	 acl_permset;
 	int		 i, entry_acl_type;
-	int		 s, ae_id, ae_tag, ae_perm;
+	int		 r, s, ae_id, ae_tag, ae_perm;
 	const char	*ae_name;
 
 #ifdef ACL_TYPE_NFS4
 	// FreeBSD "brands" ACLs as POSIX.1e or NFSv4
 	// Make sure the "brand" on this ACL is consistent
 	// with the default_entry_acl_type bits provided.
-	acl_get_brand_np(acl, &brand);
+	if (acl_get_brand_np(acl, &brand) != 0) {
+		archive_set_error(&a->archive, errno,
+		    "Failed to read ACL brand");
+		return (ARCHIVE_WARN);
+	}
 	switch (brand) {
 	case ACL_BRAND_POSIX:
 		switch (default_entry_acl_type) {
@@ -594,31 +611,43 @@ translate_acl(struct archive_read_disk *
 		case ARCHIVE_ENTRY_ACL_TYPE_DEFAULT:
 			break;
 		default:
-			// XXX set warning message?
-			return ARCHIVE_FAILED;
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+			    "Invalid ACL entry type for POSIX.1e ACL");
+			return (ARCHIVE_WARN);
 		}
 		break;
 	case ACL_BRAND_NFS4:
 		if (default_entry_acl_type & ~ARCHIVE_ENTRY_ACL_TYPE_NFS4) {
-			// XXX set warning message?
-			return ARCHIVE_FAILED;
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+			    "Invalid ACL entry type for NFSv4 ACL");
+			return (ARCHIVE_WARN);
 		}
 		break;
 	default:
-		// XXX set warning message?
-		return ARCHIVE_FAILED;
+		archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+		    "Unknown ACL brand");
+		return (ARCHIVE_WARN);
 		break;
 	}
 #endif
 
 
 	s = acl_get_entry(acl, ACL_FIRST_ENTRY, &acl_entry);
+	if (s == -1) {
+		archive_set_error(&a->archive, errno,
+		    "Failed to get first ACL entry");
+		return (ARCHIVE_WARN);
+	}
 	while (s == 1) {
 		ae_id = -1;
 		ae_name = NULL;
 		ae_perm = 0;
 
-		acl_get_tag_type(acl_entry, &acl_tag);
+		if (acl_get_tag_type(acl_entry, &acl_tag) != 0) {
+			archive_set_error(&a->archive, errno,
+			    "Failed to get ACL tag type");
+			return (ARCHIVE_WARN);
+		}
 		switch (acl_tag) {
 		case ACL_USER:
 			ae_id = (int)*(uid_t *)acl_get_qualifier(acl_entry);
@@ -653,13 +682,18 @@ translate_acl(struct archive_read_disk *
 			continue;
 		}
 
-		// XXX acl type maps to allow/deny/audit/YYYY bits
-		// XXX acl_get_entry_type_np on FreeBSD returns EINVAL for
-		// non-NFSv4 ACLs
+		// XXX acl_type maps to allow/deny/audit/YYYY bits
 		entry_acl_type = default_entry_acl_type;
 #ifdef ACL_TYPE_NFS4
-		r = acl_get_entry_type_np(acl_entry, &acl_type);
-		if (r == 0) {
+		if (default_entry_acl_type & ARCHIVE_ENTRY_ACL_TYPE_NFS4) {
+			/*
+			 * acl_get_entry_type_np() falis with non-NFSv4 ACLs
+			 */
+			if (acl_get_entry_type_np(acl_entry, &acl_type) != 0) {
+				archive_set_error(&a->archive, errno, "Failed "
+				    "to get ACL type from a NFSv4 ACL entry");
+				return (ARCHIVE_WARN);
+			}
 			switch (acl_type) {
 			case ACL_ENTRY_TYPE_ALLOW:
 				entry_acl_type = ARCHIVE_ENTRY_ACL_TYPE_ALLOW;
@@ -673,32 +707,53 @@ translate_acl(struct archive_read_disk *
 			case ACL_ENTRY_TYPE_ALARM:
 				entry_acl_type = ARCHIVE_ENTRY_ACL_TYPE_ALARM;
 				break;
+			default:
+				archive_set_error(&a->archive, errno,
+				    "Invalid NFSv4 ACL entry type");
+				return (ARCHIVE_WARN);
 			}
-		}
 
-		/*
-		 * Libarchive stores "flag" (NFSv4 inheritance bits)
-		 * in the ae_perm bitmap.
-		 */
-		// XXX acl_get_flagset_np on FreeBSD returns EINVAL for
-		// non-NFSv4 ACLs
-		r = acl_get_flagset_np(acl_entry, &acl_flagset);
-		if (r == 0) {
+			/*
+			 * Libarchive stores "flag" (NFSv4 inheritance bits)
+			 * in the ae_perm bitmap.
+			 *
+			 * acl_get_flagset_np() fails with non-NFSv4 ACLs
+			 */
+			if (acl_get_flagset_np(acl_entry, &acl_flagset) != 0) {
+				archive_set_error(&a->archive, errno,
+				    "Failed to get flagset from a NFSv4 ACL entry");
+				return (ARCHIVE_WARN);
+			}
 	                for (i = 0; i < (int)(sizeof(acl_inherit_map) / sizeof(acl_inherit_map[0])); ++i) {
-				if (acl_get_flag_np(acl_flagset,
-						    acl_inherit_map[i].platform_inherit))
+				r = acl_get_flag_np(acl_flagset,
+				    acl_inherit_map[i].platform_inherit);
+				if (r == -1) {
+					archive_set_error(&a->archive, errno,
+					    "Failed to check flag in a NFSv4 "
+					    "ACL flagset");
+					return (ARCHIVE_WARN);
+				} else if (r)
 					ae_perm |= acl_inherit_map[i].archive_inherit;
                 	}
 		}
 #endif
 
-		acl_get_permset(acl_entry, &acl_permset);
+		if (acl_get_permset(acl_entry, &acl_permset) != 0) {
+			archive_set_error(&a->archive, errno,
+			    "Failed to get ACL permission set");
+			return (ARCHIVE_WARN);
+		}
 		for (i = 0; i < (int)(sizeof(acl_perm_map) / sizeof(acl_perm_map[0])); ++i) {
 			/*
 			 * acl_get_perm() is spelled differently on different
 			 * platforms; see above.
 			 */
-			if (ACL_GET_PERM(acl_permset, acl_perm_map[i].platform_perm))
+			r = ACL_GET_PERM(acl_permset, acl_perm_map[i].platform_perm);
+			if (r == -1) {
+				archive_set_error(&a->archive, errno,
+				    "Failed to check permission in an ACL permission set");
+				return (ARCHIVE_WARN);
+			} else if (r)
 				ae_perm |= acl_perm_map[i].archive_perm;
 		}
 
@@ -707,6 +762,11 @@ translate_acl(struct archive_read_disk *
 					    ae_id, ae_name);
 
 		s = acl_get_entry(acl, ACL_NEXT_ENTRY, &acl_entry);
+		if (s == -1) {
+			archive_set_error(&a->archive, errno,
+			    "Failed to get next ACL entry");
+			return (ARCHIVE_WARN);
+		}
 	}
 	return (ARCHIVE_OK);
 }

Modified: head/contrib/libarchive/libarchive/archive_read_disk_posix.c
==============================================================================
--- head/contrib/libarchive/libarchive/archive_read_disk_posix.c	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_read_disk_posix.c	Wed Sep 14 21:15:01 2016	(r305819)
@@ -165,7 +165,7 @@ struct filesystem {
 	int		synthetic;
 	int		remote;
 	int		noatime;
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	size_t		name_max;
 #endif
 	long		incr_xfer_size;
@@ -200,7 +200,7 @@ struct tree {
 	DIR			*d;
 #define	INVALID_DIR_HANDLE NULL
 	struct dirent		*de;
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	struct dirent		*dirent;
 	size_t			 dirent_allocated;
 #endif
@@ -1592,7 +1592,7 @@ setup_current_filesystem(struct archive_
 #endif
 		t->current_filesystem->noatime = 0;
 
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	/* Set maximum filename length. */
 #if defined(HAVE_STRUCT_STATFS_F_NAMEMAX)
 	t->current_filesystem->name_max = sfs.f_namemax;
@@ -1615,7 +1615,7 @@ setup_current_filesystem(struct archive_
 	else
 		t->current_filesystem->name_max = nm;
 #endif
-#endif /* HAVE_READDIR_R */
+#endif /* USE_READDIR_R */
 	return (ARCHIVE_OK);
 }
 
@@ -1817,7 +1817,7 @@ setup_current_filesystem(struct archive_
 #endif
 		t->current_filesystem->noatime = 0;
 
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	/* Set maximum filename length. */
 	t->current_filesystem->name_max = sfs.f_namelen;
 #endif
@@ -1901,7 +1901,7 @@ setup_current_filesystem(struct archive_
 #endif
 		t->current_filesystem->noatime = 0;
 
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	/* Set maximum filename length. */
 	t->current_filesystem->name_max = sfs.f_namemax;
 #endif
@@ -1918,7 +1918,7 @@ static int
 setup_current_filesystem(struct archive_read_disk *a)
 {
 	struct tree *t = a->tree;
-#if defined(_PC_NAME_MAX) && defined(HAVE_READDIR_R)
+#if defined(_PC_NAME_MAX) && defined(USE_READDIR_R)
 	long nm;
 #endif
 	t->current_filesystem->synthetic = -1;/* Not supported */
@@ -1930,7 +1930,7 @@ setup_current_filesystem(struct archive_
 	t->current_filesystem->min_xfer_size = -1;
 	t->current_filesystem->incr_xfer_size = -1;
 
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	/* Set maximum filename length. */
 #  if defined(_PC_NAME_MAX)
 	if (tree_current_is_symblic_link_target(t)) {
@@ -1958,7 +1958,7 @@ setup_current_filesystem(struct archive_
 	else
 		t->current_filesystem->name_max = nm;
 #  endif /* _PC_NAME_MAX */
-#endif /* HAVE_READDIR_R */
+#endif /* USE_READDIR_R */
 	return (ARCHIVE_OK);
 }
 
@@ -2366,7 +2366,7 @@ tree_dir_next_posix(struct tree *t)
 	size_t namelen;
 
 	if (t->d == NULL) {
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 		size_t dirent_size;
 #endif
 
@@ -2387,7 +2387,7 @@ tree_dir_next_posix(struct tree *t)
 			t->visit_type = r != 0 ? r : TREE_ERROR_DIR;
 			return (t->visit_type);
 		}
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 		dirent_size = offsetof(struct dirent, d_name) +
 		  t->filesystem_table[t->current->filesystem_id].name_max + 1;
 		if (t->dirent == NULL || t->dirent_allocated < dirent_size) {
@@ -2404,11 +2404,11 @@ tree_dir_next_posix(struct tree *t)
 			}
 			t->dirent_allocated = dirent_size;
 		}
-#endif /* HAVE_READDIR_R */
+#endif /* USE_READDIR_R */
 	}
 	for (;;) {
 		errno = 0;
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 		r = readdir_r(t->d, t->dirent, &t->de);
 #ifdef _AIX
 		/* Note: According to the man page, return value 9 indicates
@@ -2660,7 +2660,7 @@ tree_free(struct tree *t)
 	if (t == NULL)
 		return;
 	archive_string_free(&t->path);
-#if defined(HAVE_READDIR_R)
+#if defined(USE_READDIR_R)
 	free(t->dirent);
 #endif
 	free(t->sparse_list);

Modified: head/contrib/libarchive/libarchive/archive_read_support_format_tar.c
==============================================================================
--- head/contrib/libarchive/libarchive/archive_read_support_format_tar.c	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_read_support_format_tar.c	Wed Sep 14 21:15:01 2016	(r305819)
@@ -136,6 +136,7 @@ struct tar {
 	int64_t			 entry_padding;
 	int64_t 		 entry_bytes_unconsumed;
 	int64_t			 realsize;
+	int			 sparse_allowed;
 	struct sparse_block	*sparse_list;
 	struct sparse_block	*sparse_last;
 	int64_t			 sparse_offset;
@@ -1271,6 +1272,14 @@ header_common(struct archive_read *a, st
 		 * sparse information in the extended area.
 		 */
 		/* FALLTHROUGH */
+	case '0':
+		/*
+		 * Enable sparse file "read" support only for regular
+		 * files and explicit GNU sparse files.  However, we
+		 * don't allow non-standard file types to be sparse.
+		 */
+		tar->sparse_allowed = 1;
+		/* FALLTHROUGH */
 	default: /* Regular file  and non-standard types */
 		/*
 		 * Per POSIX: non-recognized types should always be
@@ -1730,6 +1739,14 @@ pax_attribute(struct archive_read *a, st
 #endif
 	switch (key[0]) {
 	case 'G':
+		/* Reject GNU.sparse.* headers on non-regular files. */
+		if (strncmp(key, "GNU.sparse", 10) == 0 &&
+		    !tar->sparse_allowed) {
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+			    "Non-regular file cannot be sparse");
+			return (ARCHIVE_FATAL);
+		}
+
 		/* GNU "0.0" sparse pax format. */
 		if (strcmp(key, "GNU.sparse.numblocks") == 0) {
 			tar->sparse_offset = -1;

Modified: head/contrib/libarchive/libarchive/archive_write_disk_acl.c
==============================================================================
--- head/contrib/libarchive/libarchive/archive_write_disk_acl.c	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_write_disk_acl.c	Wed Sep 14 21:15:01 2016	(r305819)
@@ -153,9 +153,19 @@ set_acl(struct archive *a, int fd, const
 	if (entries == 0)
 		return (ARCHIVE_OK);
 	acl = acl_init(entries);
+	if (acl == (acl_t)NULL) {
+		archive_set_error(a, errno,
+		    "Failed to initialize ACL working storage");
+		return (ARCHIVE_FAILED);
+	}
 	while (archive_acl_next(a, abstract_acl, ae_requested_type, &ae_type,
 		   &ae_permset, &ae_tag, &ae_id, &ae_name) == ARCHIVE_OK) {
-		acl_create_entry(&acl, &acl_entry);
+		if (acl_create_entry(&acl, &acl_entry) != 0) {
+			archive_set_error(a, errno,
+			    "Failed to create a new ACL entry");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
+		}
 
 		switch (ae_tag) {
 		case ARCHIVE_ENTRY_ACL_USER:
@@ -186,53 +196,96 @@ set_acl(struct archive *a, int fd, const
 			break;
 #endif
 		default:
-			/* XXX */
-			break;
+			archive_set_error(a, ARCHIVE_ERRNO_MISC,
+			    "Unknown ACL tag");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
 		}
 
 #ifdef ACL_TYPE_NFS4
+		r = 0;
 		switch (ae_type) {
 		case ARCHIVE_ENTRY_ACL_TYPE_ALLOW:
-			acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_ALLOW);
+			r = acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_ALLOW);
 			break;
 		case ARCHIVE_ENTRY_ACL_TYPE_DENY:
-			acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_DENY);
+			r = acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_DENY);
 			break;
 		case ARCHIVE_ENTRY_ACL_TYPE_AUDIT:
-			acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_AUDIT);
+			r = acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_AUDIT);
 			break;
 		case ARCHIVE_ENTRY_ACL_TYPE_ALARM:
-			acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_ALARM);
+			r = acl_set_entry_type_np(acl_entry, ACL_ENTRY_TYPE_ALARM);
 			break;
 		case ARCHIVE_ENTRY_ACL_TYPE_ACCESS:
 		case ARCHIVE_ENTRY_ACL_TYPE_DEFAULT:
 			// These don't translate directly into the system ACL.
 			break;
 		default:
-			// XXX error handling here.
-			break;
+			archive_set_error(a, ARCHIVE_ERRNO_MISC,
+			    "Unknown ACL entry type");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
+		}
+		if (r != 0) {
+			archive_set_error(a, errno,
+			    "Failed to set ACL entry type");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
 		}
 #endif
 
-		acl_get_permset(acl_entry, &acl_permset);
-		acl_clear_perms(acl_permset);
+		if (acl_get_permset(acl_entry, &acl_permset) != 0) {
+			archive_set_error(a, errno,
+			    "Failed to get ACL permission set");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
+		}
+		if (acl_clear_perms(acl_permset) != 0) {
+			archive_set_error(a, errno,
+			    "Failed to clear ACL permissions");
+			ret = ARCHIVE_FAILED;
+			goto exit_free;
+		}
 
 		for (i = 0; i < (int)(sizeof(acl_perm_map) / sizeof(acl_perm_map[0])); ++i) {
 			if (ae_permset & acl_perm_map[i].archive_perm)
-				acl_add_perm(acl_permset,
-					     acl_perm_map[i].platform_perm);
+				if (acl_add_perm(acl_permset,
+				    acl_perm_map[i].platform_perm) != 0) {
+					archive_set_error(a, errno,
+					    "Failed to add ACL permission");
+					ret = ARCHIVE_FAILED;
+					goto exit_free;
+				}
 		}
 
 #ifdef ACL_TYPE_NFS4
-		// XXX acl_get_flagset_np on FreeBSD returns EINVAL for
-		// non-NFSv4 ACLs
-		r = acl_get_flagset_np(acl_entry, &acl_flagset);
-		if (r == 0) {
-			acl_clear_flags_np(acl_flagset);
+		if (acl_type == ACL_TYPE_NFS4) {
+			/*
+			 * acl_get_flagset_np() fails with non-NFSv4 ACLs
+			 */
+			if (acl_get_flagset_np(acl_entry, &acl_flagset) != 0) {
+				archive_set_error(a, errno,
+				    "Failed to get flagset from an NFSv4 ACL entry");
+				ret = ARCHIVE_FAILED;
+				goto exit_free;
+			}
+			if (acl_clear_flags_np(acl_flagset) != 0) {
+				archive_set_error(a, errno,
+				    "Failed to clear flags from an NFSv4 ACL flagset");
+				ret = ARCHIVE_FAILED;
+				goto exit_free;
+			}
 			for (i = 0; i < (int)(sizeof(acl_inherit_map) / sizeof(acl_inherit_map[0])); ++i) {
-				if (ae_permset & acl_inherit_map[i].archive_inherit)
-					acl_add_flag_np(acl_flagset,
-							acl_inherit_map[i].platform_inherit);
+				if (ae_permset & acl_inherit_map[i].archive_inherit) {
+					if (acl_add_flag_np(acl_flagset,
+							acl_inherit_map[i].platform_inherit) != 0) {
+						archive_set_error(a, errno,
+						    "Failed to add flag to NFSv4 ACL flagset");
+						ret = ARCHIVE_FAILED;
+						goto exit_free;
+					}
+				}
 			}
 		}
 #endif
@@ -262,6 +315,7 @@ set_acl(struct archive *a, int fd, const
 		ret = ARCHIVE_WARN;
 	}
 #endif
+exit_free:
 	acl_free(acl);
 	return (ret);
 }

Modified: head/contrib/libarchive/libarchive/archive_write_disk_posix.c
==============================================================================
--- head/contrib/libarchive/libarchive/archive_write_disk_posix.c	Wed Sep 14 21:01:02 2016	(r305818)
+++ head/contrib/libarchive/libarchive/archive_write_disk_posix.c	Wed Sep 14 21:15:01 2016	(r305819)
@@ -140,7 +140,17 @@ __FBSDID("$FreeBSD$");
 #define O_BINARY 0
 #endif
 #ifndef O_CLOEXEC
-#define O_CLOEXEC	0
+#define O_CLOEXEC 0
+#endif
+
+/* Ignore non-int O_NOFOLLOW constant. */
+/* gnulib's fcntl.h does this on AIX, but it seems practical everywhere */
+#if defined O_NOFOLLOW && !(INT_MIN <= O_NOFOLLOW && O_NOFOLLOW <= INT_MAX)
+#undef O_NOFOLLOW
+#endif
+
+#ifndef O_NOFOLLOW
+#define O_NOFOLLOW 0
 #endif
 
 struct fixup_entry {
@@ -326,12 +336,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 +2026,10 @@ create_filesystem_object(struct archive_
 	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 +2038,27 @@ create_filesystem_object(struct archive_
 #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
@@ -2040,7 +2077,7 @@ create_filesystem_object(struct archive_
 			a->deferred = 0;
 		} else if (r == 0 && a->filesize > 0) {
 			a->fd = open(a->name,
-				     O_WRONLY | O_TRUNC | O_BINARY | O_CLOEXEC);
+				     O_WRONLY | O_TRUNC | O_BINARY | O_CLOEXEC | O_NOFOLLOW);
 			__archive_ensure_cloexec_flag(a->fd);
 			if (a->fd < 0)
 				r = errno;
@@ -2351,126 +2388,233 @@ current_fixup(struct archive_write_disk 
 	return (a->current_fixup);
 }
 
-/* TODO: Make this work. */
-/*
- * TODO: The deep-directory support bypasses this; disable deep directory
- * support if we're doing symlink checks.
- */
 /*
  * TODO: Someday, integrate this with the deep dir support; they both
  * scan the path and both can be optimized by comparing against other
  * 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.
-	 */
-	/* 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;
-	}
+	 *
+	 * 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
+	 */
+	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. */
-	}
-	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);
+		/* be sure to always maintain this */
+		tail[0] = c;
+		if (tail[0] != '\0')
+			tail++; /* Advance to the next segment. */
+	}
+	/* 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 +2688,17 @@ cleanup_pathname_win(struct archive_writ
  * 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 +2707,11 @@ cleanup_pathname(struct archive_write_di
 #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 +2738,11 @@ cleanup_pathname(struct archive_write_di
 			} 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;

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609142115.u8ELF1t1019804>