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.

diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
index 39ee3b67a4a898a2dd72a8713814f3d934c8b9ad..8f0421e788a8821af760230b34e0898d214857ae 100644 (file)
--- 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') {
                                /*