Scanners: Remove allmatch checks + significant code cleanup

Also fixed a number of conditions where magic_scan() critical errors may
be ignored.

To ensure that the scan truly aborts for signature matches (not in
allmatch mode) and for timeouts, the `ctx->abort` option is now set in
these two conditions, and checked in several spots in magic_scan().

Additionally, I've consolidated some of the "scan must halt" type of
checks (mostly large switch statements) into a function so that we can
use the exact same logic in a few places in magic_scan().

I've also fixed a few minor warnings and code format issues.
This commit is contained in:
Micah Snyder 2022-08-27 10:46:21 -07:00 committed by Micah Snyder
parent 8eef7b8ac4
commit 0bd2ae26bc
16 changed files with 853 additions and 851 deletions

View file

@ -394,8 +394,8 @@ int main(int argc, char *argv[])
fprintf(stderr, "Out of memory\n");
exit(3);
}
ctx->ctx = &cctx;
cctx.engine = engine;
ctx->ctx = &cctx;
cctx.engine = engine;
cctx.evidence = evidence_new();
cctx.recursion_stack_size = cctx.engine->max_recursion_level;

View file

@ -123,7 +123,9 @@ int cli_binhex(cli_ctx *ctx)
break;
}
ret = cli_magic_scan_desc(datafd, dname, ctx, NULL, LAYER_ATTRIBUTES_NONE);
if (ret == CL_VIRUS) break;
if (ret != CL_SUCCESS) {
break;
}
}
if (dec_done)
memmove(decoded, &decoded[todo], dec_done);

View file

@ -2933,14 +2933,15 @@ cl_error_t cli_bytecode_runhook(cli_ctx *cctx, const struct cl_engine *engine, s
close(fd);
if (!cctx->engine->keeptmp) {
if (tempfile && cli_unlink(tempfile))
if (tempfile && cli_unlink(tempfile)) {
ret = CL_EUNLINK;
}
}
free(tempfile);
if (ret == CL_VIRUS) {
cli_dbgmsg("Scanning unpacked file by bytecode %u found a virus\n", bc->id);
if (ret != CL_SUCCESS) {
cli_dbgmsg("Scanning unpacked file by bytecode %u found a reason to stop: %s\n", bc->id, cl_strerror(ret));
cli_bytecode_context_clear(ctx);
return ret;
}

View file

@ -1241,8 +1241,8 @@ class LLVMCodegen
unsigned offset = GVoffsetMap[g];
Constant *Idx = ConstantInt::get(Type::getInt32Ty(Context), offset);
Value *Idxs[2] = {
ConstantInt::get(Type::getInt32Ty(Context), 0),
Idx};
ConstantInt::get(Type::getInt32Ty(Context), 0),
Idx};
Value *GEP = Builder.CreateInBoundsGEP(Ctx, ArrayRef<Value *>(Idxs, Idxs + 2));
Type *Ty = GVtypeMap[g];
Ty = PointerType::getUnqual(PointerType::getUnqual(Ty));

View file

@ -416,8 +416,8 @@ scan_overlay:
// Is there an overlay?
if (offset < map->len) {
cli_dbgmsg("GIF: Found extra data after the end of the GIF data stream: %zu bytes, we'll scan it!\n", map->len - offset);
cl_error_t nested_scan_result = cli_magic_scan_nested_fmap_type(map, offset, map->len - offset, ctx, CL_TYPE_ANY, NULL, LAYER_ATTRIBUTES_NONE);
status = nested_scan_result != CL_SUCCESS ? nested_scan_result : status;
status = cli_magic_scan_nested_fmap_type(map, offset, map->len - offset, ctx, CL_TYPE_ANY, NULL, LAYER_ATTRIBUTES_NONE);
goto done;
}
}

View file

@ -545,13 +545,17 @@ int cli_scannulsft(cli_ctx *ctx, off_t offset)
free(nsist.dir);
return CL_ESEEK;
}
if (nsist.fno == 1)
if (nsist.fno == 1) {
ret = cli_scan_desc(nsist.ofd, ctx, CL_TYPE_ANY, false, NULL, AC_SCAN_VIR, NULL, NULL, LAYER_ATTRIBUTES_NONE); /// TODO: Extract file names
else
} else {
ret = cli_magic_scan_desc(nsist.ofd, nsist.ofn, ctx, NULL, LAYER_ATTRIBUTES_NONE); /// TODO: Extract file names
}
close(nsist.ofd);
if (!ctx->engine->keeptmp)
if (cli_unlink(nsist.ofn)) ret = CL_EUNLINK;
if (!ctx->engine->keeptmp) {
if (cli_unlink(nsist.ofn)) {
ret = CL_EUNLINK;
}
}
} else if (ret == CL_EMAXSIZE) {
ret = nsist.solid ? CL_BREAK : CL_SUCCESS;
}
@ -562,8 +566,9 @@ int cli_scannulsft(cli_ctx *ctx, off_t offset)
nsis_shutdown(&nsist);
if (!ctx->engine->keeptmp)
if (!ctx->engine->keeptmp) {
cli_rmdirs(nsist.dir);
}
free(nsist.dir);

View file

@ -1114,46 +1114,56 @@ cl_error_t cli_checklimits(const char *who, cli_ctx *ctx, unsigned long need1, u
cl_error_t ret = CL_SUCCESS;
unsigned long needed;
/* if called without limits, go on, unpack, scan */
if (!ctx) return ret;
if (!ctx) {
/* if called without limits, go on, unpack, scan */
goto done;
}
needed = (need1 > need2) ? need1 : need2;
needed = (needed > need3) ? needed : need3;
/* Enforce timelimit */
if (CL_ETIMEOUT == (ret = cli_checktimelimit(ctx))) {
/* Abort the scan ... */
ret = CL_ETIMEOUT;
/* Enforce global time limit, if limit enabled */
ret = cli_checktimelimit(ctx);
if (CL_SUCCESS != ret) {
// Exceeding the time limit will abort the scan.
// The logic for this and the possible heuristic is done inside the cli_checktimelimit function.
goto done;
}
/* Enforce global scan-size limit */
if (needed && ctx->engine->maxscansize) {
/* if the remaining scansize is too small... */
if (ctx->engine->maxscansize - ctx->scansize < needed) {
/* Skip this file */
cli_dbgmsg("%s: scansize exceeded (initial: %lu, consumed: %lu, needed: %lu)\n", who, (unsigned long int)ctx->engine->maxscansize, (unsigned long int)ctx->scansize, needed);
ret = CL_EMAXSIZE;
cli_append_potentially_unwanted_if_heur_exceedsmax(ctx, "Heuristics.Limits.Exceeded.MaxScanSize");
}
/* Enforce global scan-size limit, if limit enabled */
if (needed && (ctx->engine->maxscansize != 0) && (ctx->engine->maxscansize - ctx->scansize < needed)) {
/* The size needed is greater than the remaining scansize ... Skip this file. */
cli_dbgmsg("%s: scansize exceeded (initial: %lu, consumed: %lu, needed: %lu)\n", who, (unsigned long int)ctx->engine->maxscansize, (unsigned long int)ctx->scansize, needed);
ret = CL_EMAXSIZE;
cli_append_potentially_unwanted_if_heur_exceedsmax(ctx, "Heuristics.Limits.Exceeded.MaxScanSize");
goto done;
}
/* Enforce per-file file-size limit */
if (needed && ctx->engine->maxfilesize && ctx->engine->maxfilesize < needed) {
/* Skip this file */
/* Enforce per-file file-size limit, if limit enabled */
if (needed && (ctx->engine->maxfilesize != 0) && (ctx->engine->maxfilesize < needed)) {
/* The size needed is greater than that limit ... Skip this file. */
cli_dbgmsg("%s: filesize exceeded (allowed: %lu, needed: %lu)\n", who, (unsigned long int)ctx->engine->maxfilesize, needed);
ret = CL_EMAXSIZE;
cli_append_potentially_unwanted_if_heur_exceedsmax(ctx, "Heuristics.Limits.Exceeded.MaxFileSize");
goto done;
}
/* Enforce limit on number of embedded files */
if (ctx->engine->maxfiles && ctx->scannedfiles >= ctx->engine->maxfiles) {
/* Abort the scan ... */
/* Enforce limit on number of embedded files, if limit enabled */
if ((ctx->engine->maxfiles != 0) && (ctx->scannedfiles >= ctx->engine->maxfiles)) {
/* This file would exceed the max # of files ... Skip this file. */
cli_dbgmsg("%s: files limit reached (max: %u)\n", who, ctx->engine->maxfiles);
ret = CL_EMAXFILES;
cli_append_potentially_unwanted_if_heur_exceedsmax(ctx, "Heuristics.Limits.Exceeded.MaxFiles");
ctx->abort_scan = true;
// We don't need to set the `ctx->abort_scan` flag here.
// We want `cli_magic_scan()` to finish scanning the current file, but not any future files.
// We keep track of the # scanned files with `ctx->scannedfiles`, and that should be sufficient to prevent
// additional files from being scanned.
goto done;
}
done:
return ret;
}
@ -1191,10 +1201,8 @@ cl_error_t cli_checktimelimit(cli_ctx *ctx)
if (ctx->time_limit.tv_sec != 0) {
struct timeval now;
if (gettimeofday(&now, NULL) == 0) {
if (now.tv_sec > ctx->time_limit.tv_sec) {
ctx->abort_scan = true;
ret = CL_ETIMEOUT;
} else if (now.tv_sec == ctx->time_limit.tv_sec && now.tv_usec > ctx->time_limit.tv_usec) {
if ((now.tv_sec > ctx->time_limit.tv_sec) ||
(now.tv_sec == ctx->time_limit.tv_sec && now.tv_usec > ctx->time_limit.tv_usec)) {
ctx->abort_scan = true;
ret = CL_ETIMEOUT;
}
@ -1203,6 +1211,9 @@ cl_error_t cli_checktimelimit(cli_ctx *ctx)
if (CL_ETIMEOUT == ret) {
cli_append_potentially_unwanted_if_heur_exceedsmax(ctx, "Heuristics.Limits.Exceeded.MaxScanTime");
// abort_scan flag is set so that in cli_magic_scan() we *will* stop scanning, even if we lose the status code.
ctx->abort_scan = true;
}
done:
@ -1392,6 +1403,8 @@ static cl_error_t append_virus(cli_ctx *ctx, const char *virname, IndicatorType
switch (type) {
case IndicatorType_Strong: {
status = CL_VIRUS;
// abort_scan flag is set so that in cli_magic_scan() we *will* stop scanning, even if we lose the status code.
ctx->abort_scan = true;
break;
}
case IndicatorType_PotentiallyUnwanted: {
@ -1460,7 +1473,7 @@ cl_error_t cli_recursion_stack_push(cli_ctx *ctx, cl_fmap_t *map, cli_file_t typ
recursion_level_t *new_container = NULL;
// Check the regular limits
if (CL_SUCCESS != (status = cli_checklimits("cli_updatelimits", ctx, map->len, 0, 0))) {
if (CL_SUCCESS != (status = cli_checklimits("cli_recursion_stack_push", ctx, map->len, 0, 0))) {
cli_dbgmsg("cli_recursion_stack_push: Some content was skipped. The scan result will not be cached.\n");
emax_reached(ctx); // Disable caching for all recursion layers.
goto done;

View file

@ -204,7 +204,7 @@ typedef struct cli_ctx_tag {
uint64_t scansize;
struct cl_scan_options *options;
unsigned int scannedfiles;
unsigned int corrupted_input;
unsigned int corrupted_input; /* Setting this flag will prevent the PE parser from reporting "broken executable" for unpacked/reconstructed files that may not be 100% to spec. */
recursion_level_t *recursion_stack; /* Array of recursion levels used as a stack. */
uint32_t recursion_stack_size; /* stack size must == engine->max_recursion_level */
uint32_t recursion_level; /* Index into recursion_stack; current fmap recursion level from start of scan. */

View file

@ -237,19 +237,25 @@ static int rtf_object_begin(struct rtf_state* state, cli_ctx* ctx, const char* t
return 0;
}
static int decode_and_scan(struct rtf_object_data* data, cli_ctx* ctx)
static cl_error_t decode_and_scan(struct rtf_object_data* data, cli_ctx* ctx)
{
int ret = CL_CLEAN;
cl_error_t ret = CL_CLEAN;
cli_dbgmsg("RTF:Scanning embedded object: %s\n", data->name);
if (data->fd > 0) {
if (data->bread == 1) {
cli_dbgmsg("Decoding ole object\n");
ret = cli_scan_ole10(data->fd, ctx);
} else {
ret = cli_magic_scan_desc(data->fd, data->name, ctx, NULL, LAYER_ATTRIBUTES_NONE);
}
cli_dbgmsg("RTF:Scanning embedded object:%s\n", data->name);
if (data->bread == 1 && data->fd > 0) {
cli_dbgmsg("Decoding ole object\n");
ret = cli_scan_ole10(data->fd, ctx);
} else if (data->fd > 0)
ret = cli_magic_scan_desc(data->fd, data->name, ctx, NULL, LAYER_ATTRIBUTES_NONE);
if (data->fd > 0)
close(data->fd);
data->fd = -1;
data->fd = -1;
}
if (data->name) {
if (!ctx->engine->keeptmp)
if (cli_unlink(data->name)) ret = CL_EUNLINK;
@ -257,9 +263,7 @@ static int decode_and_scan(struct rtf_object_data* data, cli_ctx* ctx)
data->name = NULL;
}
if (ret != CL_CLEAN)
return ret;
return 0;
return ret;
}
static int rtf_object_process(struct rtf_state* state, const unsigned char* input, const size_t len)

File diff suppressed because it is too large Load diff

View file

@ -517,8 +517,8 @@ static cl_error_t real_scansis(cli_ctx *ctx, const char *tmpd)
FREE(decomp);
if (CL_VIRUS == cli_magic_scan_desc(fd, ofn, ctx, original_filepath, LAYER_ATTRIBUTES_NONE)) {
status = CL_VIRUS;
status = cli_magic_scan_desc(fd, ofn, ctx, original_filepath, LAYER_ATTRIBUTES_NONE);
if (CL_SUCCESS != status) {
goto done;
}
@ -713,6 +713,8 @@ static inline void seeknext(struct SISTREAM *s)
static cl_error_t real_scansis9x(cli_ctx *ctx, const char *tmpd)
{
cl_error_t ret;
struct SISTREAM stream;
struct SISTREAM *s = &stream;
uint32_t field, optst[] = {T_CONTROLLERCHECKSUM, T_DATACHECKSUM, T_COMPRESSED};
@ -840,9 +842,10 @@ static cl_error_t real_scansis9x(cli_ctx *ctx, const char *tmpd)
break;
}
free(dst);
if (cli_magic_scan_desc(fd, tempf, ctx, NULL, LAYER_ATTRIBUTES_NONE) == CL_VIRUS) {
ret = cli_magic_scan_desc(fd, tempf, ctx, NULL, LAYER_ATTRIBUTES_NONE);
if (CL_SUCCESS != ret) {
close(fd);
return CL_VIRUS;
return ret;
}
close(fd);
break;

View file

@ -291,7 +291,7 @@ static cl_error_t scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
ret = cli_magic_scan_desc(fd, tmpname, ctx, NULL, LAYER_ATTRIBUTES_NONE);
close(fd);
if (!(ctx->engine->keeptmp)) {
if (!ctx->engine->keeptmp) {
if (cli_unlink(tmpname)) {
free(tmpname);
return CL_EUNLINK;

View file

@ -161,7 +161,7 @@ cl_error_t cli_scanxdp(cli_ctx *ctx)
rc = cli_magic_scan_buff(decoded, decodedlen, ctx, NULL, LAYER_ATTRIBUTES_NONE);
free(decoded);
if (rc != CL_SUCCESS || rc == CL_BREAK) {
if (rc != CL_SUCCESS) {
xmlFree((void *)value);
break;
}

View file

@ -2120,7 +2120,7 @@ static fc_error_t check_for_new_database_version(
&remotever,
&remotename);
switch (ret) {
case FC_SUCCESS: {
case FC_SUCCESS:
if (0 == localver) {
logg(LOGG_INFO, "%s database available for download (remote version: %d)\n",
database, remotever);
@ -2131,8 +2131,8 @@ static fc_error_t check_for_new_database_version(
break;
}
/* fall-through */
}
case FC_UPTODATE: {
case FC_UPTODATE:
if (NULL == local_database) {
logg(LOGG_ERROR, "check_for_new_database_version: server claims we're up-to-date, but we don't have a local database!\n");
status = FC_EFAILEDGET;
@ -2149,18 +2149,17 @@ static fc_error_t check_for_new_database_version(
We know it will be the same as the local version though. */
remotever = localver;
break;
}
case FC_EFORBIDDEN: {
case FC_EFORBIDDEN:
/* We tried to look up the version using HTTP and were actively blocked. */
logg(LOGG_ERROR, "check_for_new_database_version: Blocked from using server %s.\n", server);
status = FC_EFORBIDDEN;
goto done;
}
default: {
default:
logg(LOGG_ERROR, "check_for_new_database_version: Failed to find %s database using server %s.\n", database, server);
status = FC_EFAILEDGET;
goto done;
}
}
*remoteVersion = remotever;

View file

@ -194,14 +194,14 @@ END_TEST
static int get_test_file(int i, char *file, unsigned fsize, unsigned long *size);
static struct cl_engine *g_engine;
/* int cl_scandesc(int desc, const char **virname, unsigned long int *scanned, const struct cl_engine *engine, const struct cl_limits *limits, struct cl_scan_options* options) */
/* cl_error_t cl_scandesc(int desc, const char **virname, unsigned long int *scanned, const struct cl_engine *engine, const struct cl_limits *limits, struct cl_scan_options* options) */
START_TEST(test_cl_scandesc)
{
const char *virname = NULL;
char file[256];
unsigned long size;
unsigned long int scanned = 0;
int ret;
cl_error_t ret;
struct cl_scan_options options;
memset(&options, 0, sizeof(struct cl_scan_options));

View file

@ -73,7 +73,7 @@ START_TEST(test_unescape_hex)
free(str);
str = cli_unescape("%00");
ck_assert_msg(str && !strcmp(str, "\x1"), "cli_unescape %00");
ck_assert_msg(str && !strcmp(str, "\x1"), "cli_unescape 00");
free(str);
}
END_TEST