Skip to content

Commit

Permalink
Select between qubes.Filecopy and qubes.UnsafeFileCopy
Browse files Browse the repository at this point in the history 
The correct service is chosen based on the characters in the filenames
and symlink targets.  The filesystem tree checker outputs the correct
number of bytes for use in progress indication.
  • Loading branch information
@DemiMarie
DemiMarie committed May 27, 2024
1 parent 5d3a31d commit 62f97c1
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 127 deletions.
3 changes: 2 additions & 1 deletion qubes-rpc/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ install:
install -t $(DESTDIR)$(QUBESLIBDIR) \
prepare-suspend resize-rootfs \
qfile-agent qopen-in-vm qrun-in-vm qubes-sync-clock \
tar2qfile vm-file-editor xdg-icon qvm-template-repo-query
tar2qfile vm-file-editor xdg-icon qvm-template-repo-query \
qubes-fs-tree-check
# Install qfile-unpacker as SUID, because it will fail to receive
# files from other vm.
install -t $(DESTDIR)$(QUBESLIBDIR) -m 4755 qfile-unpacker
Expand Down
166 changes: 73 additions & 93 deletions qubes-rpc/qubes-fs-tree-check.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
#include <qubes/pure.h>
#include <libqubes-rpc-filecopy.h>
static bool
simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags);
simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags,
unsigned long long *size);

// This code operates on completely untrusted input.
// Therefore, it is critical to escape everything before displaying it.
Expand Down Expand Up @@ -74,103 +75,83 @@ bool machine = false;
bool ignore_symlinks = false;

static bool
process_dirent(int d_type, const char *d_name, int fd, int flags, const char *name,
bool ignore_symlinks, const char *escaped)
process_dirent(const char *d_name, int fd, int flags, const char *name,
bool ignore_symlinks, const char *escaped, unsigned long long *size)
{
bool bad = false;
if ((flags & COPY_ALLOW_UNSAFE_CHARACTERS) == 0 &&
!qubes_pure_string_safe_for_display(d_name, 0))
return true;
switch (d_type) {
case DT_FIFO:
bad_fifo:
errx(1, "Cannot copy FIFO %s", escaped);
case DT_CHR:
bad_chr:
errx(1, "Cannot copy character device %s", escaped);
case DT_BLK:
bad_blk:
errx(1, "Cannot copy block device %s", escaped);
case DT_SOCK:
bad_sock:
errx(1, "Cannot copy AF_UNIX socket %s", escaped);
case DT_REG:
return false;
case DT_DIR:
struct stat statbuf;
int res = fstatat(fd, d_name, &statbuf, AT_SYMLINK_NOFOLLOW);
if (res < 0)
err(1, "fstatat(%s)", escaped);
switch (statbuf.st_mode & S_IFMT) {
case S_IFLNK:
if (ignore_symlinks)
return false; // do not check path
if ((flags & COPY_ALLOW_SYMLINKS) == 0)
errx(1, "Cannot copy symbolic link %s", escaped);
break;
case S_IFDIR:
if ((flags & COPY_ALLOW_DIRECTORIES) == 0)
goto bad_dir;
errx(1, "Cannot copy directory %s", escaped);
break;
case DT_LNK:
if ((flags & COPY_ALLOW_SYMLINKS) == 0)
goto bad_symlink;
case S_IFREG:
break;
case DT_UNKNOWN:
default:;
struct stat statbuf;
int res = fstatat(fd, d_name, &statbuf, AT_SYMLINK_NOFOLLOW);
if (res < 0)
err(1, "fstatat(%s)", escaped);
switch (statbuf.st_mode & S_IFMT) {
case S_IFDIR:
if ((flags & COPY_ALLOW_DIRECTORIES) == 0)
goto bad_dir;
d_type = DT_DIR;
break;
case S_IFLNK:
if ((flags & COPY_ALLOW_SYMLINKS) == 0)
goto bad_symlink;
d_type = DT_LNK;
break;
case S_IFIFO:
goto bad_fifo;
case S_IFBLK:
goto bad_blk;
case S_IFCHR:
goto bad_chr;
case S_IFSOCK:
goto bad_sock;
case S_IFREG:
return false;
default:
errx(1, "File %s of unknown type found, is this a bug or corrupt filesystem?", escaped);
}
default:
// In machine readable mode, ignore these
bad = !machine && (flags & COPY_ALLOW_UNSAFE_CHARACTERS) == 0 &&
!qubes_pure_string_safe_for_display(d_name, 0);
if (bad)
warnx("%s is not safe for display", escaped);
return bad;
}
if (d_type == DT_DIR) {
if ((flags & COPY_ALLOW_UNSAFE_CHARACTERS) == 0 &&
!qubes_pure_string_safe_for_display(d_name, 0)) {
if (!machine)
warnx("%s is not safe for display", escaped);
bad = true;
}
if (S_ISDIR(statbuf.st_mode)) {
int sub_file = openat(fd, d_name, O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC | O_RDONLY);
if (sub_file < 0)
err(1, "open(%s)", escaped);
return simple_fs_walk(sub_file, ignore_symlinks, name, flags);
}
if (ignore_symlinks)
return false;
size_t size = 769;
char *buf = NULL;
for (;;) {
buf = realloc(buf, size);
if (buf == NULL)
err(1, "malloc(%zu)", size);
ssize_t res = readlinkat(fd, d_name, buf, size);
if (res < 0)
err(1, "readlink(%s)", escaped);
if ((size_t)res < size) {
buf[res] = '\0'; // add NUL terminator
break;
}
if (size >= SSIZE_MAX / 2)
err(1, "refusing to allocate %zu bytes", size);
size *= 2;
return simple_fs_walk(sub_file, ignore_symlinks, name, flags, size);
} else {
// __builtin_add_overflow uses infinite signed precision,
// so a negative number would not cause overflow.
if (statbuf.st_size < 0)
errx(1, "Negative size?");
// If this overflows, then we definitely want to error out instead of
// filling up the receiver's disk.
if (__builtin_add_overflow(*size, statbuf.st_size, size))
errx(1, "Refusing to copy over 2**64 bytes");
if (S_ISREG(statbuf.st_mode) || ignore_symlinks)
return bad;
}
if (statbuf.st_size > SSIZE_MAX - 1)
errx(1, "symbolic link too large for readlink()");
size_t link_size = (size_t)(statbuf.st_size + 1);
char *buf = malloc(link_size);
if (buf == NULL)
err(1, "malloc(%zu)", link_size);
ssize_t link_res = readlinkat(fd, d_name, buf, link_size);
if (link_res < 0)
err(1, "readlink(%s)", escaped);
if (link_res != statbuf.st_size)
errx(1, "readlink(%s) returned wrong size", escaped);
buf[link_res] = '\0'; // add NUL terminator
bad = !qubes_pure_string_safe_for_display(buf, 0);
// charset checks done already, do not repeat them
if (qubes_pure_validate_symbolic_link_v2((const uint8_t *)name,
(const uint8_t *)buf,
QUBES_PURE_ALLOW_UNSAFE_CHARACTERS) < 0)
errx(1, "Refusing to copy unsafe symbolic link %s", escaped);
free(buf);
return bad;
bad_dir:
errx(1, "Cannot copy directory %s", escaped);
bad_symlink:
errx(1, "Cannot copy symbolic link %s", escaped);
}

static bool
simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags)
simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags,
unsigned long long *size)
{
DIR *dir = fdopendir(fd);
bool bad = false;
Expand All @@ -190,19 +171,12 @@ simple_fs_walk(int fd, bool ignore_symlinks, const char *name, int flags)
if (name != NULL && asprintf(&full_name, "%s/%s", name, d->d_name) < 1)
err(1, "asprintf() failed (out of memory?)");
char *escaped = simple_strvis(full_name);
if (!qubes_pure_string_safe_for_display(d->d_name, 0)) {
if (!machine)
warnx("%s is not safe for display", escaped);
free(escaped);
if (process_dirent(d->d_name, fd, flags, full_name, ignore_symlinks,
escaped, size))
bad = true;
break;
}
bad = process_dirent(d->d_type, d->d_name, fd, flags, full_name, ignore_symlinks, escaped);
if (full_name != d->d_name)
free(full_name);
free(escaped);
if (bad)
break;
}
closedir(dir);
return bad;
Expand Down Expand Up @@ -351,6 +325,7 @@ int main(int argc, char **argv)
errx(1, "argument %d begins with - but first argument is not --", j);
}
bool bad = false;
unsigned long long size = 0;
for (int i = optind; i < argc; ++i) {
size_t len = strlen(argv[i]) + 1;
if (len < 4 && memcmp("..", argv[i], len - 1) == 0) {
Expand All @@ -369,7 +344,7 @@ int main(int argc, char **argv)
int dir_fd = open(argv[i], O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOCTTY);
if (dir_fd < 0)
err(1, "open(%s)", escaped);
if (simple_fs_walk(dir_fd, ignore_symlinks, argv[i], flags))
if (simple_fs_walk(dir_fd, ignore_symlinks, argv[i], flags, &size))
bad = true;
} else {
char *dup1 = malloc(len), *dup2 = (dup1 == NULL ? NULL : malloc(len));
Expand All @@ -380,12 +355,17 @@ int main(int argc, char **argv)
int dir_fd = open(dname, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOCTTY);
if (dir_fd < 0)
err(1, "open(%s)", escaped);
if (process_dirent(DT_UNKNOWN, bname, dir_fd, flags, argv[i], false, escaped))
if (process_dirent(bname, dir_fd, flags, argv[i], ignore_symlinks,
escaped, &size))
bad = true;
free(dup2);
free(dup1);
}
free(escaped);
}
if (machine)
printf("%llu\n", size);
if (fflush(NULL) || ferror(stdout) || ferror(stderr))
errx(1, "I/O error on standard stream");
return bad ? 2 : 0;
}
44 changes: 11 additions & 33 deletions qubes-rpc/qvm-copy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

set -e -o pipefail

unset PROGRESS_TYPE OPERATION_TYPE TARGET_TYPE MIN_ARGS FILECOPY_TOTAL_SIZE
unset PROGRESS_TYPE OPERATION_TYPE TARGET_TYPE MIN_ARGS FILECOPY_TOTAL_SIZE service

case ${0##*/} in
(qvm-move) OPERATION_TYPE=move TARGET_TYPE=default MIN_ARGS=1;;
Expand Down Expand Up @@ -74,40 +74,18 @@ else
VM="@default"
fi

for path; do
case $path in
(.|..) printf 'Refusing to copy "." or "..".
Try copying from the parent directory.\n' >&2; exit 1;;
esac
done

if [ "$PROGRESS_TYPE" = console ] ; then
find_paths=( )
for path; do
case "$path" in
(-*) find_paths+=( ./"$path" ) ;;
(*) find_paths+=( "$path" ) ;;
esac
done

FILECOPY_TOTAL_SIZE=$(
find "${find_paths[@]}" -type f -print0 2>/dev/null |
du --files0-from - -c --apparent-size -k |
tail -n 1 |
cut -f 1 |
grep -xE '[0-9]+'
) || FILECOPY_TOTAL_SIZE=0
export FILECOPY_TOTAL_SIZE
if FILECOPY_TOTAL_SIZE=$(/usr/lib/qubes/qubes-fs-tree-check \
--allow-symlinks --allow-directories --machine -- "$@"); then
service=qubes.Filecopy
else
status=$?
if [[ "$status" -ne 2 ]]; then exit "$status"; fi
service=qubes.UnsafeFileCopy
fi
if [[ "$PROGRESS_TYPE" = 'console' ]]; then export FILECOPY_TOTAL_SIZE; fi

for path; do
if [ ! -e "$path" ]; then
echo "File or directory not found: $path"
exit 1
fi
done

/usr/lib/qubes/qrexec-client-vm --filter-escape-chars-stderr -- "$VM" qubes.Filecopy /usr/lib/qubes/qfile-agent "$@"
/usr/lib/qubes/qrexec-client-vm --filter-escape-chars-stderr -- "$VM" \
"$service" /usr/lib/qubes/qfile-agent "$@"

if [ "$OPERATION_TYPE" = "move" ] ; then
rm -rf -- "$@"
Expand Down

0 comments on commit 62f97c1

Please sign in to comment.