As of ClamAV 0.105, libjson-c is required.
There is also no option to disable libjson-c support.
This commit removes the dead code associated with the old build
option.
As of ClamAV 0.105, libz is required.
There is also no option to disable zlib support.
This commit removes the dead code associated with the old build
option.
As of ClamAV 0.105, libbz2 is required.
There is also no option to disable bz2 support.
This commit removes the dead code associated with the old build
option.
As of ClamAV 0.105, libxml2 is required.
There is also no option to disable PCRE support.
This commit removes the dead code associated with the old build
option.
As of ClamAV 0.105, PCRE2 is required. PCRE (1) is not an option, and
there is also no option to disable PCRE support.
This commit removes the dead code associated with those old build
options.
The in_iconv_u16() function resolves "alignment" issues where the length
of the input string is not mod(4). The solution trims the extra bytes
off the input string. If the input string is total less than 4 bytes,
then those extra bytes are put in a 4-byte array and are converted.
However, if the input string is longer, then those extra bytes are lost.
This fix saves the extra "unaligned" bytes in the 4-byte array and
converts them afterwards so we don't accidentally lose 1 to 2
characters.
We have 2 new variations on the statx false positive,
this time from calls from the Rust code.
I'm removing the mangled symbol to simplify the rule and accomodate
future variations.
The delharc crate used to add LZH archive support appears to add
a dependency on macOS CoreFoundation library.
The error is:
[ 78%] Linking C shared library libclamav.dylib
Undefined symbols for architecture x86_64:
"_CFRelease", referenced from:
iana_time_zone::platform::get_timezone_inner::hc7da204717a39974 in libclamav_rust.a(iana_time_zone-bc4762a47da73d72.iana_time_zone.1863eb20d202562a-cgu.0.rcgu.o)
...
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [libclamav/libclamav.12.0.2.dylib] Error 1
We already link with CoreFoundation for libfreshclam and clamsubmit, so
this commit extends that to libclamav as well.
Primarily this commit fixes an issue with the size of the parameters
passed to cli_checklimits(). The parameters were "unsigned long", which
varies in size depending on platform.
I've switched them to uint64_t / u64.
While working on this, I observed some concerning warnigns on Windows,
and some less serious ones, primarily regarding inconsistencies with
`const` parameters.
Finally, in `scanmem.c`, there is a warning regarding use of `wchar_t *`
with `GetModuleFileNameEx()` instead of `GetModuleFileNameExW()`.
This made me realize this code assumes we're not defining `UNICODE`,
which would have such macros use the 'A' variant.
I have fixed it the best I can, although I'm still a little
uncomfortable with some of this code that uses `char` or `wchar_t`
instead of TCHAR.
I also remove the `if (GetModuleFileNameEx) {` conditional, because this
macro/function will always be defined. The original code was checking a
function pointer, and so this was a bug when integrating into ClamAV.
Regarding the changes to `rijndael.c`, I found that this module assumes
`unsigned long` == 32bits. It does not.
I have corrected it to use `uint32_t`.
Providing cf-ray ids to the user when freshclam fails will skip the step
of having to ask them for the ids. This should save some time when the
user is not in the same time zone.
We add the _OR_GOTO_DONE suffix to the macros that go to done if the
allocation fails. This makes it obvious what is different about the
macro versus the equivalent function, and that error handling is
built-in.
Renamed the cli_strdup to safer_strdup to make it obvious that it exists
because it is safer than regular strdup. Regular strdup doesn't have the
NULL check before trying to dup, and so may result in a NULL-deref
crash.
Also remove unused STRDUP (_OR_GOTO_DONE) macro, since the one with the
NULL-check is preferred.
Variables like the number of signature parts are considered trusted user input
and so allocations based on those values need not check the memory allocation
limit.
Specifically for the allocation of the normalized buffer in cli_scanscript,
I determined that the size of SCANBUFF is fixed and so safe, and the maxpatlen
comes from the signature load and is therefore also trusted, so we do not
need to check the allocation limit.
Allocations for bytecode signatures to work need not check against the
memory allocation limit, as bytecode signatures are considered trusted
user input.
You may note that I did not remove allocation limits from the bytecode
API functions that may be called by the signatures such as adding json
objects, hashsets, lzma and bz2 decompressors, etc. This is because it
is likely that a bytecode signature may call them more times based on
the structure of the file being scanned - particularly for the json objects.
Bytecode signature's are able to allocate buffers, but should probably
adhere to clamav's max allocation limit. This adds a check to make sure
they don't accidentally alloc too much based on untrusted user input.
The cli_max_malloc, cli_max_calloc, and cli_max_realloc functions
provide a way to protect against allocating too much memory
when the size of the allocation is derived from the untrusted input.
Specifically, we worry about values in the file being scanned being
manipulated to exhaust the RAM and crash the application.
There is no need to check the limits if the size of the allocation
is fixed, or if the size of the allocation is necessary for signature
loading, or the general operation of the applications.
E.g. checking the max-allocation limit for the size of a hash, or
for the size of the scan recursion stack, is a complete waste of
time.
Although we significantly increased the max-allocation limit in
a recent release, it is best not to check an allocation if the
allocation will be safe. It would be a waste of time.
I am also hopeful that if we can reduce the number allocations
that require a limit-check to those that require it for the safe
scan of a file, then eventually we can store the limit in the scan-
context, and make it configurable.
We have some special functions to wrap malloc, calloc, and realloc to
make sure we don't allocate more than some limit, similar to the
max-filesize and max-scansize limits. Our wrappers are really only
needed when allocating memory for scans based on untrusted user input,
where a scan file could have bytes that claim you need to allocate
some ridiculous amount of memory. Right now they're named:
- cli_malloc
- cli_calloc
- cli_realloc
- cli_realloc2
... and these names do not convey their purpose
This commit renames them to:
- cli_max_malloc
- cli_max_calloc
- cli_max_realloc
- cli_max_realloc2
The realloc ones also have an additional feature in that they will not
free your pointer if you try to realloc to 0 bytes. Freeing the memory
is undefined by the C spec, and only done with some realloc
implementations, so this stabilizes on the behavior of not doing that,
which should prevent accidental double-free's.
So for the case where you may want to realloc and do not need to have a
maximum, this commit adds the following functions:
- cli_safer_realloc
- cli_safer_realloc2
These are used for the MPOOL_REALLOC and MPOOL_REALLOC2 macros when
MPOOL is disabled (e.g. because mmap-support is not found), so as to
match the behavior in the mpool_realloc/2 functions that do not make use
of the allocation-limit.
There are a large number of allocations for fix sized buffers using the
`cli_malloc` and `cli_calloc` calls that check if the requested size is
larger than our allocation threshold for allocations based on untrusted
input. These allocations will *always* be higher than the threshold, so
the extra stack frame and check for these calls is a waste of CPU.
This commit replaces needless calls with A -> B:
- cli_malloc -> malloc
- cli_calloc -> calloc
- CLI_MALLOC -> MALLOC
- CLI_CALLOC -> CALLOC
I also noticed that our MPOOL_MALLOC / MPOOL_CALLOC are not limited by
the max-allocation threshold, when MMAP is found/enabled. But the
alternative was set to cli_malloc / cli_calloc when disabled. I changed
those as well.
I didn't change the cli_realloc/2 calls because our version of realloc
not only implements a threshold but also stabilizes the undefined
behavior in realloc to protect against accidental double-free's.
It may be worth implementing a cli_realloc that doesn't have the
threshold built-in, however, so as to allow reallocaitons for things
like buffers for loading signatures, which aren't subject to the same
concern as allocations for scanning possible malware.
There was one case in mbox.c where I changed MALLOC -> CLI_MALLOC,
because it appears to be allocating based on untrusted input.
Use the existing clamscan image fuzzy hash tests to verify that both
--scan-image=no and also --scan-image-fuzzy-hash=no will disable image
fuzzy hash based detection (at least for PNG files).
The --force-to-disk option is missing from the clamscan --help and
clamscan manpage documentation.
Also change clamd.conf.sample suggestions to differ the from default
settings so that the sample is easier to use.
Image fuzzy hashing is enabled by default. The following options have
been added to allow users to disable it, if desired.
New clamscan options:
--scan-image[=yes(*)/no]
--scan-image-fuzzy-hash[=yes(*)/no]
New clamd config options:
ScanImage yes(*)/no
ScanImageFuzzyHash yes(*)/no
New libclamav scan options:
options.parse &= ~CL_SCAN_PARSE_IMAGE;
options.parse &= ~CL_SCAN_PARSE_IMAGE_FUZZY_HASH;
This commit also changes scan behavior to disable image fuzzy hashing
for specific types when the DCONF (.cfg) signatures disable those types.
That is, if DCONF disables the PNG parser, it should not only disable
the CVE/format checker for PNG files, but also disable image fuzzy
hashing for PNG files.
Also adds a DCONF option to disable image fuzzy hashing:
OTHER_CONF_IMAGE_FUZZY_HASH
DCONF allows scanning features to be disabled using a configuration
"signature".
Make the usage clear to the user that the option specified by
'--datadir' must be an absolute path to a directory that already exists,
and is writeable by freshclam and readable by clamscan/clamd.
The '%f' filename format character has been disabled and will no longer
be replaced with the file name, due to command injection security concerns.
Use the 'CLAM_VIRUSEVENT_FILENAME' environment variable instead.
For the same reason, you should NOT use the environment variables in the
command directly, but should use it carefully from your executed script.
The CMake code to install the empty database directory doesn't appear to
do anything. Further, it is causing build issues for some users.
I tested some with trying to install the database directory directly,
like this:
```cmake
if(IS_ABSOLUTE ${DATABASE_DIRECTORY})
INSTALL(DIRECTORY DESTINATION $ENV{DESTDIR}${DATABASE_DIRECTORY} COMPONENT programs)
else()
INSTALL(DIRECTORY DESTINATION $ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}/${DATABASE_DIRECTORY} COMPONENT programs)
endif()
```
However, this causes build failures for Windows and macOS installers.
Windows:
```
CMake Error at C:/Users/build/Documents/jenkins/workspace/test-pipelines_build-1.3/clamav-devel/build64/freshclam/cmake_install.cmake:66 (message):
ABSOLUTE path INSTALL DESTINATION forbidden (by caller):
C:/Users/build/Documents/jenkins/workspace/test-pipelines_build-1.3/clamav-devel/build64/install/database/
Call Stack (most recent call first):
C:/Users/build/Documents/jenkins/workspace/test-pipelines_build-1.3/clamav-devel/build64/cmake_install.cmake:133 (include)
```
macOS:
```
CMake Error at /Users/****/jenkins/workspace/test-pipelines_build-1.3/clamav-devel/build-pkg/freshclam/cmake_install.cmake:111 (file):
file INSTALL cannot make directory "/usr/local/clamav/share/clamav": No
such file or directory.
Call Stack (most recent call first):
/Users/****/jenkins/workspace/test-pipelines_build-1.3/clamav-devel/build-pkg/cmake_install.cmake:122 (include)
CPack Error: Error when generating package: ClamAV
```
I think my best option is just to remove this line of code since it's causing
problems for some and isn't working for anyone. Even if it would be
nice to have the database directory created at install time automatically.
Fixes: https://github.com/Cisco-Talos/clamav/issues/1142
Andy Ragusa