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.
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)
In regression testing we observed occasional errors creating files or
directories. It appears as though these are caused by tmp files that use
identical prefixes. Such prefixed tmp file names currently have a
5-character hash suffix. Though these temporary files or directories are
deleted when no longer needed, there is still the possibility that there
could be a hash collision which causes the scan to error out.
This patch doubles the size of the short-hash to 10-characters to reduce
the chance of a collision.
A way is needed to record scanned file names for two purposes:
1. File names (and extensions) must be stored in the json metadata
properties recorded when using the --gen-json clamscan option. Future
work may use this to compare file extensions with detected file types.
2. File names are useful when interpretting tmp directory output when
using the --leave-temps option.
This commit enables file name retention for later use by storing file
names in the fmap header structure, if a file name exists.
To store the names in fmaps, an optional name argument has been added to
any internal scan API's that create fmaps and every call to these APIs
has been modified to pass a file name or NULL if a file name is not
required. The zip and gpt parsers required some modification to record
file names. The NSIS and XAR parsers fail to collect file names at all
and will require future work to support file name extraction.
Also:
- Added recursive extraction to the tmp directory when the
--leave-temps option is enabled. When not enabled, the tmp directory
structure remains flat so as to prevent the likelihood of exceeding
MAX_PATH. The current tmp directory is stored in the scan context.
- Made the cli_scanfile() internal API non-static and added it to
scanners.h so it would be accessible outside of scanners.c in order to
remove code duplication within libmspack.c.
- Added function comments to scanners.h and matcher.h
- Converted a TDB-type macros and LSIG-type macros to enums for improved
type safey.
- Converted more return status variables from `int` to `cl_error_t` for
improved type safety, and corrected ooxml file typing functions so
they use `cli_file_t` exclusively rather than mixing types with
`cl_error_t`.
- Restructured the magic_scandesc() function to use goto's for error
handling and removed the early_ret_from_magicscan() macro and
magic_scandesc_cleanup() function. This makes the code easier to
read and made it easier to add the recursive tmp directory cleanup to
magic_scandesc().
- Corrected zip, egg, rar filename extraction issues.
- Removed use of extra sub-directory layer for zip, egg, and rar file
extraction. For Zip, this also involved changing the extracted
filenames to be randomly generated rather than using the "zip.###"
file name scheme.
This commit improves the layout of the tmp file output and the JSON
metadata output when using the --leave-temps and --gen-json options.
For all scans, each scan target will get a unique tmp sub-directory. If
using --leave-temps, that subdir will include the basename of the
original file to make it easier to identify. Additionally, when using
--leave-temps option, all extracted objects will have their
subdirectories extracted in recursive subdirectories including filename
prefixes where available. When not using the --leave-temps option, the
layout of the tmp sub-directory will remain flat, so as to alleviate the
possibility of exceeding PATH_MAX.
The JSON metadata generated by the --gen-json option is now generated
for all file types, not just a select few. The format is also
pretty-printed for readability and now includes filenames and file paths
when available.
Also:
- Added missing ALLMATCH check when determining if bytecode hooks should
be run.
- Added cl_engine_get_str API to windows libclamav symbol export file.
This also replaces the cli_*_stats variants with a callback for stats,
so that clamd can call the cl_*_callback variants instead, and pass the filename
as context.
Micah Snyder