From nobody Wed Mar 06 16:24:28 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Tqd7134c5z5Csfk; Wed, 6 Mar 2024 16:24:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Tqd710kDTz4fRD; Wed, 6 Mar 2024 16:24:29 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1709742269; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=jx2cRIAM/D8oGCHYC/4W1kKU1QlvjOrMBnsOrNXGn7c=; b=My9ek0hNi3MFsAG3aCf83A86C/0W/O3eBYJvk08YpH9XNL6grOuJRjtXdKJ2zCLMVmIbNo 26xjLeOFTlddmFp6O2x3lENKEJpKYKCpaHksex6UIdJ6awNytcDJUpnVyFcMPAkuDDZB53 DEaWynXZsLb01iS9zujyTSgSv+J3DknA/BBUA0CXT8nVKO8lL7U4gnFXlTfIqxA+cvRbLg qy6zXvUXPNILBqneOI7y/QK0En6gg4RQHGo01m31dXBn/YroOtq5booO7UyBxzhDzyfLse 0rpglDMXs81DNkd7JGGCcYJVtpRz4Lvh4PGk8KF/Z7Pdt5AK1Zr3NGcuR4hYGw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1709742269; a=rsa-sha256; cv=none; b=nHE+/s++EgUxVcg/kFCQNwWWb+/si3U+PCpAp9m2eXHKa3clK0D39ktYWs6ac6+y6LvL3e wWiXDTmIS685METKUHJWDOTRWhMnRGBposUEHsLq7pO3Ihf/gKGWjIHj23AyPnldmWFqFn ULVbXBI/mtv1TcGWPn5q1teTyneEFOq1wDo3Mk3YlO2csQYyc7agOGYT6Kskw4U84v/h+O XFkXI0stQvG2jhyG1/Ek7iCQKCi5f9OkhdjpoP9FVtDy3BGEl1fJq5cbgBZmifVZgO4LnA aWwTpltVccimAamNWvPA9YGLoyVq6hmV7N99Z7n5XfRKy+4cwZtnpCXFJbiDgw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1709742269; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=jx2cRIAM/D8oGCHYC/4W1kKU1QlvjOrMBnsOrNXGn7c=; b=mnw2aGasFSYVsmUa07iW5OeNORllUVmPEvHyUlKqUwqaPbrSJsFUAUTj2Cq88jtSweuimV b3ObPjg0V6LWAYD1XC1CvWwlSXgoVCpESWDs0fFlNHz1L4QebePV9o6DGgYjre13Yvwz6Z rw4vgWN9eVkCpFawq2iXY0HeRPNPZtPLqCh6pMA3gqMDCx0I8JT+ZQjnWLlQvqr2MoPdCl flgd7Kf7zPNB3hdcEjDdBKS7Nwv2fmprP/YfMxvRwwX+XuNw/Rm4kUFKIfe25HYfZhk17g YeWnU0iuzSQtKHPzf2opNYRYwr2dq1XHn+NU0NBAXJzuKNGEs+I5BzkmVqGqgw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Tqd710KPXzxh5; Wed, 6 Mar 2024 16:24:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 426GOS1V025016; Wed, 6 Mar 2024 16:24:28 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 426GOSkX025013; Wed, 6 Mar 2024 16:24:28 GMT (envelope-from git) Date: Wed, 6 Mar 2024 16:24:28 GMT Message-Id: <202403061624.426GOSkX025013@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= Subject: git: 38b3683592d4 - main - tarfs: Fix two input validation issues. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 38b3683592d4c20a74f52a6e8e29368e6fa61858 Auto-Submitted: auto-generated The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=38b3683592d4c20a74f52a6e8e29368e6fa61858 commit 38b3683592d4c20a74f52a6e8e29368e6fa61858 Author: Dag-Erling Smørgrav AuthorDate: 2024-03-06 16:13:42 +0000 Commit: Dag-Erling Smørgrav 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 +#include +#include +#include +#include +#include +#include +#include + +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); +}