When switching to openssl 3.x, linking with clamsubmit fails with
undefined openssl symbols. The error message from Xcode is crazy obtuse:
ld: initializer '_OPENSSL_cpuid_setup' is >4GB from start of image in 'anon' from /Users/clamav_jenkins_svc/clamav-mussels-cookbook/test/install-x86_64/lib/libcrypto.a(libcrypto-lib-x86_64cpuid.o)
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Anyhow... It seems that we must explicitly link clamsubmit with openssl
now in order for this to work.
In addition to this change, I also found that the CMake FindRust.cmake
module breaks the ability to build specifically for just x86_64 or arm64
(i.e. possibly cross-compiling.
This commit includes a change to accommodate that scenario.
If a signature has a pattern that is too short will fail to load the
signature but does not cause the entire load process to abort.
This is bad for two reasons:
1) It is not immediately apparent that the signature is bad, and so it
could be published accidentally.
2) The signature is partially loaded by the time the bad pattern is
observed and that may cause a crash later.
Because of (1), it is not worth it to try to unload the first part of the
signature. Instead, we should just abort the signature load.
Fixes: https://github.com/Cisco-Talos/clamav/issues/923
We should also abort loading if the filter pattern for the boyer-moore
matcher is shorter than 2 bytes.
Also, do not print the final "Loading" progress bar if an error occurred.
A buffer over-read may occur when unpacking wwpack'd PE files if the
file is very small.
This commit adds a CLI_CONTAINS buffer wrap check to ensure we aren't
reading beyond the exe buffer.
We determined that this issue is not a vulnerability.
Resolves: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57374
A warning printing the HTTP code and file size was accidentally
committed at the end of ClamAV 1.1.0 dev when fixing a bug.
Remove this warning.
Resolves: https://github.com/Cisco-Talos/clamav/issues/930
The clamd and clamav-milter `--help` message and manpages do
not mention the `--pid` (`-p`) option.
The clamd `--help` message and manpage do not mention the
`--datadir` option.
Also corrected minor punctuation issues, and removed the meaningless
jargon about the "main thread" which has nothing to do with the PID.
* Add new clamd and clamscan option --cache-size
This option allows you to set the number of entries the cache can store.
Additionally, introduce CacheSize as a clamd.conf
synonym for --cache-size.
Fixes#867
I'm unsure why, but building with cmke -D MAINTAINER_MODE=ON is failing
right now. Updating to a newer version of bindgen appears to resolve the
issue.
I was able to update it by changing the version specified in
libclamav_rust/Cargo.toml, and then running `cargo update -p bindgen`
Not that I expect anyone else to be running maintainer-mode, but I did
also confirm using `cargo-msrv` that the minimum supported version of
rust did not change as a result of this commit.
common.rc is generated by CMake from common.rc.in.
But we do need to have it generate in the same directory as the other
resource files.
We simply forgot to remove common.rc after removing the Visual Studio
project files.
When processing UTF-8 HTML code, the image extraction logic may panic if
the string contains a multi-byte grapheme that includes a '(', ')',
whitespace, or one of the other characters used to split the text when
searching for the base64 image content.
The panic is because the `split_at()` method will panic if you try to
split in the middle of a unicode grapheme.
This commit fixes the issue by processing the HTML string one grapheme
at a time instead of one character (byte) at a time.
The `grapheme_indices()` method is used to get the correct position of
the start of each grapheme for splitting the string.
The code to extract CSS from HTML <style> blocks contains an off by one
in case there is no actual content it will have a chunk_size of -1.
Whoops.
Removed the -1 so it is correct, and added an extra safety check in case
something else crazy happens.
The intention behind adding `cargo update` in our Jenkinsfile earlier
was to make it so the local cache of the cargo registry index (i.e.
$HOME/.cargo/registry/index) knows about the latest version of the Rust
libraries required by our Cargo.lock file.
As it turns out, `cargo update` modifies the Cargo.lock file, which
was not intended.
This change makes removes use of `cargo update`.
It appears the real issue is that we needed a newer version of Rust to
build the 'image' crate.
For some reason we're generating a filename wiith a random hash in it
to use for the comment content in the event that codepage converstion to
utf8 fails for the comment. This makes no sense. So I'm removing it and
letting it just fail out. The calling functions ignore the failure
anyways and move on which is good.
Note: I think the "cli_genfname" call that I'm removing was a copypaste
from the logic for converting the filename to utf8. We still do that.
I'm not sure about the consequence of failing to have a filename in that
case, so I'm going to leave it as-is.
Coverity-225186, 225156: Fix possible leak of comment message in case
parsing the comment header fails after allocating the comment buffer.
Coverity-225184: Fix possible leak of egg block if the archive is not
solid and contains no files.
Additional improvements to egg parser error handling for functions that
pass back allocated memory through the parameters. Instead of checking
for failure before freeing the allocated memory, we'll hand off
ownership of the allocated memory to the parameter variable by setting
to NULL afterwards, and then always free the variable if not NULL after
the `done` label.
Coverity is unhappy with the use of the EC32, cli_readint32,
and cli_writeint32 macros (and the 64bit equivalents to potentially
change the endianess of variables in place.
It claims:
overlapping_assignment: Assigning ... to ..., which have overlapping
memory locations and different types.
Using a temporary variable in between reading and writing should
resolve these "high impact" complaints.
Resolves: Coverity-225232. 225225, 225215, 225212, 225180, 225170,
225165, 225161, 225159.
Coverity-344510: Fix unitialized sock variable in check_clamd test
program. Only close the socket on error if is a valid file descriptor.
Coverity-344507: Remove unused file-open from clamd test.
Coverity-344497: clamd test recvpartial convenience function is was
reusing the `len` variable used for "how long is the reply" also as
the buffer length. Coverity appears to be confused here and thinks that
the length of the buffer may not be long enough for the NULL terminating
character. I have reworked this to use a separate variable for managing
the length of the buffer.
Prevent double-extraction of same PDF object
Two indirect references to the same PDF object may cause it to try to
extract that object twice. This also may cause it to set the extraction
path twice, which leaks the memory from the first time.
This commit records when object extraction is attempted and prevents
doing it again. It also adds a couple extra checks to make sure that the
object path string is not leaked.
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58072
Also:
- Coverity-317959: Fix complaint about logically dead code. No need to
check if UE variable is NULL because we would've returned earlier if it
was NULL.
- A bunch of medium-severity coverity issues for PDF parser regarding
checking if a `pdf` pointer is NULL after dereferencing it.
- Coverity-192930: bytes_remaining was being checked twice in a row
without chainging it. Turns out we should have been changing it after
moving the `index` pointer.
- Coverity-192920: Switch to use CLI_REALLOC instead of cli_realloc2.
This is because cli_realloc2 would free `pdf->objs` on failure and we
still need it.
Fix possibly unitialized binop variable in bytecode module for STORE
and COPY instructions in bytecode module.
Refactored slightly to include additional opcode login in the switch statement.
Coverity-344508: Fix out-of-bound read in check_str test.
The len argument cannot be longer than the size of the source buffer.
The original test was attempting to test an append failure.
The updated test checks for correct behavior with two consecutive
appends.
Also added function comments to document correct use of textbuffer
functions.
Coverity-344493: Fix out-of-bounds read in check_jsnorm test.
The buffers passed to tokenizer_test must be NULL-terminated.
RTF:
- Coverity-344490: Use cli_realloc instead of cli_realloc2.
cli_realloc2 will free the memory if the allocation fails, though we
also free the memory later in SCAN_CLEANUP.
- Fix warning about unused variable.
AutoIt:
- Fix possible memory leaks of input and output buffers.
- Set pointer to NULL after handing off memory to new pointer.
The pointer used to index an HTML file during normalization may be
rewound to an earlier location if encoded javascript (screnc) is
detected while processing the line.
If a <style>-tag was also found in the line after the screnc bytes
then the check for the size of the style-chunk will be "negative"
and would result in a massive memcpy.
This issue was introduced during 1.1 development.
This commit ensures the style chunk size may not be negative.
Resolves: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57327
Somehow the LOGG_DEBUG-level message used to verify correct behavior
in one of the freshclam tests is not written to stdout when testing
on some Fedora systems.
This commit changes the test to verify correct behavior by checking
for a different message that is LOGG_INFO-level, and is consistently
written to stdout.
Further investigation required to find out why debug messages aren't
always written to stdout.
Previous behaviour would remove temp files by deleting the subdirectory
This caused issues in cases (on Windows) where subdirectories aren't created
due to performance concerns
This commit removes tempfiles individually if keeptemp is off
Original patch authored by Thomas Vy
The CLOSE command is failing to create a file when appending changes if
the file does not already exist. This prevents adding new files to a
database with a CDIFF and caused failures applying the test-3.cdiff file
in the freshclam feature tests.
Also improved the error message to show which command, specifically, is
failing (not just the line number).
Any cdiff or script using the UNLINK operation will fail to delete the
file claiming "No DB open for action UNLINK".
The UNLINK operation appears to be trying to delete a currently open
database, when in fact it should ensure no database is open before
deleting the local file given by the single "db_name" parameter.
Some log statements using the old ^, !, and * logg-prefix where they
were making use a ternary to determine the log level in the log
statement.
Also sigtool and freshclam weren't outputting error log messages using
the Rust log macros e.g. `error!("...")`.
Also includes:
- A sigtool test to verify that Rust log macros are working.
- Changing the freshclam tests to use --no-dns so they run faster
when DNS isn't working (e.g. no internet).
In the event that there is an issue with the CDIFF process, freshclam is
treating it as thought no patch was downloaded.
If freshclam fails to apply the patch because of an issue with the
patch, or some bug in the CDIFF module, it should retry for the whole
CVD.
Coverity complained about missing break statements for two switch cases
that end with asserts.
Adding /* fall-through */ comments appears to assuage Coverity's fears.
The strncpy intentionally is not copying the NULL terminator for the log
message prefix. The NULL will be added by vsnprintf, after.
Switching to memcpy eliminates the warning.
Micah Snyder