archive_read_disk_posix: Don't pass -1 to a function expecting errno
This fixes an unhelpful "Couldn't visit directory: Unknown error: -1" message.
Fixes: 3311bb52cbe4 ("Bring the code supporting directory traversals from bsdtar/tree.[ch] into archive_read_disk.c and modify it. Introduce new APIs archive_read_disk_open and archive_read_disk_descend.")
RAR5 reader: early fail when file declares data for a dir entry
RAR5 reader had inconsistent sanity checks for directory entries that
declare data. On one hand such declaration was accepted during the
header parsing process, but at the same time this was disallowed during
the data reading process. Disallow logic was returning the
ARCHIVE_FAILED error code that allowed the client to retry, while in
reality, the error was non-retryable.
This commit adds another sanity check during the header parsing logic
that disallows the directory entries to declare any data. This will make
clients fail early when such entry is detected.
Also, the commit changes the ARCHIVE_FAILED error code to ARCHIVE_FATAL
when trying to read data for the directory entry that declares data.
This makes sure that tools like bsdtar won't attempt to retry unpacking
such data.
RAR5 reader: fix multiple issues in extra field parsing function
This commit fixes multiple issues found in the function that parses
extra fields found in the "file"/"service" blocks.
1. In case the file declares just one extra field, which is an
unsupported field, the function returns ARCHIVE_FATAL.
The commit fixes this so this case is allowed, and the unsupported
extra field is skipped. The commit also introduces a test for this
case.
2. Current parsing method of extra fields can report parsing errors in
case the file is malformed. The problem is that next iteration of
parsing, which is meant to process the next extra field (if any),
overwrites the result of the previous iteration, even if previous
iteration has reported parsing error. A successful parse can be
returned in this case, leading to undefined behavior.
This commit changes the behavior to fail the parsing function early.
Also a test file is introduced for this case.
3. In case the file declares only the EX_CRYPT extra field, current
function simply returns ARCHIVE_FATAL, preventing the caller from
setting the proper error string. This results in libarchive returning
an ARCHIVE_FATAL without any error messages set. The PR #2096 (commit adee36b00) was specifically created to provide error strings in case
EX_CRYPT attribute was encountered, but current behavior contradicts
this case.
The commit changes the behavior so that ARCHIVE_OK is returned by the
extra field parsing function in only EX_CRYPT is encountered, so that
the caller header reading function can properly return ARCHIVE_FATAL
to the caller, at the same time setting a proper error string. A test
file is also provided for this case.
Tim Kientzle [Sat, 26 Jul 2025 18:10:24 +0000 (11:10 -0700)]
Guard against invalid type arguments
Some experiments showed strange things happen if you
provide an invalid type value when appending a new ACL entry.
Guard against that, and while we're here be a little more
paranoid elsewhere against bad types in case there is another
way to get them in.
Alex James [Sat, 12 Jul 2025 20:44:55 +0000 (15:44 -0500)]
Fix mkstemp path in setup_mac_metadata
setup_mac_metadata currently concates the template after TMPDIR without
adding a path separator, which causes mkstemp to create a temporary file
next to TMPDIR instead of in TMPDIR. Add a path separator to the
template to ensure that the temporary file is created under TMPDIR.
I hit this while rebuilding libarchive in nixpkgs. Lix recently started
using a dedicated build directory (under /nix/var/nix/builds) instead of
using a directory under /tmp [1]. nixpkgs & Lix support (optional)
on macOS sandboxing. The default sandbox profile allows builds to access
paths under the build directory and any path under /tmp. Because the
build directory is no longer under /tmp, some of libarchive's tests
started to fail as they accessed paths next to (but not under) the build
directory:
cpio/test/test_basic.c:65: Contents don't match
Description: Expected: 2 blocks
, options=
file="pack.err"
0000_62_73_64_63_70_69_6f_3a_20_43_6f_75_6c_64_20_6e_bsdcpio: Could n
0010_6f_74_20_6f_70_65_6e_20_65_78_74_65_6e_64_65_64_ot open extended
0020_20_61_74_74_72_69_62_75_74_65_20_66_69_6c_65_0a_ attribute file.
It is possible to handle entries and files with sizes which do not fit
into off_t of the current system (Windows always has 32 bit off_t and
32 bit systems without large file support also have 32 bit off_t).
Set sizes to 0 in such cases. The fstat system call would return -1 and
set errno to EOVERFLOW, but that's not how archive_entry_set_size acts.
It would simply ignore negative values and set the size to 0.
Actual callers of archive_entry_stat from foreign projects seem to not
even check for NULL return values, so let's try to handle such cases as
nice as possible.
Affects mtree's checkfs option as well (Windows only, 32 bit systems
would simply fail in fstat/stat).
Dustin Howett [Fri, 15 Oct 2021 22:47:53 +0000 (17:47 -0500)]
win32: shim wopen, and make both open/wopen use _s "secure" variant
The new `__la_wopen` wrapper is a copy of `__la_open` that
expects--rather than converts--a wcs parameter.
The `sopen` variants are offered as "more secure" variants of `open` and
`wopen`; I cannot vouch for their security, but some build systems are
strict about the use of "banned insecure APIs".
I've confirmed that `_wsopen_s` and `_open_s` are present in the Windows
Vista SDK.
I did not confirm that they are available in the Windows XP Platform
SDK, in part because in e61afbd463d1 (2016!) Tim says:
> I'd like to completely remove support for WinXP and earlier.
Rose [Wed, 11 Jun 2025 19:21:46 +0000 (15:21 -0400)]
Fix error checking in writing files
For write, 0 may not mean an error at all. We need to instead check for the length not being the same.
With fwrite, because 0 could mean an error, but not always. We must check that we wrote the entire file!
Note that unlike write, fwrite's description according to POSIX does not mention returning a negative type at all. Nor does it say you can retry unlike write.
Finally, with write, we need to check less than 0, not 0, as 0 is a valid return and does not mean an error.
Make sure that the string table size is not smaller than 6 (and also
not larger than SIZE_MAX for better 32 bit support).
Such small values would lead to a large loop limit which either leads to
a crash or wrong detection of a ".data" string in possibly uninitialized
memory.
If a sparse hole is located at the end of an entry, then the tar
parser returns ARCHIVE_EOF while updating the offset where 0 bytes of
data will follow.
If archive_read_data encounters such an ARCHIVE_EOF return value, it
has to recheck if the offsets (data offset and output offset) still
match. If they do not match, it has to keep filling 0 bytes.
This changes assumes that it's okay to call archive_read_data_block
again after an EOF. As far as I understood the parsers so far, this
should be okay, since it's always ARCHIVE_EOF afterwards.
rar: Do not forcefully set offset to unpacked size
If an entry reaches its end of file, the offset is not necessarily
the same as unp_size. This is especially true for links which have
a "0 size body" even though the unpacked size is not 0.
When _warc_read encounters end of entry, it adds 4 bytes to the last
offset for \r\n\r\n separator, which is never written. Ignore these
bytes since they are not part of the returned entry.
The string constants can be used directly for comparison, which makes
this code robust against future changes which could lead to names being
longer than str could hold on stack.
Also removes around 100 bytes from compiled library (with gcc 15).
If zlib is not supported, do not run tests to avoid false positives.
Also adjust tests to support latest gzip versions (1.10+) which store
less information for improved reproducibility. The gzip binary is
used as a fallback if zlib is not available.
If no encryption support exists, the -P option will always fail.
"Skip" the test by making sure that there really is no encryption
support according to libarchive functions.
Kyle Evans [Tue, 3 Jun 2025 02:43:28 +0000 (21:43 -0500)]
libarchive/test: fix build when memcpy() is a macro
After importing the latest libarchive into FreeBSD, Shawn Webb @
HardenedBSD noted that the test build is broken when FORTIFY_SOURCE=2
while building the base system. Braced initializer lists are a special
case that need some extra fun parentheses when we're dealing with the
preprocessor.
While it's not a particularly common setup, the extra parentheses don't
really hurt readability all that much so it's worth fixing for wider
compatibility.
Ignoring SIGCHLD gets passed to child processes. Doing that has
influence on waitpid, namely that zombie processes won't be
created. This means that a status can never be read.
We can't enforce this in library, but libarchive's tools can be
protected against this by enforcing default handling.
Use pid_t since waitpid returns a pid_t. Also check for a negative
return value in writer as well to avoid reading the possibly
unitialized status value.
Calling CloseHandle multiple times for the same handle can lead to
exceptions while debugging according to documentation.
Mimic the waitpid handling for success cases to behave more like the
Unix version which would "reap the zombie".
Doing this for an unsuccessful call is off, but the loop is never
entered again, so I guess it's okay and worth it to reduce the amount
of Windows specific definitions in source files.
The archive_utility_string_sort function won't be part of the 4.0.0 API
anymore. No users were found and such a task should be done outside of
the library.
The utility function "archive_utility_string_sort" is a custom qsort
implementation. Since qsort is specified in C11 and POSIX.1-2008
which libarchive is based on, use system's qsort directly.
The function is not used directly in libarchive, so this is a good
way to save around 500 bytes in resulting library without breaking
compatibility for any user of this function (none found).
Also allows more than UINT_MAX entries which previously were limited
by data type and (way earlier) due to recursion.
Test cases already get a C locale, which is sufficient for this test.
IF LC_TIME was not previously set, the used en_US.UTF-8 would stay
as an environment variable, possibly affecting other test cases.
Since en_US.UTF-8 is not guaranteed to be available, C is a better
choice.
Reset current locale settings through setlocale and also all
environment variables which might affect test cases which
spawn children through systemf which in turn would call setlocale
on their own, e.g. bsdtar.
Explicitly use goto to turn a recursive call into an iterative one.
Most compilers do this on their own with default settings, but MSVC
with default settings would create a binary which actually performs
recursive calls.
Fixes call stack overflow in binaries compiled with low optimization.