Don't allow trailing slashes in paths being unpacked

They would result in a misleading ENOENT error due to passing an empty
path to openat().  Instead, reject the path with EILSEQ to indicate a
malformed pathname.

(cherry picked from commit 044c4d4)

Author: DemiMarie

qrexec-lib/pure.h

@@ -253,6 +253,7 @@ enum QubeNameValidationError {
 QUBES_PURE_PUBLIC enum QubeNameValidationError
 qubes_pure_is_valid_qube_name(const struct QubesSlice untrusted_str);
 
+/// Flags for pathname validation functions.
 enum QubesFilenameValidationFlags {
     /// Disable Unicode charset restrictions and UTF-8 validity checks.
     QUBES_PURE_ALLOW_UNSAFE_CHARACTERS = (1 << 0),
@@ -264,6 +265,8 @@ enum QubesFilenameValidationFlags {
     QUBES_PURE_ALLOW_UNSAFE_SYMLINKS = (1 << 2),
     /// Allow all paths to be non-canonical, including symlinks.
     QUBES_PURE_ALLOW_NON_CANONICAL_PATHS = (1 << 3),
+    /// Allow trailing slash.
+    QUBES_PURE_ALLOW_TRAILING_SLASH = (1 << 4),
 };
 
 #ifdef __cplusplus

qrexec-lib/unicode.c

@@ -167,7 +167,8 @@ static ssize_t validate_path(const uint8_t *const untrusted_name,
         return allow_non_canonical ? 0 : -ENOLINK; // empty path
     if (untrusted_name[0] == '/')
         return -ENOLINK; // absolute path
-    for (size_t i = 0; untrusted_name[i]; i++) {
+    size_t i;
+    for (i = 0; untrusted_name[i]; i++) {
         if (i == 0 || untrusted_name[i - 1] == '/') {
             // Start of a path component
             switch (untrusted_name[i]) {
@@ -216,6 +217,14 @@ static ssize_t validate_path(const uint8_t *const untrusted_name,
             }
         }
     }
+    if (i < 1 || untrusted_name[i]) {
+        // ideally this would be COMPILETIME_UNREACHABLE but GCC can't prove this
+        assert(0);
+        return -EILSEQ;
+    }
+    if ((flags & QUBES_PURE_ALLOW_TRAILING_SLASH) == 0 &&
+            untrusted_name[i - 1] == '/')
+        return -EILSEQ;
     return non_dotdot_components;
 }
 
@@ -224,6 +233,7 @@ static bool flag_check(const uint32_t flags)
     int const allowed = (QUBES_PURE_ALLOW_UNSAFE_CHARACTERS |
                          QUBES_PURE_ALLOW_NON_CANONICAL_SYMLINKS |
                          QUBES_PURE_ALLOW_NON_CANONICAL_PATHS |
+                         QUBES_PURE_ALLOW_TRAILING_SLASH |
                          QUBES_PURE_ALLOW_UNSAFE_SYMLINKS);
     return (flags & ~(__typeof__(flags))allowed) == 0;
 }
@@ -243,7 +253,8 @@ qubes_pure_validate_file_name_v2(const uint8_t *const untrusted_filename,
 QUBES_PURE_PUBLIC bool
 qubes_pure_validate_file_name(const uint8_t *const untrusted_filename)
 {
-    return qubes_pure_validate_file_name_v2(untrusted_filename, 0) == 0;
+    return qubes_pure_validate_file_name_v2(untrusted_filename,
+                                            QUBES_PURE_ALLOW_TRAILING_SLASH) == 0;
 }
 
 QUBES_PURE_PUBLIC int
@@ -271,16 +282,19 @@ qubes_pure_validate_symbolic_link_v2(const uint8_t *untrusted_name,
     // (which resolves to "c").  Similarly and "a/b/c" can point to "../d"
     // (which resolves to "a/d") but not "../../d" (which resolves to "d").
     // This ensures that ~/QubesIncoming/QUBENAME/a/b cannot point outside
-    // of ~/QubesIncoming/QUBENAME/a.
-    ssize_t res = validate_path(untrusted_target, (size_t)(depth - 2), flags);
+    // of ~/QubesIncoming/QUBENAME/a.  Always allow trailing slash in the
+    // symbolic link target, whether or not they are allowed in the path.
+    ssize_t res = validate_path(untrusted_target, (size_t)(depth - 2),
+                                flags | QUBES_PURE_ALLOW_TRAILING_SLASH);
     return res < 0 ? res : 0;
 }
 
 QUBES_PURE_PUBLIC bool
 qubes_pure_validate_symbolic_link(const uint8_t *untrusted_name,
                                   const uint8_t *untrusted_target)
 {
-    return qubes_pure_validate_symbolic_link_v2(untrusted_name, untrusted_target, 0) == 0;
+    return qubes_pure_validate_symbolic_link_v2(untrusted_name, untrusted_target,
+                                                QUBES_PURE_ALLOW_TRAILING_SLASH) == 0;
 }
 
 QUBES_PURE_PUBLIC bool

qrexec-lib/validator-test.c

@@ -182,6 +182,14 @@ int main(int argc, char **argv)
         TEST("a/b", "b//c", QUBES_PURE_ALLOW_NON_CANONICAL_PATHS, true),
         // ...and non-canonical paths.
         TEST("a//b", "b//c", QUBES_PURE_ALLOW_NON_CANONICAL_PATHS, true),
+        // Symlinks may end in "/"...
+        TEST("a/b/c", "a/", QUBES_PURE_ALLOW_TRAILING_SLASH, true),
+        // ...even without QUBES_PURE_ALLOW_TRAILING_SLASH.
+        TEST("a/b/c", "a/", 0, true),
+        // but the path cannot...
+        TEST("a/b/c/", "a/", 0, false),
+        // ...unless QUBES_PURE_ALLOW_TRAILING_SLASH is passed.
+        TEST("a/b/c/", "a/", QUBES_PURE_ALLOW_TRAILING_SLASH, true),
     };
     int failed = 0;
 #define SYMLINK_TEST(a) symlink_test(a, sizeof(a)/sizeof(a[0]))