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
This commit is contained in:
Micah Snyder 2025-10-08 17:02:22 -04:00
parent 36f7c83ea1
commit d647e607f7
No known key found for this signature in database
GPG key ID: D522B87461CC6290
3 changed files with 15 additions and 12 deletions

View file

@ -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;
}

View file

@ -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);

View file

@ -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);