- md5 -> sha2-256 caching
- remove reliance on md5 hashes in general
- FIPS cryptographic hash limits feature to disable md5 and sha1.
- Adds related option for ClamD, ClamScan, Freshclam, Sigtool
- ClamBC: fix crashes
- signature names that start with "Weak." won't alert anymore.
- ClamScan:
- add `--hash-hint`, `--log-hash`, `--hash-alg`, `--file-type-hint`, `--log-file-type`
- accurate counts for scanned bytes and bytes read
- libclamav:
- new cl_scan*_ex() APIs
- separate temp-directory-recursion feature from keep-temps feature
- object id's for each layer scanned
- scan hash hints
- scan file type hints
- fix double-extraction for OOXML documents
- new scan callbacks, deprecate old scan callbacks
- new APIs to get access to file data and metadata from scan callbacks
- metadata.json:
- object id's
- replace "viruses" to "alerts" and add "indicators" array
- replace "FileMD5" with "sha2-256"
- json store extra hashes feature
- Related options for ClamD and ClamScan
- object id's
- related improvements
For OpenSSL 1, `EVP_get_digestbyname()` will fail with "sha2-*" algorithm names.
Must use "sha256", etc.
I made a shim that does the conversion, and I made an improvement to ignore case
when converting alg names to our hash type enumeration.
Other fixes for a few warnings.
We presently record Alerts as an array of signature names.
Instead, it should be an object with properties of its own.
We should record alerting indicators and weak indicators in a single
"Indicators", likely with the same structure as the "Alerts" objects.
When an alerting indicator is ignored (e.g. ignored by callback or if
the file is trusted by an FP signature), we can remove it from the
"Alerts" array, and for the "Indicators" array, add a "Ignored" key with
a string value that explains why it was ignored.
This eliminates the need to track and propagate the additional
"WeakIndicators" and "IgnoredAlerts" arrays.
The current name "Viruses" is incorrect both because not all malware
are viruses, but also because ClamAV may be used to classify other
data, not just to search for malware indicators.
Renaming to "Alerts" is more consistent with other language such as
the options "--alert-exceeds-max", etc.
It will match the new "IgnoredAlerts", "ContainedAlerts", and
"IgnoredContainedAlerts" JSON key names.
If the outermost layer is trusted (e.g. using an FP signature), the verdict passed
back by the `cl_scan*_ex()` functions should be CL_VERDICT_TRUSTED.
To ensure this, and other correct verdicts, I moved the logic setting the verdict
to occur when adding indicators, or trusting a given layer. Then at the end of a
scan, it will set the output verdict parameter to the top level verdict.
This commit also:
* Fixes a bug in the `ex_scan_callbacks` program where a crash would happen when
a hash was retrieved for an inner layer, but isn't for the container.
* Added debug logs whenever a hash is calculated or set, printing the hash type
and hash string.
* When a layer is trusted, in addition to removing evidence for that layer, it
will also index the metadata JSON (if that feature is enabled) and will rename
any "Viruses" to "IgnoredAlerts", and rename "ContainedIndicators" to
"IgnoredContainedIndicators".
* Fixed an issue where setting the hash algorithm with extra characters, like
setting to "sha256789" would ignore the extra characters, and report the hash
type as the same. It will now fail if the string length differs from the known
hash algorithm.
Both enum variants are 0, so it's a no-op.
`cl_error_t` should be used to determine if we keep going or stop, whether
that's because there was a detection and we're not in allmatch mode, or if
because of an error.
`cl_verdict_t` should be used to determine or say the verdict (clean is a
verdict, though I feel 'nothing found' is more accurate).
Improvements to the ex_scan_callbacks.c program:
- Print the verdict enum variant names to be more explicit.
- Add the file_props callback (aka metadata JSON) with --gen-json option.
- Add a --debug option.
- Use '-' in option names instead of '_' to be consistent with other programs.
- Add option to disable allmatch, which I named --one-match. :)
Tests: Add ex_scan_callbacks test where --allmatch is disabled.
Verify that CL_VIRUS is returned when a match occurs.
I found a few bugs and inconsistencies from this test and went and fixed
them, and improved the clamav.h function comments as well.
Largely this resulted in cleanup in `cli_magic_scan()` to make sure we
don't accidentally overwrite the return code.
But it also meant making sure that callback functions which are supposed
to trust a file actually clear the evidence/verdict and don't return
CL_VIRUS.
It is a shortcoming of existing scan APIs that it is not possible
to return an error without masking a verdict.
We presently work around this limitation by counting up detections at
the end and then overriding the error code with `CL_VIRUS`, if necessary.
The `cl_scanfile_ex()`, `cl_scandesc_ex()`, and `cl_scanmap_ex()` functions
should provide the scan verdict separately from the error code.
This introduces a new enum for recording and reporting a verdict:
`cl_verdict_t` with options:
- `CL_VERDICT_NOTHING_FOUND`
- `CL_VERDICT_TRUSTED`
- `CL_VERDICT_STRONG_INDICATOR`
- `CL_VERDICT_POTENTIALLY_UNWANTED`
Notably, the newer scan APIs may set the verdict to `CL_VERDICT_TRUSTED`
if there is a (hash-based) FP signature for a file, or in the cause where
Authenticode or similar certificate-based verification was performed, or
in the case where an application scan callback returned `CL_VERIFIED`.
CLAM-763
CLAM-865
Add string message to `cl_strerror()` and in example program for
cl_error_t::CL_BREAK enum variant.
Also fix uninitialized memory use issue in example program.
Scripted inputs may be used for automated tests.
Added automated tests for the example program to verify correct behavior
using different callback return codes and also using the new scan layer and
fmap API's.
Fixed a bug in ClamAV's evidence module (recording strong, PUA, and
weak indicators for each layer). Rust HashMaps are unordered so the
feature to get the last alert would return a random alert and not
specifically the last one. Switching to IndexMap resolves this, and
allows us to maintain insertion-order for iterating keys even when
removing a key.
Add test to ensure CVD loading fails with --fips-limits flag unless you
provide the .cvd.sign file.
Also provide .sign files in freshclam tests because they'll be needed to
pass if building in a fips-enabled environment.
In the scan metadata JSON, the result will be like this:
```json
{
"FileName":"headers",
"FilePath":"/home/micah/tmp/20250702_173504-clam-nsis.exe.991ca413f9/clamav-b8d49de1082953f591c02163a128b90b.tmp/nulsft-tmp.1416a98706/headers",
"FileSize":4357,
"ObjectID":2
},
```
Instead of this:
```json
{
"FileName":"/home/micah/tmp/20250702_172320-clam-nsis.exe.1bfb389b2c/clamav-41fa5f1bc556577438b143bc2915f57c.tmp/nulsft-tmp.a851a869ee/headers",
"FilePath":"/home/micah/tmp/20250702_172320-clam-nsis.exe.1bfb389b2c/clamav-41fa5f1bc556577438b143bc2915f57c.tmp/nulsft-tmp.a851a869ee/headers",
"FileSize":4357,
"ObjectID":2
},
```
ClamAV will not function when using a FIPS-enabled OpenSSL 3.x.
This is because ClamAV uses MD5 and SHA1 algorithms for a variety of
purposes including matching for malware detection, matching to prevent
false positives on known-clean files, and for verification of MD5-based
RSA digital signatures for determining CVD (signature database archive)
authenticity.
Interestingly, FIPS had been intentionally bypassed when creating hashes
based whole buffers and whole files (by descriptor or `FILE`-pointer):
78d4a9985a
Note: this bypassed FIPS the 1.x way with:
`EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW);`
It was NOT disabled when using `cl_hash_init()` / `cl_update_hash()` /
`cl_finish_hash()`. That likely worked by coincidence in that the hash
was already calculated most of the time. It certainly would have made
use of those functions if the hash had not been calculated prior:
78d4a9985a/libclamav/matcher.c (L743)
Regardless, bypassing FIPS entirely is not the correct solution.
The FIPS restrictions against using MD5 and SHA1 are valid, particularly
when verifying CVD digital siganatures, but also I think when using a
hash to determine if the file is known-clean (i.e. the "clean cache" and
also MD5-based and SHA1-based FP signatures).
This commit extends the work to bypass FIPS using the newer 3.x method:
`md = EVP_MD_fetch(NULL, alg, "-fips");`
It does this for the legacy `cl_hash*()` functions including
`cl_hash_init()` / `cl_update_hash()` / `cl_finish_hash()`.
It also introduces extended versions that allow the caller to choose if
they want to bypass FIPS:
- `cl_hash_data_ex()`
- `cl_hash_init_ex()`
- `cl_update_hash_ex()`
- `cl_finish_hash_ex()`
- `cl_hash_destroy_ex()`
- `cl_hash_file_fd_ex()`
See the `flags` parameter for each.
Ironically, this commit does NOT use the new functions at this time.
The rational is that ClamAV may need MD5, SHA1, and SHA-256 hashes of
the same files both for determining if the file is malware, and for
determining if the file is clean.
So instead, this commit will do a checks when:
1. Creating a new ClamAV scanning engine. If FIPS-mode enabled, it will
automatically toggle the "FIPS limits" engine option.
When loading signatures, if the engine "FIPS limits" option is enabled,
then MD5 and SHA1 FP signatures will be skipped.
2. Before verifying a CVD (e.g. also for loading, unpacking when
verification enabled).
If "FIPS limits" or FIPS-mode are enabled, then the legacy MD5-based RSA
method is disabled.
Note: This commit also refactors the interface for `cl_cvdverify_ex()`
and `cl_cvdunpack_ex()` so they take a `flags` parameters, rather than a
single `bool`. As these functions are new in this version, it does not
break the ABI.
The cache was already switched to use SHA2-256, so that's not a concern
for checking FIPS-mode / FIPS limits options.
This adds an option for `freshclam.conf` and `clamd.conf`:
FIPSCryptoHashLimits yes
And an equivalent command-line option for `clamscan` and `sigtool`:
--fips-limits
You may programmatically enable FIPS-limits for a ClamAV engine like this:
```C
cl_engine_set_num(engine, CL_ENGINE_FIPS_LIMITS, 1);
```
CLAM-2792
The ClamScan scan summary prints bytes scanned and bytes read in
multiples of 4096 (aka `CL_COUNT_PRECISION`), as is provided by the
`cl_scanfile()`, `cl_scandesc()`, `cl_scanfile_callback()`, and
`cl_scandesc_callback()` functions.
I believe this imprecision was the result of using an `unsigned long int`
which may be 64bit or 32bit, depending on platform. I believe the
intention was to be able to support scanning more than 4 GiB of data.
Since the new `cl_scan*_ex()` functions use a `uint64_t`, which
guarantees a 64bit integer and supports ~16,777,216 terabytes, I find no
reason not to report an accurate count.
For the legacy scan functions (above) I've kept the `CL_COUNT_PRECISION`
behavior to maintain backwards compatibility.
I have also improved the bytes scanned/read output to report GiB, MiB,
KiB, or B as appropriate. Previously, it always report "MB".
CLAM-1433
Add the following scan callbacks:
```c
cl_engine_set_scan_callback(engine, &pre_hash_callback, CL_SCAN_CALLBACK_PRE_HASH);
cl_engine_set_scan_callback(engine, &pre_scan_callback, CL_SCAN_CALLBACK_PRE_SCAN);
cl_engine_set_scan_callback(engine, &post_scan_callback, CL_SCAN_CALLBACK_POST_SCAN);
cl_engine_set_scan_callback(engine, &alert_callback, CL_SCAN_CALLBACK_ALERT);
cl_engine_set_scan_callback(engine, &file_type_callback, CL_SCAN_CALLBACK_FILE_TYPE);
```
Each callback may alter scan behavior using the following return codes:
* CL_BREAK
Scan aborted by callback (the rest of the scan is skipped).
This does not mark the file as clean or infected, it just skips the rest of the scan.
* CL_SUCCESS / CL_CLEAN
File scan will continue.
This is different than CL_VERIFIED because it does not affect prior or future alerts.
Return CL_VERIFIED instead if you want to remove prior alerts for this layer and skip
the rest of the scan for this layer.
* CL_VIRUS
This means you don't trust the file. A new alert will be added.
For CL_SCAN_CALLBACK_ALERT: Means you agree with the alert (no extra alert needed).
* CL_VERIFIED
Layer explicitly trusted by the callback and previous alerts removed FOR THIS layer.
You might want to do this if you trust the hash or verified a digital signature.
The rest of the scan will be skipped FOR THIS layer.
For contained files, this does NOT mean that the parent or adjacent layers are trusted.
Each callback is given a pointer to the current scan layer from which
they can get previous layers, can get the the layer's fmap, and then
various attributes of the layer and of the fmap such as:
- layer recursion level
- layer object id
- layer file type
- layer attributes (was decerypted, normalized, embedded, or re-typed)
- layer last alert
- fmap name
- fmap hash (md5, sha1, or sha2-256)
- fmap data (pointer and size)
- fmap file descriptor, if any (fd, offset, size)
- fmap filepath, if any (filepath, offset, size)
To make this possible, this commits introduced a handful of new APIs to
query scan-layer details and fmap details:
- `cl_error_t cl_fmap_set_name(cl_fmap_t *map, const char *name);`
- `cl_error_t cl_fmap_get_name(cl_fmap_t *map, const char **name_out);`
- `cl_error_t cl_fmap_set_path(cl_fmap_t *map, const char *path);`
- `cl_error_t cl_fmap_get_path(cl_fmap_t *map, const char **path_out, size_t *offset_out, size_t *len_out);`
- `cl_error_t cl_fmap_get_fd(const cl_fmap_t *map, int *fd_out, size_t *offset_out, size_t *len_out);`
- `cl_error_t cl_fmap_get_size(const cl_fmap_t *map, size_t *size_out);`
- `cl_error_t cl_fmap_set_hash(const cl_fmap_t *map, const char *hash_alg, char hash);`
- `cl_error_t cl_fmap_have_hash(const cl_fmap_t *map, const char *hash_alg, bool *have_hash_out);`
- `cl_error_t cl_fmap_will_need_hash_later(const cl_fmap_t *map, const char *hash_alg);`
- `cl_error_t cl_fmap_get_hash(const cl_fmap_t *map, const char *hash_alg, const char **hash_out);`
- `cl_error_t cl_fmap_get_data(const cl_fmap_t *map, size_t offset, size_t len, const uint8_t **data_out, size_t *data_len_out);`
- `cl_error_t cl_scan_layer_get_fmap(cl_scan_layer_t *layer, cl_fmap_t **fmap_out);`
- `cl_error_t cl_scan_layer_get_parent_layer(cl_scan_layer_t *layer, cl_scan_layer_t **parent_layer_out);`
- `cl_error_t cl_scan_layer_get_type(cl_scan_layer_t *layer, const char **type_out);`
- `cl_error_t cl_scan_layer_get_recursion_level(cl_scan_layer_t *layer, uint32_t *recursion_level_out);`
- `cl_error_t cl_scan_layer_get_object_id(cl_scan_layer_t *layer, uint64_t *object_id_out);`
- `cl_error_t cl_scan_layer_get_last_alert(cl_scan_layer_t *layer, const char **alert_name_out);`
- `cl_error_t cl_scan_layer_get_attributes(cl_scan_layer_t *layer, uint32_t *attributes_out);`
This commit deprecates but does not remove the existing scan callbacks:
- `void cl_engine_set_clcb_pre_cache(struct cl_engine *engine, clcb_pre_cache callback);`
- `void cl_engine_set_clcb_file_inspection(struct cl_engine *engine, clcb_file_inspection callback);`
- `void cl_engine_set_clcb_pre_scan(struct cl_engine *engine, clcb_pre_scan callback);`
- `void cl_engine_set_clcb_post_scan(struct cl_engine *engine, clcb_post_scan callback);`
- `void cl_engine_set_clcb_virus_found(struct cl_engine *engine, clcb_virus_found callback);`
- `void cl_engine_set_clcb_hash(struct cl_engine *engine, clcb_hash callback);`
This commit also adds an interactive test program to demonstrate the callbacks.
See: `examples/ex_scan_callbacks.c`
CLAM-255
CLAM-2485
CLAM-2626
Scans containing embedded files (e.g. appended ZIP archives) are not
running the callback functions (pre-scan, pre-cache, inspection, etc.)
on the embedded files.
Run the cli_magic_scan() function for every file we extract using
embedded file type recognition.
CLAM-2796
File-type scanning for OOXML-based office documents results in
double-extraction, because we also extract OOXML document components
in the appopriate parser modules.
CLAM-1412
Adds the following ClamScan CLI options:
* --hash-hint
The file hash so that libclamav does not need to calculate it.
The type of hash must match the '--hash-alg'.
* --log-hash
Print the file hash after each file scanned.
The type of hash printed will match the '--hash-alg'.
* --hash-alg
The hashing algorithm used for either '--hash-hint' or '--log-hash'.
Supported algorithms are 'md5', 'sha1', 'sha2-256'.
If not specified, the default is 'sha2-256'.
* --file-type-hint
The file type hint so that libclamav can optimize scanning.
E.g. 'pe', 'elf', 'zip', etc.
You may also use ClamAV type names such as 'CL_TYPE_PE'.
ClamAV will ignore the hint if it is not familiar with the specified type.
See also: https://docs.clamav.net/appendix/FileTypes.html#file-types
* --log-file-type
Print the file type after each file scanned.
Will NOT be adding this for ClamDScan, as we don't have a mechanism
in the ClamD socket API to receive scan options or a way for ClamD
to include scan metadata in the response.
Add `cl_scanfile_ex()`, `cl_scanmap_ex()`, and `cl_scandesc_ex()`
functions that provide the following additional parameters:
hash_hint (Optional) A NULL terminated string of the file hash so that
libclamav does not need to calculate it.
[out] hash_out (Optional) A NULL terminated string of the file hash.
The caller is responsible for freeing the string.
hash_alg The hashing algorithm used for either `hash_hint` or `hash_out`.
Supported algorithms are "md5", "sha1", "sha2-256".
If not specified, the default is "sha2-256".
file_type_hint (Optional) A NULL terminated string of the file type hint.
E.g. "pe", "elf", "zip", etc.
You may also use ClamAV type names such as "CL_TYPE_PE".
ClamAV will ignore the hint if it is not familiar with the specified type.
See also: https://docs.clamav.net/appendix/FileTypes.html#file-types
file_type_out (Optional) A NULL terminated string of the file type
of the top layer as determined by ClamAV.
Will take the form of the standard ClamAV file type format. E.g. "CL_TYPE_PE".
See also: https://docs.clamav.net/appendix/FileTypes.html#file-types
CLAM-2626
Temp directory recursion in ClamAV is when each layer of a scan gets its
own temp directory in the parent layer's temp directory.
In addition to temp directory recursion, ClamAV has been creating a new
subdirectory for each file scan as a risk-adverse method to ensure
no temporary file leaks fill up the disk.
Creating a directory is relatively slow on Windows in particular if
scanning a lot of very small files.
This commit:
1. Separates the temp directory recursion feature from the leave-temps
feature so that libclamav can leave temp files without making
subdirectories for each file scanned.
2. Makes it so that when temp directory recursion is off, libclamav
will just use the configure temp directory for all files.
The new option to enable temp directory recursion is for libclamav-only
at this time. It is off by default, and you can enable it like this:
```c
cl_engine_set_num(engine, CL_ENGINE_TMPDIR_RECURSION, 1);
```
For the `clamscan` and `clamd` programs, temp directory recursion will
be enabled when `--leave-temps` / `LeaveTemporaryFiles` is enabled.
The difference is that when disabled, it will return to using the
configured temp directory without making a subdirectory for each file
scanned, so as to improve scan performance for small files, mostly on
Windows.
Under the hood, this commit also:
1. Cleans up how we keep track of tmpdirs for each layer.
The goal here is to align how we keep track of layer-specific stuff
using the scan_layer structure.
2. Cleans up how we record metadata JSON for embedded files.
Note: Embedded files being different from Contained files, as they
are extracted not with a parser, but by finding them with
file type magic signatures.
CLAM-1583
Move recording of evidence (aka Strong, PUA, and Weak indicators) to be
done in each layer of a scan, and passed up to the parent layer with the
top level only connecting the results at the very end of the scan.
This is needed to provide access the last alert for a given layer when
we upgrade the scan callbacks.
Note that when adding evidence from a child layer that is a normalized
layer, we do not want to increase the depth. It should appear as though
the match occured on the parent layer.
This is for two reasons:
1. We don't run the scan callbacks on normalized layers.
2. Future matches on Weak Indicators should be able to treat normalized
layer matches the same as original file matches. Keep reading for
more about Weak Indicators.
Recording scan matches at each recursion layer is also needed to support
Weak Indicators, a feature where an alerting signature (aka Strong
Indicator) may require the the match of a non-alerting signature (aka
Weak Indicator) on the same layer or on child layers in order to alert.
Support for Weak indicators was blocked by not keeping track of where
indicators were found. So this commit also enables support for recording
Weak indicators.
Like PUA, Weak indicators are treated differently based on the signature
prefix. That is, any signatures starting with "Weak." won't cause an
alert on its own.
The next step to completing Weak Indicator support will be adding a
logical subsignature feature to depend on a weak indicator match.
CLAM-2626
CLAM-2485
There is a check which makes it so this feature only records URIs when
the HTML file is at the top level. E.g. if you zip an HTML file, then
a scan of the ZIP will no longer record URIs for the HTML within the
ZIP.
I think this is a mistake and I have removed that check.
Every time we push a new map onto the scanning recursion context, give
it a unique object id number, which counts from zero.
Moved the location where we add metadata for each file from the
"cli_magic_scan" function over to the "recursion stack push" function.
Include a "path" as a parameter for creating a new fmap, and rename some
related variables and functions to be more intuitive.
CLAM-2796
See also: CLAM-2485, CLAM-2626
Change the clean-cache to use SHA2-256 instead of MD5.
Note that all references are changed to specify "SHA2-256" now instead
of "SHA256", for clarity. But there is no plan to add support for SHA3
algorithms at this time.
Significant code cleanup. E.g.:
- Implemented goto-done error handling.
- Used `uint8_t *` instead of `unsigned char *`.
- Use `bool` for boolean checks, rather than `int.
- Used `#defines` instead of magic numbers.
- Removed duplicate `#defines` for things like hash length.
Add new option to calculate and record additional hash types when the
"generate metadata JSON" feature is enabled:
- libclamav option: `CL_SCAN_GENERAL_STORE_EXTRA_HASHES`
- clamscan option: `--json-store-extra-hashes` (default off)
- clamd.conf option: `JsonStoreExtraHashes` (default 'no')
Renamed the sigtool option `--sha256` to `--sha2-256`.
The original option is still functional, but is deprecated.
For the "generate metadata JSON" feature, the file hash is now stored as
"sha2-256" instead of "FileMD5". If you enable the "extra hashes" option,
then it will also record "md5" and "sha1".
Deprecate and disable the internal "SHA collect" feature.
This option had been hidden behind C #ifdef checks for an option that
wasn't exposed through CMake, so it was basically unavailable anyways.
Changes to calculate file hashes when they're needed and no sooner.
For the FP feature in the matcher module, I have mimiced the
optimization in the FMAP scan routine which makes it so that it can
calculate multiple hashes in a single pass of the file.
The `HandlerType` feature stores a hash of the file in the scan ctx to
prevent retyping the exact same data more than once.
I removed that hash field and replaced it with an attribute flag that is
applied to the new recursion stack layer when retyping a file.
This also closes a minor bug that would prevent retyping a file with an
all-zero hash. :)
The work upgrading cache.c to support SHA2-256 sized hashes thanks to:
https://github.com/m-sola
CLAM-255
CLAM-1858
CLAM-1859
CLAM-1860
I accidentally introduced a NULL-dereference bug when scanning any OOXML
file in https://github.com/Cisco-Talos/clamav/pull/1548
I overlooked the test failure out of haste. 😔
The NULL-dereference happens because the `unzip_search()` feature
allowed searching some other file than the one that is currently being
scanned, which you would do by setting `ctx` to NULL and setting an
`fmap` parameter instead.
In practice, the current layer's `fmap` from the `ctx` was always passed in.
This fix makes it so the `unzip_search()` and related functions only
take the `ctx` parameter and do not have and `fmap` or `fsize` field
(Note: the `fsize` was never needed, because `fmap->len` take care of that).
CLAM-2837
ClamAV supports multiple platforms and sometimes requires the
debugging of platform-specific bugs.
This commit adds one devcontainer for Debian and another for
AlmaLinux so that the codebase can be reopened inside of a
devcontainer for debugging/testing purposes.
Jira: CLAM-2740
---------
Signed-off-by: John Humlick <15677335+jhumlick@users.noreply.github.com>
On Windows, the cli_basename function should treat both '/' and '\' as path
separators. Most Windows APIs also accept both.
On Linux/Unix, it makes sense when using a filepath that is more for
informational purposes or where it may have come from a Windows system,
to treat the '\' as a path separator.
But in situations where the the path is needed for some critical action,
like moving or deleting a file, we can't treat it as a path separator.
An infinite loop may occur when scanning some malformed ZIP files.
I introduced this issue in 96c00b6d80
with this line:
```c
// decrement coff by 1 to account for the increment at the end of the loop
coff -= 1;
```
The problem is that the function may return 0, which should
indicate that there are no more files. The result was that
`coff` would stay the same and the loop would repeat.
This issue is in 1.5 development and affects the 1.5.0 beta but
does not affect any production versions.
Fixes: https://github.com/Cisco-Talos/clamav/issues/1534
Special thanks to Sophie0x2E for an initial fix, proposed in
https://github.com/Cisco-Talos/clamav/pull/1539
In review, I was uncomfortable with other existing code and
decided to to a more significant overhaul of the error handling
in the ZIP module.
In addition to cleanup, this commit has some functional changes:
- When parsing a central directory file header inside of
`parse_central_directory_file_header()`, it will now fail out if the
"extra length" or "comment length" fields would exceced the length of
the archive. That doesn't mean the associated local file header won't
be parsed later, but it won't use the central directory file header
to find it. Instead, the ZIP module will have to find the local file
header by searching for extra records not listed in the central directory.
This change was mostly to tidy up complex error handling.
- Add two FTM new signatures to identify split ZIP archives.
This signature identifies the first segment (first file) in a split or
spanned ZIP archive. It may also be found on a single-segment "split"
archive, depending on the ZIP archiver.
```
0:0:504b0708504b0304:ZIP (First segment split/spanned):CL_TYPE_ANY:CL_TYPE_ZIP
```
Practically speaking, this new signature makes it so ClamAV identifies
the file as a ZIP right away without having to rely on SFX_ZIP detection.
Extraction is then handled by the ZIP `cli_unzip` function rather than
extracting each with `cli_unzip_single` which handles SFX_ZIP entries.
Note: ClamAV isn't capable of finding additional files on disk to support
handling the additional segments. So it doesn't make any difference with
handling those other files.
This signature is for single-segment split/spanned archives, depending
on the ZIP archiver.
```
0:0:504b0303504b0304:ZIP (Single-segment split/spanned):CL_TYPE_ANY:CL_TYPE_ZIP
```
Like the first one, this also means we won't rely on SFX_ZIP detection
and will treat this files as regular ZIPs.
- Added a test file to verify that ClamAV can extract a single-file
"split" ZIP.
- Added a clamscan test with test files to verify that scanning a split
archive across two segments correctly extracts the properly formed zip
file entries. Sadly, we can't join the segments to extract everything.
The new Sigtool feature tests are failing on Windows because CMake is
providing the CVD_CERTS_DIR environment variable with '/' path separators.
My fix is to convert '/' to '\\' when converting paths to UNC paths.
I have also changed CMake to provide CVD_CERTS_DIR with native paths.
I also made a couple of fixes to the Sigtool tests because Windows wants
to make CRLF line endings in Python unless you write byte-strings and
Sigtool will get upset when making a diff if a signature file contains
CRLF line endings.
And putting Windows path separators into 'verify_output()' for the tests
gets confused because it is actually a regex string and the backslashes
mess it up.
