From: Tim Kientzle Date: Sun, 3 May 2026 16:32:00 +0000 (-0700) Subject: [Zip] Reject empty pathnames in ZIP writer X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=14da31ff23737a890121fda1885a225cd164201a;p=thirdparty%2Flibarchive.git [Zip] Reject empty pathnames in ZIP writer An empty pathname caused a one-byte OOB read before the heap buffer in write_path() due to two compounding bugs: (1) misuse of bitwise & instead of logical &&, and (2) missing gaurd for an empty pathname --- diff --git a/Makefile.am b/Makefile.am index b19e6837c..eb470963f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -679,6 +679,7 @@ libarchive_test_SOURCES= \ libarchive/test/test_write_format_zip_compression_bzip2.c \ libarchive/test/test_write_format_zip_compression_lzmaxz.c \ libarchive/test/test_write_format_zip_empty.c \ + libarchive/test/test_write_format_zip_empty_pathname.c \ libarchive/test/test_write_format_zip_empty_zip64.c \ libarchive/test/test_write_format_zip_entry_size_unset.c \ libarchive/test/test_write_format_zip_file.c \ diff --git a/libarchive/archive_write_set_format_zip.c b/libarchive/archive_write_set_format_zip.c index 81d3db0db..452322d7a 100644 --- a/libarchive/archive_write_set_format_zip.c +++ b/libarchive/archive_write_set_format_zip.c @@ -797,6 +797,7 @@ archive_write_zip_header(struct archive_write *a, struct archive_entry *entry) unsigned char *e; unsigned char *cd_extra; size_t filename_length; + const char *path; const char *slink = NULL; size_t slink_size = 0; struct archive_string_conv *sconv = get_sconv(a, zip); @@ -959,6 +960,12 @@ archive_write_zip_header(struct archive_write *a, struct archive_entry *entry) } } filename_length = path_length(zip->entry); + path = archive_entry_pathname(zip->entry); + if (path == NULL || path[0] == '\0') { + archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC, + "ZIP format requires a non-empty pathname"); + return (ARCHIVE_FAILED); + } /* Determine appropriate compression and size for this entry. */ if (type == AE_IFLNK) { @@ -2304,7 +2311,7 @@ write_path(struct archive_entry *entry, struct archive_write *archive) written_bytes += strlen(path); /* Folders are recognized by a trailing slash. */ - if ((type == AE_IFDIR) & (path[strlen(path) - 1] != '/')) { + if ((type == AE_IFDIR) && (path[strlen(path) - 1] != '/')) { ret = __archive_write_output(archive, "/", 1); if (ret != ARCHIVE_OK) return (ARCHIVE_FATAL); diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt index 2d2ff013f..38371459e 100644 --- a/libarchive/test/CMakeLists.txt +++ b/libarchive/test/CMakeLists.txt @@ -313,6 +313,7 @@ IF(ENABLE_TEST) test_write_format_zip_compression_lzmaxz.c test_write_format_zip_compression_zstd.c test_write_format_zip_empty.c + test_write_format_zip_empty_pathname.c test_write_format_zip_empty_zip64.c test_write_format_zip_entry_size_unset.c test_write_format_zip_file.c diff --git a/libarchive/test/test_write_format_zip_empty_pathname.c b/libarchive/test/test_write_format_zip_empty_pathname.c new file mode 100644 index 000000000..e607aafa3 --- /dev/null +++ b/libarchive/test/test_write_format_zip_empty_pathname.c @@ -0,0 +1,75 @@ +/*- + * Copyright (c) 2026 Tim Kientzle + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "test.h" + +/* + * An empty pathname must be rejected (ARCHIVE_FAILED) by the ZIP writer. + * + * write_path() used bitwise & instead of logical && when checking whether + * to append a trailing slash for directory entries. Because & does not + * short-circuit, path[strlen(path)-1] was evaluated even when the entry + * was not a directory — and when the path is empty, strlen(path)-1 wraps + * to SIZE_MAX, producing a one-byte OOB read before the heap buffer. + */ +DEFINE_TEST(test_write_format_zip_empty_pathname) +{ + struct archive *a; + struct archive_entry *ae; + char buf[4096]; + size_t used; + + /* Regular file with empty pathname */ + assert((a = archive_write_new()) != NULL); + assertEqualInt(ARCHIVE_OK, archive_write_set_format_zip(a)); + assertEqualInt(ARCHIVE_OK, + archive_write_open_memory(a, buf, sizeof(buf), &used)); + + assert((ae = archive_entry_new()) != NULL); + archive_entry_copy_pathname(ae, ""); + archive_entry_set_mode(ae, AE_IFREG | 0644); + archive_entry_set_size(ae, 0); + assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae)); + archive_entry_free(ae); + + assertEqualInt(ARCHIVE_OK, archive_write_close(a)); + assertEqualInt(ARCHIVE_OK, archive_write_free(a)); + + /* Directory entry with empty pathname */ + assert((a = archive_write_new()) != NULL); + assertEqualInt(ARCHIVE_OK, archive_write_set_format_zip(a)); + assertEqualInt(ARCHIVE_OK, + archive_write_open_memory(a, buf, sizeof(buf), &used)); + + assert((ae = archive_entry_new()) != NULL); + archive_entry_copy_pathname(ae, ""); + archive_entry_set_mode(ae, AE_IFDIR | 0755); + archive_entry_set_size(ae, 0); + assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae)); + archive_entry_free(ae); + + assertEqualInt(ARCHIVE_OK, archive_write_close(a)); + assertEqualInt(ARCHIVE_OK, archive_write_free(a)); +}