Skip to content

Commit d96a6b8

Browse files
author
Steven Morgan
committed
bb11588 - fix out of bounds read.
1 parent b171157 commit d96a6b8

File tree

1 file changed

+50
-29
lines changed

1 file changed

+50
-29
lines changed

Diff for: libclamav/xar.c

+50-29
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,20 @@ static int xar_cleanup_temp_file(cli_ctx *ctx, int fd, char * tmpname)
7171
value - pointer to long to contain the returned value
7272
returns - CL_SUCCESS or CL_EFORMAT
7373
*/
74-
static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, long * value)
74+
static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, size_t * value)
7575
{
7676
const xmlChar * numstr;
77+
ssize_t numval;
78+
7779
if (xmlTextReaderRead(reader) == 1 && xmlTextReaderNodeType(reader) == XML_READER_TYPE_TEXT) {
7880
numstr = xmlTextReaderConstValue(reader);
7981
if (numstr) {
80-
*value = atol((const char *)numstr);
81-
if (*value < 0) {
82+
numval = atol((const char *)numstr);
83+
if (numval < 0) {
8284
cli_dbgmsg("cli_scanxar: XML element value %li\n", *value);
8385
return CL_EFORMAT;
8486
}
87+
*value = numval;
8588
return CL_SUCCESS;
8689
}
8790
}
@@ -123,8 +126,18 @@ static void xar_get_checksum_values(xmlTextReaderPtr reader, unsigned char ** ck
123126
if (xmlTextReaderRead(reader) == 1 && xmlTextReaderNodeType(reader) == XML_READER_TYPE_TEXT) {
124127
xmlval = xmlTextReaderConstValue(reader);
125128
if (xmlval) {
126-
*cksum = xmlStrdup(xmlval);
127-
cli_dbgmsg("cli_scanxar: checksum value is %s.\n", *cksum);
129+
cli_dbgmsg("cli_scanxar: checksum value is %s.\n", xmlval);
130+
if (*hash == XAR_CKSUM_SHA1 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_SHA1 ||
131+
*hash == XAR_CKSUM_MD5 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_MD5)
132+
{
133+
*cksum = xmlStrdup(xmlval);
134+
}
135+
else
136+
{
137+
cli_dbgmsg("cli_scanxar: checksum type is unknown or length is invalid.\n");
138+
*hash = XAR_CKSUM_OTHER;
139+
*cksum = NULL;
140+
}
128141
} else {
129142
*cksum = NULL;
130143
cli_dbgmsg("cli_scanxar: xmlTextReaderConstValue() returns NULL for checksum value.\n");
@@ -149,7 +162,7 @@ static void xar_get_checksum_values(xmlTextReaderPtr reader, unsigned char ** ck
149162
e_hash - pointer to int for returning extracted checksum algorithm.
150163
returns - CL_FORMAT, CL_SUCCESS, CL_BREAK. CL_BREAK indicates no more <data>/<ea> element.
151164
*/
152-
static int xar_get_toc_data_values(xmlTextReaderPtr reader, long *length, long *offset, long *size, int *encoding,
165+
static int xar_get_toc_data_values(xmlTextReaderPtr reader, size_t *length, size_t *offset, size_t *size, int *encoding,
153166
unsigned char ** a_cksum, int * a_hash, unsigned char ** e_cksum, int * e_hash)
154167
{
155168
const xmlChar *name;
@@ -386,10 +399,10 @@ static int xar_hash_check(int hash, const void * result, const void * expected)
386399
return 1;
387400
switch (hash) {
388401
case XAR_CKSUM_SHA1:
389-
len = SHA1_HASH_SIZE;
402+
len = CLI_HASHLEN_SHA1;
390403
break;
391404
case XAR_CKSUM_MD5:
392-
len = CLI_HASH_MD5;
405+
len = CLI_HASHLEN_MD5;
393406
break;
394407
case XAR_CKSUM_OTHER:
395408
case XAR_CKSUM_NONE:
@@ -417,7 +430,7 @@ int cli_scanxar(cli_ctx *ctx)
417430
int fd = -1;
418431
struct xar_header hdr;
419432
fmap_t *map = *ctx->fmap;
420-
long length, offset, size, at;
433+
size_t length, offset, size, at;
421434
int encoding;
422435
z_stream strm;
423436
char *toc, *tmpname;
@@ -490,6 +503,13 @@ int cli_scanxar(cli_ctx *ctx)
490503
goto exit_toc;
491504
}
492505

506+
if (hdr.toc_length_decompressed != strm.total_out) {
507+
cli_dbgmsg("TOC decompress length %" PRIu64 " does not match amount decompressed %lu\n",
508+
hdr.toc_length_decompressed, strm.total_out);
509+
toc[strm.total_out] = '\0';
510+
hdr.toc_length_decompressed = strm.total_out;
511+
}
512+
493513
/* cli_dbgmsg("cli_scanxar: TOC xml:\n%s\n", toc); */
494514
/* printf("cli_scanxar: TOC xml:\n%s\n", toc); */
495515
/* cli_dbgmsg("cli_scanxar: TOC end:\n"); */
@@ -557,8 +577,8 @@ int cli_scanxar(cli_ctx *ctx)
557577
goto exit_reader;
558578
}
559579

560-
cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %li,\n"
561-
"from xar heap offset %li length %li\n",
580+
cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %zu,\n"
581+
"from xar heap offset %zu length %zu\n",
562582
tmpname, size, offset, length);
563583

564584

@@ -638,11 +658,14 @@ int cli_scanxar(cli_ctx *ctx)
638658
#define CLI_LZMA_IBUF_SIZE CLI_LZMA_OBUF_SIZE>>2 /* estimated compression ratio 25% */
639659
{
640660
struct CLI_LZMA lz;
641-
unsigned long in_remaining = length;
661+
unsigned long in_remaining = MIN(length, map->len - at);
642662
unsigned long out_size = 0;
643663
unsigned char * buff = __lzma_wrap_alloc(NULL, CLI_LZMA_OBUF_SIZE);
644664
int lret;
645-
665+
666+
if (length > in_remaining)
667+
length = in_remaining;
668+
646669
memset(&lz, 0, sizeof(lz));
647670
if (buff == NULL) {
648671
cli_dbgmsg("cli_scanxar: memory request for lzma decompression buffer fails.\n");
@@ -655,8 +678,8 @@ int cli_scanxar(cli_ctx *ctx)
655678
if (blockp == NULL) {
656679
char errbuff[128];
657680
cli_strerror(errno, errbuff, sizeof(errbuff));
658-
cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
659-
length, at, errbuff);
681+
cli_dbgmsg("cli_scanxar: Can't read %i bytes @ %li, errno:%s.\n",
682+
CLI_LZMA_HDR_SIZE, at, errbuff);
660683
rc = CL_EREAD;
661684
__lzma_wrap_free(NULL, buff);
662685
goto exit_tmpfile;
@@ -693,7 +716,7 @@ int cli_scanxar(cli_ctx *ctx)
693716
char errbuff[128];
694717
cli_strerror(errno, errbuff, sizeof(errbuff));
695718
cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno: %s.\n",
696-
length, at, errbuff);
719+
lz.avail_in, at, errbuff);
697720
rc = CL_EREAD;
698721
__lzma_wrap_free(NULL, buff);
699722
cli_LzmaShutdown(&lz);
@@ -758,33 +781,31 @@ int cli_scanxar(cli_ctx *ctx)
758781
/* for uncompressed, bzip2, xz, and unknown, just pull the file, cli_magic_scandesc does the rest */
759782
do_extract_cksum = 0;
760783
{
761-
unsigned long write_len;
762-
784+
size_t writelen = MIN(map->len - at, length);
785+
763786
if (ctx->engine->maxfilesize)
764-
write_len = MIN((size_t)(ctx->engine->maxfilesize), (size_t)length);
765-
else
766-
write_len = length;
787+
writelen = MIN((size_t)(ctx->engine->maxfilesize), writelen);
767788

768-
if (!(blockp = (void*)fmap_need_off_once(map, at, length))) {
789+
if (!(blockp = (void*)fmap_need_off_once(map, at, writelen))) {
769790
char errbuff[128];
770791
cli_strerror(errno, errbuff, sizeof(errbuff));
771-
cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
772-
length, at, errbuff);
792+
cli_dbgmsg("cli_scanxar: Can't read %zu bytes @ %zu, errno:%s.\n",
793+
writelen, at, errbuff);
773794
rc = CL_EREAD;
774795
goto exit_tmpfile;
775796
}
776797

777798
if (a_hash_ctx != NULL)
778-
xar_hash_update(a_hash_ctx, blockp, length, a_hash);
799+
xar_hash_update(a_hash_ctx, blockp, writelen, a_hash);
779800

780-
if (cli_writen(fd, blockp, write_len) < 0) {
781-
cli_dbgmsg("cli_scanxar: cli_writen error %li bytes @ %li.\n", length, at);
801+
if (cli_writen(fd, blockp, writelen) < 0) {
802+
cli_dbgmsg("cli_scanxar: cli_writen error %zu bytes @ %li.\n", writelen, at);
782803
rc = CL_EWRITE;
783804
goto exit_tmpfile;
784805
}
785806
/*break;*/
786807
}
787-
}
808+
} /* end of switch */
788809

789810
if (rc == CL_SUCCESS) {
790811
if (a_hash_ctx != NULL) {
@@ -871,7 +892,7 @@ int cli_scanxar(cli_ctx *ctx)
871892
cli_dbgmsg("cli_scanxar: can't scan xar files, need libxml2.\n");
872893
#endif
873894
if (cksum_fails + extract_errors != 0) {
874-
cli_warnmsg("cli_scanxar: %u checksum errors and %u extraction errors, use --debug for more info.\n",
895+
cli_dbgmsg("cli_scanxar: %u checksum errors and %u extraction errors.\n",
875896
cksum_fails, extract_errors);
876897
}
877898

0 commit comments

Comments
 (0)