Skip to content

Commit

Permalink
Do not use path containing "/" in linkat() or stat()
Browse files Browse the repository at this point in the history
This defeats the protections provided by opendir_safe().  Instead, use
the already-open file descriptor for the file's containing directory.

It is unclear whether this can be used to escape a bind mount, as
linkat() might fail with -EXDEV in this case.  However, it is definitely
wrong and needs to be fixed.

A search for "untrusted_name" in qrexec-lib/unpack.c finds that these
are the only places where an untrusted path that may contain "/" is
used as a path in a system call argument.  In all other cases, either
the path is trusted or only paths that are guaranteed to not contain "/"
are used, ensuring that the vulnerability in Qubes Security Bulletin 014
can never be a problem again.

Fixes: ce2df91 ("Initial work on safe open")
  • Loading branch information
DemiMarie committed May 20, 2024
1 parent 5582539 commit edece29
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions qrexec-lib/unpack.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,6 @@ void process_one_file_reg(struct file_header *untrusted_hdr,
fdout = openat(safe_dirfd, last_segment, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, 0000);
if (fdout < 0)
do_exit(errno, untrusted_name);
if (safe_dirfd != AT_FDCWD)
close(safe_dirfd);

/* sizes are signed elsewhere */
if (untrusted_hdr->filelen > LLONG_MAX || (bytes_limit && untrusted_hdr->filelen > bytes_limit))
Expand All @@ -251,10 +249,12 @@ void process_one_file_reg(struct file_header *untrusted_hdr,
char fd_str[11];
if ((unsigned)snprintf(fd_str, sizeof(fd_str), "%d", fdout) >= sizeof(fd_str))
abort();
if (linkat(procdir_fd, fd_str, AT_FDCWD, untrusted_name, AT_SYMLINK_FOLLOW) < 0)
if (linkat(procdir_fd, fd_str, safe_dirfd, last_segment, AT_SYMLINK_FOLLOW) < 0)
do_exit(errno, untrusted_name);
}
fix_times_and_perms(fdout, untrusted_hdr, NULL, untrusted_name);
if (safe_dirfd != AT_FDCWD)
close(safe_dirfd);
close(fdout);
free(path_dup);
}
Expand All @@ -281,7 +281,7 @@ void process_one_file_dir(struct file_header *untrusted_hdr,
}
if (errno != EEXIST)
do_exit(errno, untrusted_name);
if (stat(untrusted_name,&buf) < 0)
if (fstatat(safe_dirfd, last_segment, &buf, AT_SYMLINK_NOFOLLOW) < 0)
do_exit(errno, untrusted_name);
total_bytes += buf.st_size;
/* size accumulated after the fact, so don't check limit here */
Expand Down

0 comments on commit edece29

Please sign in to comment.