Skip to content

Commit

Permalink
bb12461 - error out properly when pdf parser fails to allocate a map;…
Browse files Browse the repository at this point in the history 
… normalize/sanitize user supplied filename and comment info when parsing arj headers; add better bound checking and error handling to arj header parsers
  • Loading branch information
Mickey Sola authored and Mickey Sola (micksola) committed Jan 31, 2020
1 parent 56f3d14 commit 75a4267
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 38 deletions.
24 changes: 18 additions & 6 deletions libclamav/pdf.c
View file
Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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]) {
Expand Down
6 changes: 4 additions & 2 deletions libclamav/scanners.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -3493,14 +3493,16 @@ 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;
return magic_scandesc_cleanup(ctx, type, hash, hashed_size, cache_clean, ret, parent_property);
}
}


ctx->recursion++;
perf_nested_start(ctx, PERFT_CONTAINER, PERFT_SCAN);
/* set current level as container AFTER recursing */
Expand Down
128 changes: 98 additions & 30 deletions libclamav/unarj.c
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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");
Expand All @@ -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);
Expand All @@ -902,14 +926,30 @@ 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)
{
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;
Expand All @@ -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);
Expand All @@ -947,30 +990,42 @@ 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) */
if (file_hdr.first_hdr_size > 30) {
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;
Expand All @@ -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;
Expand All @@ -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)
Expand Down

0 comments on commit 75a4267

Please sign in to comment.