Created attachment 248855 [details] image that causes tarfs_alloc_one() to write outside of a buffer This sequence in tarfs_alloc_one(): size_t len = strtoul(line, &sep, 10); ... if (line + len > exthdr + sz) { TARFS_DPF(ALLOC, "%s: exthdr overflow\n", __func__); error = EINVAL; goto bad; } eol = line + len - 1; *eol = '\0'; Can cause the *eol assignment to write outside of the buffer if the length in the tar file is huge enough that line + len wraps, e.g. 0xffffffffffffffff. I've attached a demo image. It causes a redzone error on my machine with patches D44161 and D44166. # uname -a FreeBSD 15.0-CURRENT FreeBSD 15.0-CURRENT #248 main-n250978-0a0623e9c824-dirty: Fri Mar 1 12:38:44 EST 2024 rtm@zika:/usr/obj/usr/rtm/symbsd/src/riscv.riscv64/sys/RTM riscv # mount -t tarfs tarfs11b.img /mnt REDZONE: Buffer underflow detected. 1 byte corrupted before 0xffffffd007bd9580 (7 bytes allocated). Allocation backtrace: #0 0xffffffc000660432 at redzone_setup+0xac #1 0xffffffc000315a5c at malloc+0xa6 #2 0xffffffc00027220c at tarfs_alloc_mount+0xa96 #3 0xffffffc0002710fc at tarfs_mount+0x584 #4 0xffffffc000404a10 at vfs_domount_first+0x20e #5 0xffffffc000401360 at vfs_domount+0x278 #6 0xffffffc0003ffe58 at vfs_donmount+0x89e #7 0xffffffc0003ff588 at sys_nmount+0x5a #8 0xffffffc0006c15f2 at do_trap_user+0x23a #9 0xffffffc0006b0132 at cpu_exception_handler_user+0x72 Free backtrace: #0 0xffffffc0006606b0 at redzone_check+0x1f8 #1 0xffffffc000316130 at free+0x48 #2 0xffffffc0002729fe at tarfs_alloc_mount+0x1288 #3 0xffffffc0002710fc at tarfs_mount+0x584 #4 0xffffffc000404a10 at vfs_domount_first+0x20e #5 0xffffffc000401360 at vfs_domount+0x278 #6 0xffffffc0003ffe58 at vfs_donmount+0x89e #7 0xffffffc0003ff588 at sys_nmount+0x5a #8 0xffffffc0006c15f2 at do_trap_user+0x23a #9 0xffffffc0006b0132 at cpu_exception_handler_user+0x72 panic: Stopping here. panic() at panic+0x26 redzone_check() at redzone_check+0x382 free() at free+0x48 tarfs_alloc_mount() at tarfs_alloc_mount+0x1288 tarfs_mount() at tarfs_mount+0x584 vfs_domount_first() at vfs_domount_first+0x20e vfs_domount() at vfs_domount+0x278 vfs_donmount() at vfs_donmount+0x89e sys_nmount() at sys_nmount+0x5a do_trap_user() at do_trap_user+0x23a cpu_exception_handler_user() at cpu_exception_handler_user+0x72 --- syscall (378, FreeBSD ELF64, nmount)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c291b7914e1db9469cc820abcb1f5dde7a6f7f28 commit c291b7914e1db9469cc820abcb1f5dde7a6f7f28 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-03-06 16:13:54 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-03-06 16:13:54 +0000 tarfs: Avoid overflow in exthdr calculation. MFC after: 3 days PR: 277420 Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D44202 sys/fs/tarfs/tarfs_vfsops.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=08e799c0cc235b8c5a98921653127a915a985220 commit 08e799c0cc235b8c5a98921653127a915a985220 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-11 12:19:06 +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 (cherry picked from commit 38b3683592d4c20a74f52a6e8e29368e6fa61858) tarfs: Improve validation of numeric fields. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: sjg, allanjude Differential Revision: https://reviews.freebsd.org/D44166 (cherry picked from commit 8427d94ce05682abb6c75e2a27c8c497962c0dc5) tarfs: Avoid overflow in exthdr calculation. MFC after: 3 days PR: 277420 Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D44202 (cherry picked from commit c291b7914e1db9469cc820abcb1f5dde7a6f7f28) tarfs: Remove unnecessary hack and obsolete comment. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: allanjude Differential Revision: https://reviews.freebsd.org/D44203 (cherry picked from commit e212f0c0666e7d3a24dce03b8c88920d14b80e47) tarfs: Fix checksum calculation. The checksum code assumed that struct ustar_header filled an entire block and calculcated the checksum based on the size of the structure. The header is in fact only 500 bytes long while the checksum covers the entire block (“logical record” in POSIX terms). Add padding and an assertion, and clean up the checksum code. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: imp Differential Revision: https://reviews.freebsd.org/D44226 (cherry picked from commit 0118b0c8e58a438a931a5ce1bf8d7ae6208cc61b) tarfs: Factor out common test code. MFC after: 3 days Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: allanjude Differential Revision: https://reviews.freebsd.org/D44227 (cherry picked from commit 32b8aac6f9b77a1c4326083472d634e5de427547) tarfs: Fix checksum on 32-bit platforms. MFC after: 3 days Fixes: b56872332e47786afc09515a4daaf1388da4d73c Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Reviewed by: bapt Differential Revision: https://reviews.freebsd.org/D44261 (cherry picked from commit cbddb2f02c7687d1039abcffd931e94e481c11a5) sys/fs/tarfs/tarfs_vfsops.c | 222 +++++++++++++++++++++----------------- tests/sys/fs/tarfs/Makefile | 2 +- tests/sys/fs/tarfs/tarfs_test.sh | 120 ++++++++++++++++++--- tests/sys/fs/tarfs/tarsum.c (new) | 128 ++++++++++++++++++++++ 4 files changed, 357 insertions(+), 115 deletions(-)