This patch relocates the real-path check from clamdscan and clamonacc
to clamd. While clamonacc is unlikely to send directories or symlinks
to be scanned, clamdscan may send directories. Real-path checks have
to be performed on the files, not the directories -- both because the
directories may contain symlinks and because the cli_realpath()
function wasn't written to support directories on Windows.
Using file type recognition scan mode for disk images and other raw
archive formats is problematic. One simple reason is that the contained
files will be detected and parsed and scanned twice, first when deteced
by the type recog scan, and later when the archive is extracted and the
files are properly scanned. Another reason is an increased likelihood
for incorrect type recognition, as seen with supposed MHTML files (they
weren't) found in GPT disk images.
Though a previous patch disabled embedded type recognition for GPT
files, this one extens this to the following:
- CL_TYPE_CPIO_OLD
- CL_TYPE_ZIP
- CL_TYPE_OLD_TAR
- CL_TYPE_POSIX_TAR
ZIP is included because file entries in a ZIP are incorrectly detected
as ZIPSFX's and though we also ensure not to scan ZIPSFX's found in
ZIP's, it's more efficient not to do the type recognition in the first
place and it prevents us from adding those bogus ZIPSFX entries into the
scan properties JSON.
This patch also fixes what appears to be a copy-paste typo, where
CL_TYPE_ISHIELD_MSI types were accidentally having their container value
set to CL_TYPE_AUTOIT.
Exit early from VBA scanning loop if virus found.
Add VBA/XLM suffix to ContainsMacros heuristics.
Fix setting status code for error and virus conditions.
Increment/decrement recursion counter when scanning vba dir.
Notably the commit adds a heuristic alert when VBA is extracted using
the new VBA extraction code and similarly adds "HasMacros":true to the
JSON scan properties.
In addition, a change was added to the cli_sanitize_filepath() function
so it converts posix pathseps to Windows pathseps on Windows and also
outputs a sanitized basename pointer (optional) which is used when
generating a temporary filename so that using a prefix with pathseps in
it won't cause file creation failures (observed with --leave-temps where
original filenames are incorporated into temporarily filenames).
Included soem error handling improvements for cli_vba_scandir() to
better track alert and macro detections.
Downgraded utf8 conversion error messages to debug messages because they
are too verbose in files with invalid filenames (observed in some
malware).
Changed the xlm macro and vba project temp filenames to include
"xlm_macros" and "vba_project" prefix, to make it easier to find them.
Relocated XLM and VBA temp files from the top-level tmp directory to the
current sub_tmpdir, so tempfiles for a given scan are more organized.
Fix an infinite loop in the new XLM macro parser.
Fix error handling, resource cleanup in OLE2 parser.
Fix issues tracking detected "viruses" in VBA & OLE2 parsers affecting
non-allmatch (regular) scan mode, wherein multiple viruses may be found
but each record lost and the overall detection comes up clean.
Also silence switch() fall-through warning for WORD/PPT/XL/HWP (OOXML)
file type fall-throughs to the ZIP parser (because they are zips).
Also silence switch() fall-through warning when handling the limits-
exceeded error types, checking for the limits-exceeded heuristic, and
continuing on to bail out with a clean verdict.
Changes cli_checkfp_virus to a recursive function which checks all
parent fmaps in the context for false positives
Simplifies params needed for cli_checkfp_virus to use the current digest
and fmap length that resides within the fmap struct itself
Add missing ping_clamd() declaration in client.h
Fix check for ping option to first check if ping option is NULL before
strdup'ing and checking if the alloc failed.
Fix format string for uint64_t print.
Correctly assign name pointer to stack buffer in cpio parser.
Remove vestigial variables from insert_list() function matcher-ac.c,
left over from before the load-time optimizations completely
restructured everything.
Silence warnings about unused parameters in progress bar callback
function.
Valgrind reports uninitialized `tag` stack buffer being used. While this
appears to be a false positive, it can't hurt to initialize this and
similar buffers in this function.
Fixes bound checks in recently rewritten VBA parser code (i.e. issue
does not affect prior versions).
Also improves VBA terminator header parsing to better match the spec,
per recommendation by Jonas Zaddach.
The clamonacc command doesn't present a `--debug` flag, but according
to your blog https://blog.clamav.net/2019/09/understanding-and-transitioning-to.html
the correct flag should be `--verbose`:
"[...]This is akin to clamd’s or clamscan’s --debug option, but isn’t
quite so noisy as either of those. By default, clamonacc does not
print any output after daemonizing, so you will have to pair this
option with --log or --foreground to use it.[...]"
- Updated `SUMHEAD` string macro with corrected alignment of `DBVER`
and `DBTIME` labels to accommodate current (wider) ClamAV version
numbers.
- Tweaked printing of primary thread counts and header values so
they fit in 80-column windows (as best as possible; the `h` suffix
of the `DBTIME` hour value doesn't fit, but at least the value
does). Previously, an 83-column window was required for the
information to fit.
- Make another line truncate at screen edge instead of 2 cols short.
- Make print_con_info truncate at screen edge and properly handle
messages with a newline.
- Use strrchr in print_con_info; handle wrapped lines in task list.
- Improve alignment of task list header labels.
- Fix null host/port bug; curses_inited should be initially 0.
- Fix compiler warning; fixes to line wrap patch; correct ENGINE
field width; a little tidy-up.
- Fix warnings; cleanup mem stats; version parsing fix; formatting tweaks
- Use newterm() and delscreen() for cleaner exit.
Also relocated codepage table from msdoc.h to entconv.h
Also adds new macros for codepages to reduce use of magic numbers when
referencing code pages elsewhere in libclamav.
294429: Negative check for fd_out occurs after a call to fdopen where
the value must not be negative. Coverity interprets this as a high
severity issue, even though it really isn't. Removing the needless check
should silence the false positive.
Somewhere between bison versions 3.5 and 3.6.4, having multiple
characters in a single-quoted string went from a warning to an error.
This commit corrects that by changing a literal to string,
Removed all autotools generates files. Autotools (autoconf, automake,
libtool, pkg-config, m4) will be required from now on for builds from
git clones.
Added autogen.sh to be run before ./configure.
Significant update to main .gitignore file.
Removed extraneous .gitignore files. A Git repository only needs one
.gitignore file.
Overflow check "(value >> 32) * 10 < INT32_MAX" may not work in
certain conditions, e.g. value is 0xcccccccdbcdc9cc
Note: This fixes oss-fuzz bug 16117.
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
Fixes a possible stack buffer overflow introduced in 0.103 development
when we added optional names to file maps (fmaps). The CPIO parser uses
a stack buffer to store the name (if present). If no name present, then
the stack buffer was passed unitialized to the fmap scanning function
which could cause an overflow.
This fix both initializes the buffer and uses a pointer so the scan
function gets NULL instead of a buffer in the event that a name isn't
present as that's the intended way to use the API, rather than passing
an empty string name buffer.
Fixes a stack overflow that resulted in stack corruption and general
mayhem. This bugfix only applies to the 0.103 dev branch.
The issue was caused by buffering formatted XLM macro content to a small
buffer without regard for possible overflow. Instead of buffering
manually, use of snprintf and later cli_writen were replaced with direct
calls to fwrite / fprintf / fputc.
The ARJ parser fix from 0.102.3 was insufficient and still allowed for some overflow. This commit passes correct buffer sizes into text_normalize functions.
Add notices to man pages and help strings cautioning against running
bytecode signatures from untrusted sources.
Also adds missing BytecodeUnsigned option to clamd.conf.sample files.
A malicious user could replace a scan target's directory with a symlink
to another path to trick clamscan, clamdscan, or clamonacc into removing
or moving a different file (eg. a critical system file). The issue would
affect users that use the `--move` or `--remove` options for clamscan,
clamdscan, and clamonacc.
This patch gets the real path for the scan target before the scan,
and if the file alerts and the --move or --remove quarantine features
are used, it mitigates the symlink attack by traversing the path one
directory at a time until reaching the leaf directory where the scan
target file resides before unlinking (or renaming) the file directly.
This commit applies a similar tactic used in the previous commit for
Windows builds, using the Win32 Native API to traverse a path and delete
or move files by handle rather than by file path.
I had some trouble using SetFileInformationByHandle to rename a file by
handle, so for Windows instead it will copy the file to the new location
and then use the safe unlink technique to remove the old file. If the
symlink attack occurs, the unlink will fail, and the system will not be
damaged.
For more information about AV quarantine attacks using links, see the
[RACK911 Lab's report](https://www.rack911labs.com/research/exploiting-almost-every-antivirus-software)
dboptions should be an unsigned int.
Julius Plenz accidentally missing from NEWS.md.
Fail out if cl_engine_settings_apply() fails, because the only possible
cause of failure is a malloc issue.
Add clamd config option to force blocking clamd database reload to
conserve RAM. Users may set `ConcurrentDatabaseReload no` in their
clamd.conf config file to force a blocking reload.
The blocking mode will still perform the reload in a new thread, but
will first free the current database, wait for scans targeting that
database to complete, and then load the new database in the new thread
and wait (`pthread_join()`) on that thread. Once loaded, any pending
scans will continue. This is effectively the same behavior as how
clamd reloads worked before the multi-threaded database reload feature
was added.
Offload the DB load to a separate thread and only replace the current
engine instance afterwards.
While reload is pending:
- existing scan requests use the old db (this is unchanged)
- new scan requests are honored instead of blocked and they also use
the old db (this is new)
After the reload is complete:
- existing scan requests use the old db (this is unchanged)
- new scan requests use the new db (this is unchanged)
The existing engine is refcounted so it'll be eventually freed when no
longer in use.
Reload requests while reload is pending are silently ignored (i.e. It
never forks more than a single reload thread).
Patch courtesy of Alberto Wu. We would also like to thank Julius Plenz
for original work on this issue, as well as Alexander Sulfrian,
Arjen de Korte, David Heidelberg, and Ged Haywood for their work
updating and testing these patches.
PDFs may contain Javascript actions in objects. Those may actually be
indirect references to other objects where the Javascript resides rather
than storing the Javascript in the current object. The thing is,
sometimes the indirect object is empty (no actual script), in which case
the PDF may not have any active content.
This commit changes the Javascript detection logic so it only records
the stats/JSON after detecting the "Javascript" and "JS" tags in the
object (indirect or direct) where the Javascript is supposed to reside.
This moves the logic from an action callback when the object dictionary
key "Javascript" is detected over into the object extraction logic,
after all of the objects have been parsed.
Micah Snyder (micasnyd)