From 75a4267eff9a9e7a969735a0c83cd2d0f0082730 Mon Sep 17 00:00:00 2001 From: Mickey Sola Date: Fri, 31 Jan 2020 11:51:29 -0800 Subject: [PATCH] bb12461 - error out properly when pdf parser fails to allocate a map; normalize/sanitize user supplied filename and comment info when parsing arj headers; add better bound checking and error handling to arj header parsers --- libclamav/pdf.c | 24 ++++++-- libclamav/scanners.c | 6 +- libclamav/unarj.c | 128 +++++++++++++++++++++++++++++++++---------- 3 files changed, 120 insertions(+), 38 deletions(-) diff --git a/libclamav/pdf.c b/libclamav/pdf.c index 3690cb390e..046466244d 100644 --- a/libclamav/pdf.c +++ b/libclamav/pdf.c @@ -1584,9 +1584,14 @@ int pdf_extract_obj(struct pdf_struct *pdf, struct pdf_obj *obj, uint32_t flags) cli_dbgmsg("Error decoding stream! Error code: %d\n", rc); /* It's ok if we couldn't decode the stream, - * make a best effort to keep parsing. */ - if (CL_EPARSE == rc) + * make a best effort to keep parsing... + * Unless we were unable to allocate memory.*/ + if (CL_EMEM == rc) { + goto err; + } + if (CL_EPARSE == rc) { rc = CL_SUCCESS; + } if (NULL != objstm) { /* @@ -1764,11 +1769,14 @@ int pdf_extract_obj(struct pdf_struct *pdf, struct pdf_obj *obj, uint32_t flags) } } +err: close(fout); - if (flags & PDF_EXTRACT_OBJ_SCAN && !pdf->ctx->engine->keeptmp) - if (cli_unlink(fullname) && rc != CL_VIRUS) - rc = CL_EUNLINK; + if (CL_EMEM != rc) { + if (flags & PDF_EXTRACT_OBJ_SCAN && !pdf->ctx->engine->keeptmp) + if (cli_unlink(fullname) && rc != CL_VIRUS) + rc = CL_EUNLINK; + } return rc; } @@ -3481,7 +3489,10 @@ int cli_pdf(const char *dir, cli_ctx *ctx, off_t offset) objs_found = pdf.nobjs; rc = pdf_find_and_extract_objs(&pdf, &alerts); - if (pdf.nobjs <= objs_found) { + if (CL_EMEM == rc) { + cli_dbgmsg("cli_pdf: pdf_find_and_extract_objs had an allocation failure\n"); + goto err; + } else if (pdf.nobjs <= objs_found) { cli_dbgmsg("cli_pdf: pdf_find_and_extract_objs did not find any new objects!\n"); } else { cli_dbgmsg("cli_pdf: pdf_find_and_extract_objs found %d new objects.\n", pdf.nobjs - objs_found); @@ -3532,6 +3543,7 @@ int cli_pdf(const char *dir, cli_ctx *ctx, off_t offset) pdf_export_json(&pdf); #endif +err: if (pdf.objstms) { for (i = 0; i < pdf.nobjstms; i++) { if (pdf.objstms[i]) { diff --git a/libclamav/scanners.c b/libclamav/scanners.c index 059a7dbe61..abcf8d77c5 100644 --- a/libclamav/scanners.c +++ b/libclamav/scanners.c @@ -3108,7 +3108,7 @@ static int cli_scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_file_ cli_warnmsg("cli_scanraw: Type %u not handled in fpt loop\n", fpt->type); } - if (nret == CL_VIRUS || break_loop) + if (nret == CL_VIRUS || nret == CL_EMEM || break_loop) break; fpt = fpt->next; @@ -3493,7 +3493,8 @@ static int magic_scandesc(cli_ctx *ctx, cli_file_t type) } if (type != CL_TYPE_IGNORED && ctx->engine->sdb) { - if ((ret = cli_scanraw(ctx, type, 0, &dettype, (ctx->engine->engine_options & ENGINE_OPTIONS_DISABLE_CACHE) ? NULL : hash)) == CL_VIRUS) { + ret = cli_scanraw(ctx, type, 0, &dettype, (ctx->engine->engine_options & ENGINE_OPTIONS_DISABLE_CACHE) ? NULL : hash); + if (ret == CL_EMEM || ret == CL_VIRUS) { ret = cli_checkfp(hash, hashed_size, ctx); cli_bitset_free(ctx->hook_lsig_matches); ctx->hook_lsig_matches = old_hook_lsig_matches; @@ -3501,6 +3502,7 @@ static int magic_scandesc(cli_ctx *ctx, cli_file_t type) } } + ctx->recursion++; perf_nested_start(ctx, PERFT_CONTAINER, PERFT_SCAN); /* set current level as container AFTER recursing */ diff --git a/libclamav/unarj.c b/libclamav/unarj.c index bd7edd21ff..025770f7bc 100644 --- a/libclamav/unarj.c +++ b/libclamav/unarj.c @@ -39,6 +39,7 @@ #include "clamav.h" #include "others.h" #include "unarj.h" +#include "textnorm.h" #define FIRST_HDR_SIZE 30 #define COMMENT_MAX 2048 @@ -832,8 +833,13 @@ static int arj_read_main_header(arj_metadata_t *metadata) { uint16_t header_size, count; arj_main_hdr_t main_hdr; - const char *filename, *comment; + const char *filename = NULL; + const char *comment = NULL; off_t header_offset; + struct text_norm_state fnstate, comstate; + unsigned char *fnnorm = NULL; + unsigned char *comnorm = NULL; + uint32_t ret = TRUE; if (fmap_readn(metadata->map, &header_size, metadata->offset, 2) != 2) return FALSE; @@ -844,14 +850,18 @@ static int arj_read_main_header(arj_metadata_t *metadata) cli_dbgmsg("Header Size: %d\n", header_size); if (header_size == 0) { /* End of archive */ - return FALSE; + ret = FALSE; + goto done; } if (header_size > HEADERSIZE_MAX) { cli_dbgmsg("arj_read_header: invalid header_size: %u\n ", header_size); - return FALSE; + ret = FALSE; + goto done; + } + if (fmap_readn(metadata->map, &main_hdr, metadata->offset, 30) != 30) { + ret = FALSE; + goto done; } - if (fmap_readn(metadata->map, &main_hdr, metadata->offset, 30) != 30) - return FALSE; metadata->offset += 30; cli_dbgmsg("ARJ Main File Header\n"); @@ -865,34 +875,48 @@ static int arj_read_main_header(arj_metadata_t *metadata) if (main_hdr.first_hdr_size < 30) { cli_dbgmsg("Format error. First Header Size < 30\n"); - return FALSE; + ret = FALSE; + goto done; } if (main_hdr.first_hdr_size > 30) { metadata->offset += main_hdr.first_hdr_size - 30; } + fnnorm = cli_calloc(sizeof(unsigned char), header_size + 1); filename = fmap_need_offstr(metadata->map, metadata->offset, header_size); if (!filename) { cli_dbgmsg("UNARJ: Unable to allocate memory for filename\n"); - return FALSE; + ret = FALSE; + goto done; } - metadata->offset += strlen(filename) + 1; + metadata->offset += CLI_STRNLEN(filename, header_size) + 1; + comnorm = cli_calloc(sizeof(unsigned char), header_size + 1); comment = fmap_need_offstr(metadata->map, metadata->offset, header_size); - if (!comment) { + if (!comment || !comnorm) { cli_dbgmsg("UNARJ: Unable to allocate memory for comment\n"); - return FALSE; + ret = FALSE; + goto done; } - metadata->offset += strlen(comment) + 1; - cli_dbgmsg("Filename: %s\n", filename); - cli_dbgmsg("Comment: %s\n", comment); + metadata->offset += CLI_STRNLEN(comment, header_size) + 1; + + text_normalize_init(&fnstate, fnnorm, header_size); + text_normalize_init(&comstate, comnorm, header_size); + + text_normalize_buffer(&fnstate, filename, metadata->offset); + text_normalize_buffer(&comstate, comment, metadata->offset); + + cli_dbgmsg("Filename: %s\n", fnnorm); + cli_dbgmsg("Comment: %s\n", comnorm); metadata->offset += 4; /* crc */ /* Skip past any extended header data */ for (;;) { const uint16_t *countp = fmap_need_off_once(metadata->map, metadata->offset, 2); - if (!countp) - return FALSE; + if (!countp) { + ret = FALSE; + goto done; + } count = cli_readint16(countp); metadata->offset += 2; cli_dbgmsg("Extended header size: %d\n", count); @@ -902,7 +926,19 @@ static int arj_read_main_header(arj_metadata_t *metadata) /* Skip extended header + 4byte CRC */ metadata->offset += count + 4; } - return TRUE; + +done: + + if (fnnorm) { + free(fnnorm); + fnnorm = NULL; + } + + if (comnorm) { + free(comnorm); + comnorm = NULL; + } + return ret; } static int arj_read_file_header(arj_metadata_t *metadata) @@ -910,6 +946,10 @@ static int arj_read_file_header(arj_metadata_t *metadata) uint16_t header_size, count; const char *filename, *comment; arj_file_hdr_t file_hdr; + struct text_norm_state fnstate, comstate; + unsigned char *fnnorm = NULL; + unsigned char *comnorm = NULL; + uint32_t ret = CL_SUCCESS; if (fmap_readn(metadata->map, &header_size, metadata->offset, 2) != 2) return CL_EFORMAT; @@ -919,15 +959,18 @@ static int arj_read_file_header(arj_metadata_t *metadata) cli_dbgmsg("Header Size: %d\n", header_size); if (header_size == 0) { /* End of archive */ - return CL_BREAK; + ret = CL_BREAK; + goto done; } if (header_size > HEADERSIZE_MAX) { cli_dbgmsg("arj_read_file_header: invalid header_size: %u\n ", header_size); - return CL_EFORMAT; + ret = CL_EFORMAT; + goto done; } if (fmap_readn(metadata->map, &file_hdr, metadata->offset, 30) != 30) { - return CL_EFORMAT; + ret = CL_EFORMAT; + goto done; } metadata->offset += 30; file_hdr.comp_size = le32_to_host(file_hdr.comp_size); @@ -947,7 +990,8 @@ static int arj_read_file_header(arj_metadata_t *metadata) if (file_hdr.first_hdr_size < 30) { cli_dbgmsg("Format error. First Header Size < 30\n"); - return CL_EFORMAT; + ret = CL_EFORMAT; + goto done; } /* Note: this skips past any extended file start position data (multi-volume) */ @@ -955,22 +999,33 @@ static int arj_read_file_header(arj_metadata_t *metadata) metadata->offset += file_hdr.first_hdr_size - 30; } + fnnorm = cli_calloc(sizeof(unsigned char), header_size + 1); filename = fmap_need_offstr(metadata->map, metadata->offset, header_size); if (!filename) { cli_dbgmsg("UNARJ: Unable to allocate memory for filename\n"); - return FALSE; + ret = FALSE; + goto done; } - metadata->offset += strlen(filename) + 1; + metadata->offset += CLI_STRNLEN(filename, header_size) + 1; + comnorm = cli_calloc(sizeof(unsigned char), header_size + 1); comment = fmap_need_offstr(metadata->map, metadata->offset, header_size); if (!comment) { cli_dbgmsg("UNARJ: Unable to allocate memory for comment\n"); - return FALSE; + ret = FALSE; + goto done; } - metadata->offset += strlen(comment) + 1; - cli_dbgmsg("Filename: %s\n", filename); - cli_dbgmsg("Comment: %s\n", comment); - metadata->filename = cli_strdup(filename); + metadata->offset += CLI_STRNLEN(comment, header_size) + 1; + + text_normalize_init(&fnstate, fnnorm, header_size); + text_normalize_init(&comstate, comnorm, header_size); + + text_normalize_buffer(&fnstate, filename, metadata->offset); + text_normalize_buffer(&comstate, comment, metadata->offset); + + cli_dbgmsg("Filename: %s\n", fnnorm); + cli_dbgmsg("Comment: %s\n", comnorm); + metadata->filename = CLI_STRNDUP(filename, header_size); /* Skip CRC */ metadata->offset += 4; @@ -982,7 +1037,8 @@ static int arj_read_file_header(arj_metadata_t *metadata) if (metadata->filename) free(metadata->filename); metadata->filename = NULL; - return CL_EFORMAT; + ret = CL_EFORMAT; + goto done; } count = cli_readint16(countp); metadata->offset += 2; @@ -999,10 +1055,22 @@ static int arj_read_file_header(arj_metadata_t *metadata) metadata->encrypted = ((file_hdr.flags & GARBLE_FLAG) != 0) ? TRUE : FALSE; metadata->ofd = -1; if (!metadata->filename) { - return CL_EMEM; + ret = CL_EMEM; + goto done; } - return CL_SUCCESS; + done: + + if (fnnorm) { + free(fnnorm); + fnnorm = NULL; + } + + if (comnorm) { + free(comnorm); + comnorm = NULL; + } + return ret; } int cli_unarj_open(fmap_t *map, const char *dirname, arj_metadata_t *metadata, size_t off)