The seek implementations for files return ARCHIVE_FATAL if seek fails to
adjust the position. ARCHIVE_FAILED is used in case of pipes etc.
Also, if a file seek fails, the position is not adjusted.
Adjust the memory implementations to do the same.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Actually follow the bidder comment and return amount of bits. Has the
nice advantage of removing the int/ssize_t handling, even though it was
safe anyway.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If open functions fail to store client data, free internally allocated
memory before returning an error code.
Writer open functions must check magic themselves.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If upstream filter provides huge amount of bytes, e.g. if archive is
fully stored in memory, bidding accesses too many bytes. This could in
turn overflow int return value, possibly leading to signed integer
overflows.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If registering a format parser bidder fails or triggers a warning,
forward the information instead of silently turning it into ARCHIVE_OK.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If a signature is too large, the bid value could overflow INT_MAX. With
its current implementation, it could even lead to a signed integer
overflow, which is undefined behavior.
Disallow huge signatures, which most likely won't impact anyone.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The name was only needed to find the appropriate bidder for a filter.
Since this has been resolved by accessing the last bidder, since in these
situations the bidder is added first, then the filter, the name is not
needed anymore.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Appending a filter implies appending a bidder. Always use the last added
bidder, since it was just added. This avoids desync situations where
previously added bidders could erroneously match.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
A varint field can actually have up to 10 bytes to resemble whole 64 bit
of a value. Use checked arithmetic to detect integer overflows.
Also, treat a varint which never has a byte without continuation bit as
faulty.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If there is no good reason to deviate from the rule, always use "f" for
filter just as we use "a" for archive.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
- If filter is meant, use "f" instead of "self" or "filter".
- If bidder is meant, use "b" instead of "self".
Matches writer filter style.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use "struct filter *filter" pattern, with filter being the name of the
filter. This matches the format parser pattern.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
- Declare access variable at the beginning
- Remove unneeded cast (see warc parser, which never did it)
- Remove inline cast function
- Avoid unneeded brackets to unify style
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Even though int64_t would be the correct data type because no more data
than INT64_MAX can be written, use the easier solution for format
writers for now.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Files which cannot fit into a stream should never return success, which
can happen due to truncation on 32 bit systems.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The isdigit function is locale-dependent, which means that characters
outside of 0-9 range can return true here, e.g. "power of 2" for a few
locales on Windows.
Test for ASCII character range from '0' to '9' instead.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If allocation of filter fails, gracefully handle it instead of
eventually triggering a NULL pointer dereference.
Allocate the filter in last step, because it's rather difficult to
cleanly revert in case of error.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Create free_data function for later reusing.
Also make sure that f->data is always set to NULL.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The standard has no fixed 10 bit limit. Instead, a pre-tree could
contain up to 16 bit codes, although this is very unlikely.
Grow the table if needed.
While at it, clarify now that max_bits are actually lookup_bits.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If a pre-tree is larger than 10 bit, libarchive fails to read the data.
According to spec, up to 16 bit are valid.
Example file provided by Duncan Horn. Thanks a lot!
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
- Huffman operations are up to 16 bit, thus uint16_t
- LZX itself can handle up to 32 bit, thus uint32_t
- Memory sizes are size_t
Otherwise:
- Use uint8_t to show where bit lengths are stored
- Use int to clarify that a status is stored
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The check against table size is redundant since Kraft's inequality
already verified that this operation cannot exceed table size anymore.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use 0 as marker for maximum symbol count to be able to eventually switch
to another data type.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
This step is not actually needed, but will prevent static analysis
warnings that values might exist which have never been initialized.
"Invalid" values only exist outside of the addressable area, which can
happen if maximum code bit length is smaller than table bit length.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
It's actually the amount of symbols/codes in Huffman tree. Clarify it by
using the field name symbol_count instead.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Some lzx functions reverse the generic logic of 0 being success and
anything else being an error. It's debatable from a boolean point of
view but use the regular ARCHIVE_OK in case of success, which is 0.
Unifies with other lzx functions and general idea of libarchive
functions.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
We really have to check if Kraft's inequality is 1 (in our case it's
0x10000) or 0 for an empty tree. Anything else is an error.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
As long as the given index is in range of the table, the previous
creation of the lookup table takes care that all elements are valid.
This is a left over of actual tree removal done in commit
c253f0aae9.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Most if not all compilers today understand inline, but it's also just a
hint. Static functions will be inlined if it's worth it from compiler
perspective and inline functions will be still functions, too.
Remove it and let compilers decide. Increases portability for very old
compilers which cannot handle it.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Make sure that multiplications do not overflow (or if they overflow,
handle them properly).
This also means that proper data type size_t instead of int is used.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If strnlen is not available, custom code is used. Make sure that the
custom code keeps premise of strnlen API for maximum compatibility.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Explain the algorithm and data structure involved in Huffman tree
handling as array. Hopefully simplifies upcoming code change
verifications.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If enough bytes have to be skipped, a signed integer overflow could
occur (most realistically on 32 bit systems). Use an unsigned type,
which could still overflow but has only negative impact on diagnostic
warning message.
Follow up of 02fa05675f but this time for
function find_newc_header.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If the raw parser ever treats a basically endless stream which
eventually overflows INT64_MAX, fail instead of overflowing
offset + size calculation, leading to subsequent issues in callers.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Covers basic E8 translations, yet it lacks the handling of negative
values because this would require large archives.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The cab specification states that E8 translation shall be performed only
within the first 1 GB range of an uncompressed file, i.e. for the first
0x8000 CFDATA entries in a folder.
Take this limit properly into account.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The memory allocation in archive_read_set_callback_data2, which is
called by archive_read_set_callback_data, could fail.
Check result and properly handle failure.
Resolves#3210.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use correct data types to reduce amount of casts. This should further
improve readability.
While at it, also improve white space usage.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
It's quite rare that "p" is not the iteration pointer but the base
pointer, while "b" is not the base pointer but the iteration pointer.
Use more descriptive variable names to assist auditors to figure out how
E8 call translation reversal works. It's already hard by itself.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
It's a file size and used for signed integer comparison. Since it's
literally a file size, its maximum value should be properly checked as
well.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use more macro definitions instead of magic numbers to improve code
readability. Hopefully this explains a bit more where these values come
from.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Move the macro definitions up to the field where they are used, which is
done for other such fields as well.
This in turn allows to avoid the magic value 0 in initial assignment.
Use ST_RD_TRANSLATION instead for better documentation where the finite
state machine starts (and that this state cannot be reached again).
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Commit e1f890dc62 fixed the underlying
root cause of these OOB checks: If a header is invalid and ARCHIVE_FATAL
is returned, do not allow data reads anymore.
The checks while reading CFDATA already looked rather off, since the
check exists in header parser and the value is never updated, but
prevent worst case scenario.
Keep the tests as regression tests if archive_read.c is modified again.
This makes the code easier to read and audit.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The comment states that a warning should be returned, but
97f964e3e0 explicitly changed that to
fatal. Adjust the comment and remove unneeded function call.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The function ar_parse_common_header can be called at exactly one
position in _ar_read_header, since it parses common information.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The __archive_read_consume function is binary: Either all bytes are
consumed or a failure occurred. No partial consumption can occur.
Simplify its calls to clarify this.
While at it, adjust style of callers as well.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If a file close to SSIZE_MAX in size is mapped into memory and checked
for mtree, a signed integer overflow could occur.
While this this generally harmless and the scenario very unlikely, fix
the issue with a proper cast.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If values are only parsed and verified but not used later on, skip field
assignments. This reduces amount of fields to audit.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Make sure that all bytes are actually skipped. Normally, the result can
be ignored because the bytes were read previously. Here, more bytes than
previously read will be skipped.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If more than UINT_MAX byte are available, e.g. if a very large archive
is mapped into memory, the BZ2/DEFLATE avail_in/avail_out values are
truncated.
Set to maximum allowed value in such cases.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
With correct data types in place, simplify the overflow check by using
checked arithmetic and simple comparison.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Follow idea of internal libarchive APIs:
- File offsets and sizes are int64_t
- Memory offsets and sizes are size_t
Also use correct data types for iterator variables.
This refactoring also makes parse_7zip_uint64 obsolete by turning it
into parse_7zip_int64.
Resolves GHSA-m4vx-w6j3-jp82.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use checked arithmetic and unify unpack_streams check to cover all
cases. This should be a purely defensive measure.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Introduce parse_7zip_size, which performs the UMAX_ENTRY check.
Generally, use this function whenever memory allocation sizes or indices
into memory allocations are used, i.e. when size_t is meant.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Use checked arithmetic to avoid overflows. Use it also where checks are
already in place for better readability.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Since we have checked integer arithmetic now, use it.
Clarify that INT64_MAX is the maximum according to standard.
With that, switch to int64_t to avoid (unsigned long)-1 casts.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Verify that sum of numUnpackStreams will never overflow numDigests. For
this, also set numDigests to size_t because that's how the variable is
used later on.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If the minimum amount of bytes could overflow size_t after alignment,
allocate exact minimum amount instead.
This avoids integer overflow and will most likely lead to regular OOM
error handling instead.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
The skip_stream function is supposed to skip a specified amount of bytes
from input stream. Since streams could be larger than 4 GB even on 32
bit systems, do not cast to size_t. Instead, read the data in small
chunks until all bytes are skipped.
This also helps to avoid large allocations on 64 bit systems.
Also verify that skipped_bytes tracking cannot overflow.
While at it, change return value to int because it's only needed for
status information, i.e. if operation was successful or not. This avoids
another cast issue with uint64_t values larger than INT64_MAX.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Tobias Stoeckmann