Merge pull request #3232 from kientzle/kientzle-read-data-into-fd-size-boundary

[Write] Truncate write-to-fd at the declared entry size
This commit is contained in:
Dustin L. Howett authored and GitHub committed 2026-07-04 15:08:23 -05:00
commit 3c3ec69d44
12 files changed
+307 -6

No files matched your search

+6
View File
@@ -456,6 +456,7 @@ libarchive_test_SOURCES= \
libarchive/test/test_open_filename.c \
libarchive/test/test_pax_filename_encoding.c \
libarchive/test/test_pax_xattr_header.c \
libarchive/test/test_read_data_into_fd_size_exceeds_declared.c \
libarchive/test/test_read_data_large.c \
libarchive/test/test_read_disk.c \
libarchive/test/test_read_disk_directory_traversals.c \
@@ -826,6 +827,8 @@ libarchive_test_EXTRA_DIST=\
libarchive/test/test_rar_multivolume_uncompressed_files.part08.rar.uu \
libarchive/test/test_rar_multivolume_uncompressed_files.part09.rar.uu \
libarchive/test/test_rar_multivolume_uncompressed_files.part10.rar.uu \
libarchive/test/test_read_data_into_fd_size_exceeds_declared_deflate.zip.uu \
libarchive/test/test_read_data_into_fd_size_exceeds_declared_stored.zip.uu \
libarchive/test/test_read_files_compressed.part01.uu \
libarchive/test/test_read_files_compressed.part02.uu \
libarchive/test/test_read_filter_grzip.tar.grz.uu \
@@ -1332,6 +1335,7 @@ bsdtar_test_SOURCES= \
tar/test/test_option_r.c \
tar/test/test_option_s.c \
tar/test/test_option_safe_writes.c \
tar/test/test_option_stdout_size_exceeds_declared.c \
tar/test/test_option_uid_uname.c \
tar/test/test_option_uuencode.c \
tar/test/test_option_xattrs.c \
@@ -1383,6 +1387,8 @@ bsdtar_test_EXTRA_DIST= \
tar/test/test_option_keep_newer_files.tar.Z.uu \
tar/test/test_option_passphrase.zip.uu \
tar/test/test_option_s.tar.Z.uu \
tar/test/test_option_stdout_size_exceeds_declared_deflate.zip.uu \
tar/test/test_option_stdout_size_exceeds_declared_stored.zip.uu \
tar/test/test_patterns_2.tar.uu \
tar/test/test_patterns_3.tar.uu \
tar/test/test_patterns_4.tar.uu \
+7
View File
@@ -108,6 +108,7 @@ archive_read_new(void)
return (NULL);
}
a->archive.vtable = &archive_read_vtable;
a->entry_bytes_declared = -1;
a->passphrases.last = &a->passphrases.first;
@@ -688,6 +689,12 @@ _archive_read_next_header2(struct archive *_a, struct archive_entry *entry)
break;
}
if (r2 == ARCHIVE_OK || r2 == ARCHIVE_WARN)
a->entry_bytes_declared = archive_entry_size_is_set(entry)
? archive_entry_size(entry) : -1;
else
a->entry_bytes_declared = -1;
__archive_reset_read_data(&a->archive);
a->data_start_node = a->client.cursor;
+50 -6
View File
@@ -37,6 +37,7 @@
#include "archive.h"
#include "archive_private.h"
#include "archive_read_private.h"
/* Maximum amount of data to write at one time. */
#define MAX_WRITE (1024 * 1024)
@@ -97,12 +98,15 @@ archive_read_data_into_fd(struct archive *a, int fd)
int64_t fd_offset;
int64_t target_offset;
int64_t actual_offset = 0;
int64_t declared_size;
int can_lseek;
char *nulls = NULL;
archive_check_magic(a, ARCHIVE_READ_MAGIC, ARCHIVE_STATE_DATA,
"archive_read_data_into_fd");
declared_size = ((struct archive_read *)a)->entry_bytes_declared;
can_lseek = (fstat(fd, &st) == 0) && S_ISREG(st.st_mode);
if (can_lseek) {
fd_offset = lseek(fd, 0, SEEK_CUR);
@@ -113,6 +117,24 @@ archive_read_data_into_fd(struct archive *a, int fd)
while ((r = archive_read_data_block(a, &buff, &size, &target_offset)) ==
ARCHIVE_OK) {
const char *p = buff;
int overran = 0;
if (declared_size >= 0) {
/* We know the expected size, let's enforce that we don't overrun. */
if (target_offset > declared_size) {
/* We're already beyond the end? Don't write any more. */
target_offset = declared_size;
size = 0;
overran = 1;
} else if (size > (uint64_t)(declared_size - target_offset)) {
/* Above is safe because target_offset <= declared_size */
/* This block is bigger than the expected remainder */
size = (size_t)(declared_size - target_offset);
overran = 1;
}
/* Else size <= expected remainder of data and we're OK. */
}
if (target_offset > actual_offset) {
r = pad_to(a, fd, can_lseek, &nulls,
target_offset, actual_offset);
@@ -134,15 +156,37 @@ archive_read_data_into_fd(struct archive *a, int fd)
p += bytes_written;
size -= bytes_written;
}
if (overran) {
archive_set_error(a, ARCHIVE_ERRNO_MISC,
"Actual entry size exceeds the declared size "
"(%jd); truncated output at the declared size",
(intmax_t)declared_size);
r = ARCHIVE_FAILED;
goto cleanup;
}
}
if (r == ARCHIVE_EOF && target_offset > actual_offset) {
r2 = pad_to(a, fd, can_lseek, &nulls,
target_offset, actual_offset);
if (r2 != ARCHIVE_OK)
r = r2;
else
actual_offset = target_offset;
int trailing_hole_overran =
declared_size >= 0 && target_offset > declared_size;
if (trailing_hole_overran)
target_offset = declared_size;
if (target_offset > actual_offset) {
r2 = pad_to(a, fd, can_lseek, &nulls,
target_offset, actual_offset);
if (r2 != ARCHIVE_OK)
r = r2;
else
actual_offset = target_offset;
}
if (trailing_hole_overran) {
archive_set_error(a, ARCHIVE_ERRNO_MISC,
"Actual entry size exceeds the declared size "
"(%jd); truncated output at the declared size",
(intmax_t)declared_size);
r = ARCHIVE_FAILED;
goto cleanup;
}
}
cleanup:
+9
View File
@@ -180,6 +180,15 @@ struct archive_read {
/* File offset of beginning of most recently-read header. */
int64_t header_position;
/*
* Declared uncompressed size of the entry whose header was most
* recently read, or -1 if no size was declared. Lets format-
* agnostic consumers like archive_read_data_into_fd() cap their
* output at the size an entry actually promised, regardless of
* whether the format reader enforces that itself.
*/
int64_t entry_bytes_declared;
/* Nodes and offsets of compressed data block */
unsigned int data_start_node;
unsigned int data_end_node;
+1
View File
@@ -85,6 +85,7 @@ IF(ENABLE_TEST)
test_open_filename.c
test_pax_filename_encoding.c
test_pax_xattr_header.c
test_read_data_into_fd_size_exceeds_declared.c
test_read_data_large.c
test_read_disk.c
test_read_disk_directory_traversals.c
@@ -0,0 +1,119 @@
/*-
* 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"
#if defined(_WIN32) && !defined(__CYGWIN__)
#define open _open
#define close _close
#endif
/*
* archive_read_data_into_fd() writes archive_read_data_block() output
* straight to an fd with no notion of the entry's declared size -- it's
* used directly by callers like bsdtar's "-O" (stdout) extraction and
* bsdcat, which bypass archive_write_disk (and the size-truncation
* safeguard archive_write_disk already has). This is a format-agnostic
* backstop for that gap: it must never write more than the entry's
* declared size, regardless of which format reader produced the data
* or whether that reader independently enforces its own size boundary.
*
* The reference archives are crafted ZIP files (a stored entry whose
* physical body is longer than its declared size, and a deflate entry
* whose real inflated output exceeds 256KiB so it can't complete within
* a single internal decode call -- see zip_read_data_deflate()'s
* internal output buffer). Each has two entries: "first" declares
* uncompressed_size=10 but its real content is longer, and "second" is
* a well-formed 11-byte "hello world" file, read right after "first"
* fails to confirm the mis-declared entry doesn't disrupt the rest of
* the archive. This test is deliberately independent of the ZIP reader
* itself enforcing this boundary -- archive_read_data_into_fd() must
* cap its own output regardless of whether the format reader does.
*/
static void
verify(const char *refname)
{
struct archive *a;
struct archive_entry *ae;
char tmpfilename[] = "data_into_fd_test";
int fd;
char buff[32];
size_t n;
FILE *f;
extract_reference_file(refname);
assert((a = archive_read_new()) != NULL);
assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a));
assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a));
assertEqualIntA(a, ARCHIVE_OK,
archive_read_open_filename(a, refname, 10240));
assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
assertEqualString("first", archive_entry_pathname(ae));
assertEqualInt(10, archive_entry_size(ae));
fd = open(tmpfilename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
assert(fd >= 0);
failure("archive_read_data_into_fd() must report an error when the "
"entry's actual data exceeds its declared size");
assert(archive_read_data_into_fd(a, fd) != ARCHIVE_OK);
close(fd);
f = fopen(tmpfilename, "rb");
assert(f != NULL);
n = fread(buff, 1, sizeof(buff), f);
fclose(f);
failure("archive_read_data_into_fd() must never write more bytes "
"than the entry's declared size");
assert(n <= 10);
assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
assertEqualString("second", archive_entry_pathname(ae));
assertEqualInt(11, archive_entry_size(ae));
fd = open(tmpfilename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
assert(fd >= 0);
assertEqualIntA(a, ARCHIVE_OK, archive_read_data_into_fd(a, fd));
close(fd);
f = fopen(tmpfilename, "rb");
assert(f != NULL);
n = fread(buff, 1, sizeof(buff), f);
fclose(f);
assertEqualInt(11, (int)n);
assertEqualMem(buff, "hello world", 11);
assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header(a, &ae));
assertEqualInt(ARCHIVE_OK, archive_read_free(a));
}
DEFINE_TEST(test_read_data_into_fd_size_exceeds_declared_stored)
{
verify("test_read_data_into_fd_size_exceeds_declared_stored.zip");
}
DEFINE_TEST(test_read_data_into_fd_size_exceeds_declared_deflate)
{
verify("test_read_data_into_fd_size_exceeds_declared_deflate.zip");
}
@@ -0,0 +1,23 @@
begin 644 test_read_data_into_fd_size_exceeds_declared_deflate.zip
M4$L#!!0````(``````#"V8Q_D`(```H````%````9FER<W3MR#$!```(P*!P
MLW\F+TT!)W7FY9QSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCG7`E!+`P04````````````A1%*#0L````+````!@``
M`'-E8V]N9&AE;&QO('=O<FQD4$L!`A0`%`````@``````,+9C'^0`@``"@``
M``4``````````````````````&9I<G-T4$L!`A0`%````````````(412@T+
M````"P````8`````````````````LP(``'-E8V]N9%!+!08``````@`"`&<`
(``#B`@``````
`
end
@@ -0,0 +1,9 @@
begin 644 test_read_data_into_fd_size_exceeds_declared_stored.zip
M4$L#!!0```````````#-F1X3%`````H````%````9FER<W1!04%!04%!04%!
M0D)"0D)"0D)"0E!+`P04````````````A1%*#0L````+````!@```'-E8V]N
M9&AE;&QO('=O<FQD4$L!`A0`%````````````,V9'A,4````"@````4`````
M`````````````````&9I<G-T4$L!`A0`%````````````(412@T+````"P``
M``8`````````````````-P```'-E8V]N9%!+!08``````@`"`&<```!F````
"````
`
end
+1
View File
@@ -70,6 +70,7 @@ IF(ENABLE_TAR AND ENABLE_TEST)
test_option_r.c
test_option_s.c
test_option_safe_writes.c
test_option_stdout_size_exceeds_declared.c
test_option_uid_uname.c
test_option_uuencode.c
test_option_xattrs.c
@@ -0,0 +1,50 @@
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2026 Tim Kientzle
* All rights reserved.
*/
#include "test.h"
/*
* `bsdtar -x` writing to real files is protected by archive_write_disk,
* which already truncates output at an entry's declared size. `bsdtar
* -xO` (stdout) bypasses archive_write_disk entirely and writes
* archive_read_data_into_fd()'s output straight to fd 1 -- this test
* exercises that path's own backstop (added directly to
* archive_read_data_into_fd(), so it applies no matter which archive
* format is being read) at the bsdtar CLI level.
*
* The reference archives are crafted ZIP files with two entries: "first"
* declares uncompressed_size=10 but its true content is longer
* ("AAAAAAAAAABBBBBBBBBB" for _stored, 256KiB+6 bytes of
* "CCCCCCCCCCDDDDDDDDDD..." for _deflate), and "second" is a normal
* 11-byte "hello world" file, extracted right after "first" fails to
* confirm the mis-declared entry doesn't disrupt the rest of the
* archive.
*
* The ZIP reader itself also rejects "first" outright (a separate,
* format-level fix -- see test_read_format_zip_size_exceeds_declared),
* so archive_read_data_block() never hands this backstop any bytes for
* "first" to (partially) pass through: stdout ends up with none of
* "first"'s data at all, followed directly by "second". That's still
* "never more than declared" -- just fewer bytes than the boundary
* allows, since the format reader was already more conservative here.
*/
static void
verify(const char *refname)
{
extract_reference_file(refname);
failure("bsdtar -xO must report an error for the mis-declared entry");
assert(systemf("%s -xOf %s >test.out 2>test.err", testprog, refname)
!= 0);
assertFileContents("hello world", 11, "test.out");
assertNonEmptyFile("test.err");
}
DEFINE_TEST(test_option_stdout_size_exceeds_declared)
{
verify("test_option_stdout_size_exceeds_declared_stored.zip");
verify("test_option_stdout_size_exceeds_declared_deflate.zip");
}
@@ -0,0 +1,23 @@
begin 644 test_option_stdout_size_exceeds_declared_deflate.zip
M4$L#!!0````(``````#"V8Q_D`(```H````%````9FER<W3MR#$!```(P*!P
MLW\F+TT!)W7FY9QSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGGG'/..>><<\XYYYQSSCGG
MG'/..>><<\XYYYQSSCG7`E!+`P04````````````A1%*#0L````+````!@``
M`'-E8V]N9&AE;&QO('=O<FQD4$L!`A0`%`````@``````,+9C'^0`@``"@``
M``4``````````````````````&9I<G-T4$L!`A0`%````````````(412@T+
M````"P````8`````````````````LP(``'-E8V]N9%!+!08``````@`"`&<`
(``#B`@``````
`
end
@@ -0,0 +1,9 @@
begin 644 test_option_stdout_size_exceeds_declared_stored.zip
M4$L#!!0```````````#-F1X3%`````H````%````9FER<W1!04%!04%!04%!
M0D)"0D)"0D)"0E!+`P04````````````A1%*#0L````+````!@```'-E8V]N
M9&AE;&QO('=O<FQD4$L!`A0`%````````````,V9'A,4````"@````4`````
M`````````````````&9I<G-T4$L!`A0`%````````````(412@T+````"P``
M``8`````````````````-P```'-E8V]N9%!+!08``````@`"`&<```!F````
"````
`
end