290424 Missing break in switch - In hash_match: Missing break
statement between cases in switch statement
290414 Resource leak - In cli_scanishield_msi: Leak of memory or
pointers to system resources. Memory leak in a fail case
288197 Resource leak - In decrypt_any: Leak of memory or pointers
to system resources. Memory leak in a fail case
290426 Resource leak - In cli_magic_scan: Leak of memory or pointers
to system resources. Leaked a file prefix when running with
--save-temps
192923 Resource leak - In cli_scanrar: Leak of memory or pointers to
system resources. Leaked a file descriptor if a virus was found in
a RAR file comment
225146 Resource leak - In cli_scanegg: Leak of memory or pointers
to system resources. Leaked a file descriptor if unable to write
a comment file to disk
290425 Resource leak - In scan_common: Leak of memory or pointers
to system resources. Memory leaks in various fail cases.
Also changes cli_scanrar to write out the file comment only if
--leave-temps is specified and scan the buffer (like what is done
in cli_scanegg) instead of writing the file out, scanning that,
and then deleting the file if --leave-temps is not specified.
The unit tests stopped working when correcting an issue with a
switch statement that determined what type of signature had matched
on a Google SafeBrowsing GDB rule. Looking into the unit tests, it
looks like the code had always assumed that the test cases would be
detected by a malware test rule in unit_tests/input/daily.gdb, but
now some of the tests get matched on the phishing test rule.
I updated the test logic to be more clear, and added tests for both
cases now.
Fix some memory leaks in libclamav/scanners.c
Reviewing Coverity bug reports we found that the return value to this
filter_search call was effectively being ignored, causing no filtering
to occur. Fixing this issue resulted in a unit test that uses the
following match list regex to fail when searching for `ebay.com`.:
.+\\.paypal\\.(com|de|fr|it)([/?].*)?:.+\\.ebay\\.(at|be|ca|ch|co\\.uk|de|es|fr|ie|in|it|nl|ph|pl|com(\\.(au|cn|hk|my|sg))?)/
After investigating further, this is because the regex_list_add_pattern
call, which parses the regex for suffixes and attempts to add these to
the filter, can't handle the `com(\\.(au|cn|hk|my|sg))?` portion of
the regex. As a result, it only adds `ebay.at`, `ebay.be`, `ebay.ca`, up
through `ebay.pl` into the filter). With the code returning if no filter match
is found, the `ebay.com` suffix not existing in the filter causes incoming URLs
to be treated as if there are no corresponding regexes for ebay.com, which results
in no regex rules being evaluated against it.
We should get the regex parsing code working (and ensure it handles any
other complex cases in daily.cdb) before re-enabling this code. The code
has had no effect for 12+ years at this point, though, so it's probably
safe to wait a bit longer without it.
Fixed the following Coverity issues:
- 225236 - In cli_egg_extract_file: Dereference of an explicit
null value (CWE-476). The first fail case checked handle for
NULL and then dereferenced it in the done block
- 225209 - In executeIfNewVersion: Leak of memory or pointers
to system resources (CWE-404). modifiedCommand was defined
twice, with the inner instance being assigned to and the
outer instance being freed
- 225201 - In regex_list_match: Code can never be reached
because of a logical contradiction (CWE-561). The code had
logic off to the side that may have been missed:
filter_search_rc = filter_search(&matcher->filter, (const unsigned char *)bufrev, buffer_len) != -1;
if (filter_search_rc == -1) {
- 225198 - In phishingCheck: Leak of memory or pointers to
system resources (CWE-404). A fail case caused by malloc
failing would leak previously allocated memory.
- 225197 - In updatecustomdb: A pointer to freed memory
is dereferenced, used as a function argument, or otherwise
used (CWE-416). In a fail case, a pointer was freed and
then used in a debug print statement
- 225190 - In updatedb: A pointer to freed memory is
dereferenced, used as a function argument, or otherwise used
(CWE-416). In a fail case, a pointer was freed and then used
in a debug print statement
- 225195 - In cli_egg_open: The sizeof operator is used on a
wrong argument that incidentally has the same size (CWE-467).
sizeof(char **) was being used instead of sizeof(char *)
- 225193 - In egg_parse_comment_header: Code can never be
reached because of a logical contradiction (CWE-561).
A cleanup case for variable comment was unnecessary, and
to fix comment was removed entirely.
- 225147 - In get_server_node: Code can never be reached
because of a logical contradiction (CWE-561). A cleanup
case for variable url was unnecessary
- 225168 - In download_complete_callback: Missing break
statement between cases in switch statement (CWE-484).
In the case where forking failed, freshclam would check
the database without forking but then continue on to
execute the code intended to be done in the child process
because of a missing break statement
- 225152 - In cli_egg_lzma_decompress: Use of an
uninitialized variable (CWE-457). Certain fail cases
would call cli_LzmaShutdown on an uninitialized stream.
Now it’s only called after initialization occurs.
Some detections, like phishing, are considered heuristic alerts because
they match based on behavior more than on content. A subset of these
are considered "potentially unwanted" (low-severity). These
low-severity alerts include:
- phishing
- PDFs with obfuscated object names
- bytecode signature alerts that start with "BC.Heuristics"
The concept is that unless you enable "heuristic precedence" (a method
of lowing the threshold to immediateley alert on low-severity
detections), the scan should continue after a match in case a higher
severity match is found. Only at the end will it print the low-severity
match if nothing else was found.
The current implementation is buggy though. Scanning of archives does
not correctly bail out for the entire archive if one email contains a
phishing link. Instead, it sets the "heuristic found" flag then and
alerts for every subsequent file in the archive because it doesn't know
if the heuristic was found in an embedded file or the target file.
Because it's just a heuristic and the status is "clean", it keeps
scanning.
This patch corrects the behavior by checking if a low-severity alerts
were found at the end of scanning the target file, instead of at the end
of each embedded file.
Additionally, this patch fixes an in issue with phishing alerts wherein
heuristic precedence mode did not cause a scan to stop after the first
alert.
The above changes required restructuring to create an fmap inside of
cl_scandesc_callback() so that scan_common() could be modified to
require an fmap and set up so that the current *ctx->fmap pointer is
never NULL when scan_common() evaluates match results.
Also fixed a couple minor bugs in the phishing unit tests and cleaned up
the test code for improved legitibility and type safety.
and relative offsets into BM matcher (load other ones into AC) and
use per-file computed offset table to pick up best shifts (not
enabled by default, bb#1300)
