Skip to content

Commit

Permalink
Fix bug when collect-metadata is enabled and caching is disabled
Browse files Browse the repository at this point in the history
If SCAN_COLLECT_METADATA is enabled, and caching is disabled, we zero-out
the hash after recording it.
This results in a non-NULL and invalid-hash that may be passed to
`cli_scan_fmap()` for the "raw mode" scan.
It's an uncommon code path, but would result in comparing hash-sigs with
a zeroed hash rather than the valid hash.
This bug could result in a missed hash-based sig matches.

There is no reason to invalidate or zero-out the hash if we happen to
calculate it. We avoid the cache-lookup by checking the engine setting,
not by checking if we have a hash.
  • Loading branch information
micahsnyder committed Apr 19, 2024
1 parent 16d1392 commit 31bb8cb
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions libclamav/scanners.c
Original file line number Diff line number Diff line change
Expand Up @@ -4443,9 +4443,6 @@ cl_error_t cli_magic_scan(cli_ctx *ctx, cli_file_t type)
hash[8], hash[9], hash[10], hash[11], hash[12], hash[13], hash[14], hash[15]);

ret = cli_jsonstr(ctx->wrkproperty, "FileMD5", hashstr);
if (ctx->engine->engine_options & ENGINE_OPTIONS_DISABLE_CACHE) {
memset(hash, 0, CLI_HASHLEN_MD5);
}
if (ret != CL_SUCCESS) {
cli_dbgmsg("cli_magic_scan: returning %d %s (no post, no cache)\n", ret, __AT__);
goto early_ret;
Expand Down Expand Up @@ -4498,7 +4495,7 @@ cl_error_t cli_magic_scan(cli_ctx *ctx, cli_file_t type)
* If self protection mechanism enabled, do the scanraw() scan first
* before extracting with a file type parser.
*/
ret = scanraw(ctx, type, 0, &dettype, (ctx->engine->engine_options & ENGINE_OPTIONS_DISABLE_CACHE) ? NULL : hash);
ret = scanraw(ctx, type, 0, &dettype, hash);

// Evaluate the result from the scan to see if it end the scan of this layer early,
// and to decid if we should propagate an error or not.
Expand Down Expand Up @@ -4935,7 +4932,7 @@ cl_error_t cli_magic_scan(cli_ctx *ctx, cli_file_t type)

/* CL_TYPE_HTML: raw HTML files are not scanned, unless safety measure activated via DCONF */
if (type != CL_TYPE_IGNORED && (type != CL_TYPE_HTML || !(SCAN_PARSE_HTML) || !(DCONF_DOC & DOC_CONF_HTML_SKIPRAW)) && !ctx->engine->sdb) {
ret = scanraw(ctx, type, typercg, &dettype, (ctx->engine->engine_options & ENGINE_OPTIONS_DISABLE_CACHE) ? NULL : hash);
ret = scanraw(ctx, type, typercg, &dettype, hash);

// Evaluate the result from the scan to see if it end the scan of this layer early,
// and to decid if we should propagate an error or not.
Expand Down

0 comments on commit 31bb8cb

Please sign in to comment.