git: 38b3683592d4 - main - tarfs: Fix two input validation issues.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 06 Mar 2024 16:24:28 UTC
The branch main has been updated by des:
URL: https://cgit.FreeBSD.org/src/commit/?id=38b3683592d4c20a74f52a6e8e29368e6fa61858
commit 38b3683592d4c20a74f52a6e8e29368e6fa61858
Author: Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-03-06 16:13:42 +0000
Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-03-06 16:13:42 +0000
tarfs: Fix two input validation issues.
* Reject hard or soft links with an empty target path. Currently, a
debugging kernel will hit an assertion in tarfs_lookup_path() while
a non-debugging kernel will happily create a link to the mount root.
* Use a temporary variable to store the result of the link target path,
and copy it to tnp->other only once we have found it to be valid.
Otherwise we error out after creating a reference to the target but
before incrementing the target's reference count, which results in a
use-after-free situation in the cleanup code.
* Correctly return ENOENT from tarfs_lookup_path() if the requested
path was not found and create_dirs is false. Luckily, existing
callers did not rely solely on the return value.
MFC after: 3 days
PR: 277360
Sponsored by: Juniper Networks, Inc.
Sponsored by: Klara, Inc.
Reviewed by: sjg
Differential Revision: https://reviews.freebsd.org/D44161
---
sys/fs/tarfs/tarfs_vfsops.c | 37 +++++++----
tests/sys/fs/tarfs/Makefile | 2 +-
tests/sys/fs/tarfs/tarfs_test.sh | 68 ++++++++++++++++++++-
tests/sys/fs/tarfs/tarsum.c | 128 +++++++++++++++++++++++++++++++++++++++
4 files changed, 221 insertions(+), 14 deletions(-)
diff --git a/sys/fs/tarfs/tarfs_vfsops.c b/sys/fs/tarfs/tarfs_vfsops.c
index 4489b41699ec..e45503373793 100644
--- a/sys/fs/tarfs/tarfs_vfsops.c
+++ b/sys/fs/tarfs/tarfs_vfsops.c
@@ -2,7 +2,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2013 Juniper Networks, Inc.
- * Copyright (c) 2022-2023 Klara, Inc.
+ * Copyright (c) 2022-2024 Klara, Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -379,8 +379,10 @@ tarfs_lookup_path(struct tarfs_mount *tmp, char *name, size_t namelen,
tnp = tarfs_lookup_node(parent, NULL, &cn);
if (tnp == NULL) {
do_lookup = false;
- if (!create_dirs)
+ if (!create_dirs) {
+ error = ENOENT;
break;
+ }
}
}
name += cn.cn_namelen;
@@ -451,7 +453,7 @@ tarfs_alloc_one(struct tarfs_mount *tmp, off_t *blknump)
int64_t num;
int endmarker = 0;
char *namep, *sep;
- struct tarfs_node *parent, *tnp;
+ struct tarfs_node *parent, *tnp, *other;
size_t namelen = 0, linklen = 0, realsize = 0, sz;
ssize_t res;
dev_t rdev;
@@ -732,28 +734,39 @@ again:
link = hdrp->linkname;
linklen = strnlen(link, sizeof(hdrp->linkname));
}
- error = tarfs_alloc_node(tmp, namep, sep - namep, VREG,
- 0, 0, 0, 0, 0, 0, 0, NULL, 0, parent, &tnp);
- if (error != 0) {
+ if (linklen == 0) {
+ TARFS_DPF(ALLOC, "%s: %.*s: link without target\n",
+ __func__, (int)namelen, name);
+ error = EINVAL;
goto bad;
}
error = tarfs_lookup_path(tmp, link, linklen, NULL,
- NULL, NULL, &tnp->other, false);
- if (tnp->other == NULL ||
- tnp->other->type != VREG ||
- tnp->other->other != NULL) {
- TARFS_DPF(ALLOC, "%s: %.*s: dead hard link to %.*s\n",
+ NULL, NULL, &other, false);
+ if (error != 0 || other == NULL ||
+ other->type != VREG || other->other != NULL) {
+ TARFS_DPF(ALLOC, "%s: %.*s: invalid link to %.*s\n",
__func__, (int)namelen, name, (int)linklen, link);
error = EINVAL;
goto bad;
}
- tnp->other->nlink++;
+ error = tarfs_alloc_node(tmp, namep, sep - namep, VREG,
+ 0, 0, 0, 0, 0, 0, 0, NULL, 0, parent, &tnp);
+ if (error == 0) {
+ tnp->other = other;
+ tnp->other->nlink++;
+ }
break;
case TAR_TYPE_SYMLINK:
if (link == NULL) {
link = hdrp->linkname;
linklen = strnlen(link, sizeof(hdrp->linkname));
}
+ if (linklen == 0) {
+ TARFS_DPF(ALLOC, "%s: %.*s: link without target\n",
+ __func__, (int)namelen, name);
+ error = EINVAL;
+ goto bad;
+ }
error = tarfs_alloc_node(tmp, namep, sep - namep, VLNK,
0, linklen, mtime, uid, gid, mode, flags, link, 0,
parent, &tnp);
diff --git a/tests/sys/fs/tarfs/Makefile b/tests/sys/fs/tarfs/Makefile
index b16c6544d33f..72355a08a158 100644
--- a/tests/sys/fs/tarfs/Makefile
+++ b/tests/sys/fs/tarfs/Makefile
@@ -3,7 +3,7 @@ PACKAGE= tests
TESTSDIR= ${TESTSBASE}/sys/fs/tarfs
BINDIR= ${TESTSDIR}
-PROGS+= mktar
+PROGS+= mktar tarsum
ATF_TESTS_SH+= tarfs_test
diff --git a/tests/sys/fs/tarfs/tarfs_test.sh b/tests/sys/fs/tarfs/tarfs_test.sh
index 15354aac501a..6f45062c18d9 100644
--- a/tests/sys/fs/tarfs/tarfs_test.sh
+++ b/tests/sys/fs/tarfs/tarfs_test.sh
@@ -2,7 +2,7 @@
#-
# SPDX-License-Identifier: BSD-2-Clause
#
-# Copyright (c) 2023 Klara, Inc.
+# Copyright (c) 2023-2024 Klara, Inc.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions
@@ -43,6 +43,10 @@ mktar() {
"$(atf_get_srcdir)"/mktar ${TARFS_USE_GNU_TAR+-g} "$@"
}
+tarsum() {
+ "$(atf_get_srcdir)"/tarsum
+}
+
atf_test_case tarfs_basic cleanup
tarfs_basic_head() {
atf_set "descr" "Basic function test"
@@ -225,6 +229,65 @@ tarfs_notdir_file_gnu_cleanup() {
tarfs_notdir_file_cleanup
}
+atf_test_case tarfs_emptylink cleanup
+tarfs_emptylink_head() {
+ atf_set "descr" "Regression test for PR 277360: empty link target"
+ atf_set "require.user" "root"
+}
+tarfs_emptylink_body() {
+ kldload -n tarfs || atf_skip "This test requires tarfs and could not load it"
+ mkdir "${mnt}"
+ touch z
+ ln -f z hard
+ ln -fs z soft
+ tar -cf - z hard soft | dd bs=512 skip=1 | tr z '\0' | \
+ tarsum >> tarfs_emptylink.tar
+ atf_check -s not-exit:0 -e match:"Invalid" \
+ mount -rt tarfs tarfs_emptylink.tar "${mnt}"
+}
+tarfs_emptylink_cleanup() {
+ umount "${mnt}" || true
+}
+
+atf_test_case tarfs_linktodir cleanup
+tarfs_linktodir_head() {
+ atf_set "descr" "Regression test for PR 277360: link to directory"
+ atf_set "require.user" "root"
+}
+tarfs_linktodir_body() {
+ kldload -n tarfs || atf_skip "This test requires tarfs and could not load it"
+ mkdir "${mnt}"
+ mkdir d
+ tar -cf - d | dd bs=512 count=1 > tarfs_linktodir.tar
+ rmdir d
+ touch d
+ ln -f d link
+ tar -cf - d link | dd bs=512 skip=1 >> tarfs_linktodir.tar
+ atf_check -s not-exit:0 -e match:"Invalid" \
+ mount -rt tarfs tarfs_linktodir.tar "${mnt}"
+}
+tarfs_linktodir_cleanup() {
+ umount "${mnt}" || true
+}
+
+atf_test_case tarfs_linktononexistent cleanup
+tarfs_linktononexistent_head() {
+ atf_set "descr" "Regression test for PR 277360: link to nonexistent target"
+ atf_set "require.user" "root"
+}
+tarfs_linktononexistent_body() {
+ kldload -n tarfs || atf_skip "This test requires tarfs and could not load it"
+ mkdir "${mnt}"
+ touch f
+ ln -f f link
+ tar -cf - f link | dd bs=512 skip=1 >> tarfs_linktononexistent.tar
+ atf_check -s not-exit:0 -e match:"Invalid" \
+ mount -rt tarfs tarfs_linktononexistent.tar "${mnt}"
+}
+tarfs_linktononexistent_cleanup() {
+ umount "${mnt}" || true
+}
+
atf_init_test_cases() {
atf_add_test_case tarfs_basic
atf_add_test_case tarfs_basic_gnu
@@ -236,4 +299,7 @@ atf_init_test_cases() {
atf_add_test_case tarfs_notdir_dotdot_gnu
atf_add_test_case tarfs_notdir_file
atf_add_test_case tarfs_notdir_file_gnu
+ atf_add_test_case tarfs_emptylink
+ atf_add_test_case tarfs_linktodir
+ atf_add_test_case tarfs_linktononexistent
}
diff --git a/tests/sys/fs/tarfs/tarsum.c b/tests/sys/fs/tarfs/tarsum.c
new file mode 100644
index 000000000000..73ead2230a5e
--- /dev/null
+++ b/tests/sys/fs/tarfs/tarsum.c
@@ -0,0 +1,128 @@
+/*-
+ * Copyright (c) 2024 Klara, Inc.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * This program reads a tarball from stdin, recalculates the checksums of
+ * all ustar records within it, and writes the result to stdout.
+ */
+
+#include <err.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static bool opt_v;
+
+static int
+verbose(const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ if (!opt_v)
+ return (0);
+ va_start(ap, fmt);
+ ret = vfprintf(stderr, fmt, ap);
+ va_end(ap);
+ return (ret);
+}
+
+static void
+tarsum(FILE *in, const char *ifn, FILE *out, const char *ofn)
+{
+ union {
+ uint8_t bytes[512];
+ struct {
+ uint8_t prelude[148];
+ char checksum[8];
+ uint8_t interlude[101];
+ char magic[6];
+ char version[2];
+ char postlude[];
+ };
+ } ustar;
+ unsigned long sum;
+ off_t offset = 0;
+ ssize_t ret;
+ size_t i;
+
+ for (;;) {
+ if ((ret = fread(&ustar, 1, sizeof(ustar), in)) < 0)
+ err(1, "%s", ifn);
+ else if (ret == 0)
+ break;
+ else if ((size_t)ret < sizeof(ustar))
+ errx(1, "%s: Short read", ifn);
+ if (strcmp(ustar.magic, "ustar") == 0 &&
+ ustar.version[0] == '0' && ustar.version[1] == '0') {
+ verbose("header found at offset %#lx\n", offset);
+ verbose("current checksum %.*s\n",
+ (int)sizeof(ustar.checksum), ustar.checksum);
+ memset(ustar.checksum, ' ', sizeof(ustar.checksum));
+ for (sum = i = 0; i < sizeof(ustar); i++)
+ sum += ustar.bytes[i];
+ verbose("calculated checksum %#lo\n", sum);
+ sprintf(ustar.checksum, "%#lo", sum);
+ }
+ if ((ret = fwrite(&ustar, 1, sizeof(ustar), out)) < 0)
+ err(1, "%s", ofn);
+ else if ((size_t)ret < sizeof(ustar))
+ errx(1, "%s: Short write", ofn);
+ offset += sizeof(ustar);
+ }
+ verbose("%lu bytes processed\n", offset);
+}
+
+static void
+usage(void)
+{
+ fprintf(stderr, "usage: tarsum [-v] [-o output] [input]\n");
+ exit(1);
+}
+
+int
+main(int argc, char *argv[])
+{
+ const char *ifn, *ofn = NULL;
+ FILE *in, *out;
+ int opt;
+
+ while ((opt = getopt(argc, argv, "o:v")) != -1) {
+ switch (opt) {
+ case 'o':
+ ofn = optarg;
+ break;
+ case 'v':
+ opt_v = true;
+ break;
+ default:
+ usage();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+ if (argc == 0 || strcmp(*argv, "-") == 0) {
+ ifn = "stdin";
+ in = stdin;
+ } else if (argc == 1) {
+ ifn = *argv;
+ if ((in = fopen(ifn, "rb")) == NULL)
+ err(1, "%s", ifn);
+ } else {
+ usage();
+ }
+ if (ofn == NULL || strcmp(ofn, "-") == 0) {
+ ofn = "stdout";
+ out = stdout;
+ } else {
+ if ((out = fopen(ofn, "wb")) == NULL)
+ err(1, "%s", ofn);
+ }
+ tarsum(in, ifn, out, ofn);
+ return (0);
+}