From d647e607f710eff30908a602adc2f39f01c69c40 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Wed, 8 Oct 2025 17:02:22 -0400 Subject: [PATCH] Fix performance issue scanning some Windows executables Scanning CL_TYPE_MSEXE that have embedded file type signature matches for CL_TYPE_MSEXE are incorrectly passing the PE header check for the contained file, resulting in excessive scan times. The problem is that the `peinfo` struct needs to have the `offset` set for the contained `CL_TYPE_MSEXE` match prior to the header check. Without that, the header check was actually validating the PE header of the original file, which would always pass when that's a PE, and would always fail if it's an OLE2 file (the other type which we check for contained PEs). The additional code change in this commit is to make it so the `ctx` parameter must never be NULL, and removing the `map` parameter because, in practice, that is always from `ctx->fmap`. This is to safeguard against future changes to the function that may accidentally use `ctx` without a proper NULL check. CLAM-2882 --- libclamav/pe.c | 21 ++++++++++++--------- libclamav/pe.h | 2 +- libclamav/scanners.c | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libclamav/pe.c b/libclamav/pe.c index b899c1b9b..e08e2d9cd 100644 --- a/libclamav/pe.c +++ b/libclamav/pe.c @@ -2781,7 +2781,7 @@ int cli_scanpe(cli_ctx *ctx) cli_exe_info_init(peinfo, 0); - peheader_ret = cli_peheader(map, peinfo, opts, ctx); + peheader_ret = cli_peheader(ctx, peinfo, opts); // Warn the user if PE header parsing failed - if it's a binary that runs // successfully on Windows, we need to relax our PE parsing standards so @@ -4360,7 +4360,7 @@ int cli_scanpe(cli_ctx *ctx) cl_error_t cli_pe_targetinfo(cli_ctx *ctx, struct cli_exe_info *peinfo) { - return cli_peheader(ctx->fmap, peinfo, CLI_PEHEADER_OPT_EXTRACT_VINFO, NULL); + return cli_peheader(ctx, peinfo, CLI_PEHEADER_OPT_EXTRACT_VINFO); } /** Parse the PE header and, if successful, populate peinfo @@ -4420,7 +4420,7 @@ cl_error_t cli_pe_targetinfo(cli_ctx *ctx, struct cli_exe_info *peinfo) * * TODO Same as above but with JSON creation */ -cl_error_t cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ctx *ctx) +cl_error_t cli_peheader(cli_ctx *ctx, struct cli_exe_info *peinfo, uint32_t opts) { cl_error_t ret = CL_ERROR; @@ -4446,17 +4446,20 @@ cl_error_t cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, size_t read; uint32_t temp; + fmap_t *map = NULL; + int toval = 0; struct json_object *pe_json = NULL; char jsonbuf[128]; - if (ctx == NULL && - (opts & CLI_PEHEADER_OPT_COLLECT_JSON || - opts & CLI_PEHEADER_OPT_DBG_PRINT_INFO)) { - cli_errmsg("cli_peheader: ctx can't be NULL for options specified\n"); + if (ctx == NULL) { + cli_errmsg("cli_peheader: ctx can't be NULL\n"); + ret = CL_EARG; goto done; } + map = ctx->fmap; + if (opts & CLI_PEHEADER_OPT_COLLECT_JSON) { pe_json = get_pe_property(ctx); } @@ -5469,7 +5472,7 @@ cl_error_t cli_check_auth_header(cli_ctx *ctx, struct cli_exe_info *peinfo) peinfo = &_peinfo; cli_exe_info_init(peinfo, 0); - if (CL_SUCCESS != cli_peheader(ctx->fmap, peinfo, CLI_PEHEADER_OPT_NONE, NULL)) { + if (CL_SUCCESS != cli_peheader(ctx, peinfo, CLI_PEHEADER_OPT_NONE)) { cli_exe_info_destroy(peinfo); return CL_EFORMAT; } @@ -5741,7 +5744,7 @@ cl_error_t cli_genhash_pe(cli_ctx *ctx, unsigned int class, cli_hash_type_t type // if so, use that to avoid having to re-parse the header cli_exe_info_init(peinfo, 0); - if (cli_peheader(ctx->fmap, peinfo, CLI_PEHEADER_OPT_NONE, NULL) != CL_SUCCESS) { + if (cli_peheader(ctx, peinfo, CLI_PEHEADER_OPT_NONE) != CL_SUCCESS) { cli_exe_info_destroy(peinfo); return CL_EFORMAT; } diff --git a/libclamav/pe.h b/libclamav/pe.h index 1cc633222..3a3070df6 100644 --- a/libclamav/pe.h +++ b/libclamav/pe.h @@ -88,7 +88,7 @@ enum { #define CLI_PEHEADER_OPT_REMOVE_MISSING_SECTIONS 0x10 cl_error_t cli_pe_targetinfo(cli_ctx *ctx, struct cli_exe_info *peinfo); -cl_error_t cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ctx *ctx); +cl_error_t cli_peheader(cli_ctx *ctx, struct cli_exe_info *peinfo, uint32_t opts); cl_error_t cli_check_auth_header(cli_ctx *ctx, struct cli_exe_info *peinfo); cl_error_t cli_genhash_pe(cli_ctx *ctx, unsigned int class, cli_hash_type_t type, stats_section_t *hashes); diff --git a/libclamav/scanners.c b/libclamav/scanners.c index fc19cc63e..2cb2cca49 100644 --- a/libclamav/scanners.c +++ b/libclamav/scanners.c @@ -4143,10 +4143,10 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi break; } - cli_exe_info_init(&peinfo, 0); + cli_exe_info_init(&peinfo, fpt->offset); // Header validity check to prevent false positives from being scanned. - ret = cli_peheader(ctx->fmap, &peinfo, CLI_PEHEADER_OPT_NONE, NULL); + ret = cli_peheader(ctx, &peinfo, CLI_PEHEADER_OPT_NONE); // peinfo memory may have been allocated and must be freed even if it failed. cli_exe_info_destroy(&peinfo);