From: Tim Kientzle Date: Wed, 1 Jun 2016 04:01:59 +0000 (-0700) Subject: Issue 682: Correctly write gnutar filenames of exactly 512 bytes X-Git-Tag: v3.2.1~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ed497f0e18339d5412e0dfd9e86e671187dadc9d;p=thirdparty%2Flibarchive.git Issue 682: Correctly write gnutar filenames of exactly 512 bytes Previous code omitted the final zero byte for filenames and linknames. This is usually okay since the final block is padded with zero bytes, but if the filename exactly filled the block, there would be no zero byte. --- diff --git a/Makefile.am b/Makefile.am index 4a1967358..b45c727d9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -223,6 +223,7 @@ libarchive_la_SOURCES= \ libarchive/archive_write_set_format_ustar.c \ libarchive/archive_write_set_format_v7tar.c \ libarchive/archive_write_set_format_gnutar.c \ + libarchive/archive_write_set_format_gnutar_filenames.c \ libarchive/archive_write_set_format_warc.c \ libarchive/archive_write_set_format_xar.c \ libarchive/archive_write_set_format_zip.c \ diff --git a/libarchive/archive_write_set_format_gnutar.c b/libarchive/archive_write_set_format_gnutar.c index 647079de6..1d635d2dc 100644 --- a/libarchive/archive_write_set_format_gnutar.c +++ b/libarchive/archive_write_set_format_gnutar.c @@ -467,7 +467,7 @@ archive_write_gnutar_header(struct archive_write *a, } } if (gnutar->linkname_length > GNUTAR_linkname_size) { - size_t todo = gnutar->linkname_length; + size_t length = gnutar->linkname_length + 1; struct archive_entry *temp = archive_entry_new2(&a->archive); /* Uname/gname here don't really matter since no one reads them; @@ -476,7 +476,7 @@ archive_write_gnutar_header(struct archive_write *a, archive_entry_set_gname(temp, "wheel"); archive_entry_set_pathname(temp, "././@LongLink"); - archive_entry_set_size(temp, gnutar->linkname_length + 1); + archive_entry_set_size(temp, length); ret = archive_format_gnutar_header(a, buff, temp, 'K'); if (ret < ARCHIVE_WARN) goto exit_write_header; @@ -484,11 +484,12 @@ archive_write_gnutar_header(struct archive_write *a, if(ret < ARCHIVE_WARN) goto exit_write_header; archive_entry_free(temp); - /* Write as many 512 bytes blocks as needed to write full name. */ - ret = __archive_write_output(a, gnutar->linkname, todo); + /* Write name and trailing null byte. */ + ret = __archive_write_output(a, gnutar->linkname, length); if(ret < ARCHIVE_WARN) goto exit_write_header; - ret = __archive_write_nulls(a, 0x1ff & (-(ssize_t)todo)); + /* Pad to 512 bytes */ + ret = __archive_write_nulls(a, 0x1ff & (-(ssize_t)length)); if (ret < ARCHIVE_WARN) goto exit_write_header; } @@ -496,7 +497,7 @@ archive_write_gnutar_header(struct archive_write *a, /* If pathname is longer than 100 chars we need to add an 'L' header. */ if (gnutar->pathname_length > GNUTAR_name_size) { const char *pathname = gnutar->pathname; - size_t todo = gnutar->pathname_length; + size_t length = gnutar->pathname_length + 1; struct archive_entry *temp = archive_entry_new2(&a->archive); /* Uname/gname here don't really matter since no one reads them; @@ -505,7 +506,7 @@ archive_write_gnutar_header(struct archive_write *a, archive_entry_set_gname(temp, "wheel"); archive_entry_set_pathname(temp, "././@LongLink"); - archive_entry_set_size(temp, gnutar->pathname_length + 1); + archive_entry_set_size(temp, length); ret = archive_format_gnutar_header(a, buff, temp, 'L'); if (ret < ARCHIVE_WARN) goto exit_write_header; @@ -513,11 +514,12 @@ archive_write_gnutar_header(struct archive_write *a, if(ret < ARCHIVE_WARN) goto exit_write_header; archive_entry_free(temp); - /* Write as many 512 bytes blocks as needed to write full name. */ - ret = __archive_write_output(a, pathname, todo); + /* Write pathname + trailing null byte. */ + ret = __archive_write_output(a, pathname, length); if(ret < ARCHIVE_WARN) goto exit_write_header; - ret = __archive_write_nulls(a, 0x1ff & (-(ssize_t)todo)); + /* Pad to multiple of 512 bytes. */ + ret = __archive_write_nulls(a, 0x1ff & (-(ssize_t)length)); if (ret < ARCHIVE_WARN) goto exit_write_header; } diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt index ae5a1aa8b..b70be7422 100644 --- a/libarchive/test/CMakeLists.txt +++ b/libarchive/test/CMakeLists.txt @@ -222,6 +222,7 @@ IF(ENABLE_TEST) test_write_format_cpio_newc.c test_write_format_cpio_odc.c test_write_format_gnutar.c + test_write_format_gnutar_filenames.c test_write_format_iso9660.c test_write_format_iso9660_boot.c test_write_format_iso9660_empty.c diff --git a/libarchive/test/test_write_format_gnutar_filenames.c b/libarchive/test/test_write_format_gnutar_filenames.c new file mode 100644 index 000000000..32f5ce4f3 --- /dev/null +++ b/libarchive/test/test_write_format_gnutar_filenames.c @@ -0,0 +1,145 @@ +/*- + * Copyright (c) 2016 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" +__FBSDID("$FreeBSD$"); + +/* + * Inspired by Github issue #682, which reported that gnutar filenames + * of exactly 512 bytes weren't getting written correctly. + * + * This writes a filename of every length from 1 to 2000 bytes and + * reads back to verify it. + */ + +static char filename[1024]; + +DEFINE_TEST(test_write_format_gnutar_filenames) +{ + size_t buffsize = 1000000; + char *buff; + struct archive_entry *ae, *template; + struct archive *a; + size_t used; + + buff = malloc(buffsize); /* million bytes of work area */ + assert(buff != NULL); + + /* Create a template entry. */ + assert((template = archive_entry_new()) != NULL); + archive_entry_set_atime(template, 2, 20); + archive_entry_set_birthtime(template, 3, 30); + archive_entry_set_ctime(template, 4, 40); + archive_entry_set_mtime(template, 5, 50); + archive_entry_set_mode(template, S_IFREG | 0755); + archive_entry_set_size(template, 8); + + for (int i = 0; i < 2000; ++i) { + filename[i] = 'a'; + filename[i + 1] = '\0'; + archive_entry_copy_pathname(template, filename); + + /* Write a one-item gnutar format archive. */ + assert((a = archive_write_new()) != NULL); + assertA(0 == archive_write_set_format_gnutar(a)); + assertA(0 == archive_write_add_filter_none(a)); + assertA(0 == archive_write_open_memory(a, buff, buffsize, &used)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_header(a, template)); + assertEqualIntA(a, 8, archive_write_data(a, "12345678", 9)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_close(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_free(a)); + + + /* Read back and verify the filename. */ + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, 0, archive_read_support_format_all(a)); + assertEqualIntA(a, 0, archive_read_support_filter_all(a)); + assertEqualIntA(a, 0, archive_read_open_memory(a, buff, used)); + + assertEqualIntA(a, 0, archive_read_next_header(a, &ae)); + assertEqualString(filename, archive_entry_pathname(ae)); + assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header(a, &ae)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a)); + } + + archive_entry_free(template); + + free(buff); +} + + +DEFINE_TEST(test_write_format_gnutar_linknames) +{ + size_t buffsize = 1000000; + char *buff; + struct archive_entry *ae, *template; + struct archive *a; + size_t used; + + buff = malloc(buffsize); /* million bytes of work area */ + assert(buff != NULL); + + /* Create a template entry. */ + assert((template = archive_entry_new()) != NULL); + archive_entry_set_atime(template, 2, 20); + archive_entry_set_birthtime(template, 3, 30); + archive_entry_set_ctime(template, 4, 40); + archive_entry_set_mtime(template, 5, 50); + archive_entry_set_mode(template, S_IFLNK | 0755); + archive_entry_copy_pathname(template, "link"); + + for (int i = 0; i < 2000; ++i) { + filename[i] = 'a'; + filename[i + 1] = '\0'; + archive_entry_copy_symlink(template, filename); + + /* Write a one-item gnutar format archive. */ + assert((a = archive_write_new()) != NULL); + assertA(0 == archive_write_set_format_gnutar(a)); + assertA(0 == archive_write_add_filter_none(a)); + assertA(0 == archive_write_open_memory(a, buff, buffsize, &used)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_header(a, template)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_close(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_write_free(a)); + + + /* Read back and verify the filename. */ + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, 0, archive_read_support_format_all(a)); + assertEqualIntA(a, 0, archive_read_support_filter_all(a)); + assertEqualIntA(a, 0, archive_read_open_memory(a, buff, used)); + + assertEqualIntA(a, 0, archive_read_next_header(a, &ae)); + assertEqualString("link", archive_entry_pathname(ae)); + assertEqualString(filename, archive_entry_symlink(ae)); + assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header(a, &ae)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a)); + } + + archive_entry_free(template); + + free(buff); +}