CVE-2018-1000085: Out-of-bounds Read (CWE-125)
value: ClamAV version version 0.99.3 contains a Out of bounds heap memory read vulnerability in XAR parser, function xar_hash_check() that can result in Leaking of memory, may help in developing exploit chains.. This attack appear to be exploitable via The victim must scan a crafted XAR file. This vulnerability appears to have been fixed in after commit d96a6b8bcc7439fa7e3876207aa0a8e79c8451b6.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2018-03-13T15:29Z
Repository: https://github.com/Cisco-Talos/clamav-devel
Language: C
Files changed: libclamav/xar.c
Commit date: 2016-06-21 17:24:04 -0400
Commit message: bb11588 - fix out of bounds read.

Diff: diff --git a/libclamav/xar.c b/libclamav/xar.c
index 6a1a287d7..edba7e502 100644
--- a/libclamav/xar.c
+++ b/libclamav/xar.c
@@ -69,21 +69,24 @@ static int xar_cleanup_temp_file(cli_ctx *ctx, int fd, char * tmpname)
    parameters:
      reader - xmlTextReaderPtr
      value - pointer to long to contain the returned value
    returns - CL_SUCCESS or CL_EFORMAT
  */
-static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, long * value)
+static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, size_t * value)
 {
     const xmlChar * numstr;
+    ssize_t numval;
+
     if (xmlTextReaderRead(reader) == 1 && xmlTextReaderNodeType(reader) == XML_READER_TYPE_TEXT) {
         numstr = xmlTextReaderConstValue(reader);
         if (numstr) {
-            *value = atol((const char *)numstr);
-            if (*value < 0) {
+            numval = atol((const char *)numstr);
+            if (numval < 0) {
                 cli_dbgmsg("cli_scanxar: XML element value %li\n", *value);
                 return CL_EFORMAT;
             }
+            *value = numval;
             return CL_SUCCESS;
         }
     }
     cli_dbgmsg("cli_scanxar: No text for XML element\n");
     return CL_EFORMAT;
@@ -121,12 +124,22 @@ static void xar_get_checksum_values(xmlTextReaderPtr reader, unsigned char ** ck
         xmlFree(style);
 
     if (xmlTextReaderRead(reader) == 1 && xmlTextReaderNodeType(reader) == XML_READER_TYPE_TEXT) {
         xmlval = xmlTextReaderConstValue(reader);
         if (xmlval) {
-            *cksum = xmlStrdup(xmlval); 
-            cli_dbgmsg("cli_scanxar: checksum value is %s.\n", *cksum);
+            cli_dbgmsg("cli_scanxar: checksum value is %s.\n", xmlval);
+            if (*hash == XAR_CKSUM_SHA1 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_SHA1 ||
+                *hash == XAR_CKSUM_MD5 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_MD5)
+                {
+                    *cksum = xmlStrdup(xmlval); 
+                } 
+            else
+                {
+                    cli_dbgmsg("cli_scanxar: checksum type is unknown or length is invalid.\n");
+                    *hash = XAR_CKSUM_OTHER;
+                    *cksum = NULL;
+                }
         } else {
             *cksum = NULL;
             cli_dbgmsg("cli_scanxar: xmlTextReaderConstValue() returns NULL for checksum value.\n");           
         }
     }
@@ -147,11 +160,11 @@ static void xar_get_checksum_values(xmlTextReaderPtr reader, unsigned char ** ck
      a_hash - pointer to int for returning archived checksum algorithm.
      e_cksum - pointer to char* for return extracted checksum value.
      e_hash - pointer to int for returning extracted checksum algorithm.
    returns - CL_FORMAT, CL_SUCCESS, CL_BREAK. CL_BREAK indicates no more <data>/<ea> element.
  */
-static int xar_get_toc_data_values(xmlTextReaderPtr reader, long *length, long *offset, long *size, int *encoding,
+static int xar_get_toc_data_values(xmlTextReaderPtr reader, size_t *length, size_t *offset, size_t *size, int *encoding,
                                    unsigned char ** a_cksum, int * a_hash, unsigned char ** e_cksum, int * e_hash)
 {
     const xmlChar *name;
     int indata = 0, inea = 0;
     int rc, gotoffset=0, gotlength=0, gotsize=0;
@@ -384,14 +397,14 @@ static int xar_hash_check(int hash, const void * result, const void * expected)
 
     if (!result || !expected)
         return 1;
     switch (hash) {
     case XAR_CKSUM_SHA1:
-        len = SHA1_HASH_SIZE;
+        len = CLI_HASHLEN_SHA1;
         break;
     case XAR_CKSUM_MD5:
-        len = CLI_HASH_MD5;
+        len = CLI_HASHLEN_MD5;
         break;
     case XAR_CKSUM_OTHER:
     case XAR_CKSUM_NONE:
     default:
         return 1;
@@ -415,11 +428,11 @@ int cli_scanxar(cli_ctx *ctx)
     unsigned int extract_errors = 0;
 #if HAVE_LIBXML2
     int fd = -1;
     struct xar_header hdr;
     fmap_t *map = *ctx->fmap;
-    long length, offset, size, at;
+    size_t length, offset, size, at;
     int encoding;
     z_stream strm;
     char *toc, *tmpname;
     xmlTextReaderPtr reader = NULL;
     int a_hash, e_hash;
@@ -488,10 +501,17 @@ int cli_scanxar(cli_ctx *ctx)
         cli_dbgmsg("cli_scanxar:inflateEnd error %i \n", rc);
         rc = CL_EFORMAT;
         goto exit_toc;
     }
 
+    if (hdr.toc_length_decompressed != strm.total_out) {
+        cli_dbgmsg("TOC decompress length %" PRIu64 " does not match amount decompressed %lu\n",
+                   hdr.toc_length_decompressed, strm.total_out);
+        toc[strm.total_out] = '\0';
+        hdr.toc_length_decompressed = strm.total_out;
+    }
+
     /* cli_dbgmsg("cli_scanxar: TOC xml:\n%s\n", toc); */
     /* printf("cli_scanxar: TOC xml:\n%s\n", toc); */
     /* cli_dbgmsg("cli_scanxar: TOC end:\n"); */
     /* printf("cli_scanxar: TOC end:\n"); */
 
@@ -555,12 +575,12 @@ int cli_scanxar(cli_ctx *ctx)
         if ((rc = cli_gentempfd(ctx->engine->tmpdir, &tmpname, &fd)) != CL_SUCCESS) {
             cli_dbgmsg("cli_scanxar: Can't generate temporary file.\n");
             goto exit_reader;
         }
 
-        cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %li,\n"
-                   "from xar heap offset %li length %li\n",
+        cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %zu,\n"
+                   "from xar heap offset %zu length %zu\n",
                    tmpname, size, offset, length);
 
 
         a_hash_ctx = xar_hash_init(a_hash, &a_sc, &a_mc);
         e_hash_ctx = xar_hash_init(e_hash, &e_sc, &e_mc);
@@ -636,15 +656,18 @@ int cli_scanxar(cli_ctx *ctx)
 #define CLI_LZMA_OBUF_SIZE 1024*1024
 #define CLI_LZMA_HDR_SIZE LZMA_PROPS_SIZE+8
 #define CLI_LZMA_IBUF_SIZE CLI_LZMA_OBUF_SIZE>>2 /* estimated compression ratio 25% */
             {
                 struct CLI_LZMA lz;
-                unsigned long in_remaining = length;
+                unsigned long in_remaining = MIN(length, map->len - at);
                 unsigned long out_size = 0;
                 unsigned char * buff = __lzma_wrap_alloc(NULL, CLI_LZMA_OBUF_SIZE);
                 int lret;
-                
+
+                if (length > in_remaining)
+                    length = in_remaining;
+
                 memset(&lz, 0, sizeof(lz));
                 if (buff == NULL) {
                     cli_dbgmsg("cli_scanxar: memory request for lzma decompression buffer fails.\n");
                     rc = CL_EMEM;
                     goto exit_tmpfile;
@@ -653,12 +676,12 @@ int cli_scanxar(cli_ctx *ctx)
 
                 blockp = (void*)fmap_need_off_once(map, at, CLI_LZMA_HDR_SIZE);
                 if (blockp == NULL) {
                     char errbuff[128];
                     cli_strerror(errno, errbuff, sizeof(errbuff));
-                    cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
-                               length, at, errbuff);
+                    cli_dbgmsg("cli_scanxar: Can't read %i bytes @ %li, errno:%s.\n",
+                               CLI_LZMA_HDR_SIZE, at, errbuff);
                     rc = CL_EREAD;
                     __lzma_wrap_free(NULL, buff);
                     goto exit_tmpfile;
                 }
 
@@ -691,11 +714,11 @@ int cli_scanxar(cli_ctx *ctx)
                     lz.next_in = next_in = (void*)fmap_need_off_once(map, at, lz.avail_in);
                     if (lz.next_in == NULL) {
                         char errbuff[128];
                         cli_strerror(errno, errbuff, sizeof(errbuff));
                         cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno: %s.\n",
-                                   length, at, errbuff);
+                                   lz.avail_in, at, errbuff);
                         rc = CL_EREAD;
                         __lzma_wrap_free(NULL, buff);
                         cli_LzmaShutdown(&lz);
                         goto exit_tmpfile;
                     }
@@ -756,37 +779,35 @@ int cli_scanxar(cli_ctx *ctx)
         case CL_TYPE_BZ:
         case CL_TYPE_XZ:
             /* for uncompressed, bzip2, xz, and unknown, just pull the file, cli_magic_scandesc does the rest */
             do_extract_cksum = 0;
             {
-                unsigned long write_len;
-                
+                size_t writelen = MIN(map->len - at, length);
+
                 if (ctx->engine->maxfilesize)
-                    write_len = MIN((size_t)(ctx->engine->maxfilesize), (size_t)length);
-                else
-                    write_len = length;
+                    writelen = MIN((size_t)(ctx->engine->maxfilesize), writelen);
                     
-                if (!(blockp = (void*)fmap_need_off_once(map, at, length))) {
+                if (!(blockp = (void*)fmap_need_off_once(map, at, writelen))) {
                     char errbuff[128];
                     cli_strerror(errno, errbuff, sizeof(errbuff));
-                    cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
-                               length, at, errbuff);
+                    cli_dbgmsg("cli_scanxar: Can't read %zu bytes @ %zu, errno:%s.\n",
+                               writelen, at, errbuff);
                     rc = CL_EREAD;
                     goto exit_tmpfile;
                 }
                 
                 if (a_hash_ctx != NULL)
-                    xar_hash_update(a_hash_ctx, blockp, length, a_hash);
+                    xar_hash_update(a_hash_ctx, blockp, writelen, a_hash);
                 
-                if (cli_writen(fd, blockp, write_len) < 0) {
-                    cli_dbgmsg("cli_scanxar: cli_writen error %li bytes @ %li.\n", length, at);
+                if (cli_writen(fd, blockp, writelen) < 0) {
+                    cli_dbgmsg("cli_scanxar: cli_writen error %zu bytes @ %li.\n", writelen, at);
                     rc = CL_EWRITE;
                     goto exit_tmpfile;
                 }
                 /*break;*/
             }          
-        }
+        } /* end of switch */
 
         if (rc == CL_SUCCESS) {
             if (a_hash_ctx != NULL) {
                 xar_hash_final(a_hash_ctx, result, a_hash);
                 a_hash_ctx = NULL;
@@ -869,11 +890,11 @@ int cli_scanxar(cli_ctx *ctx)
         rc = CL_SUCCESS;
 #else
     cli_dbgmsg("cli_scanxar: can't scan xar files, need libxml2.\n");
 #endif
     if (cksum_fails + extract_errors != 0) {
-        cli_warnmsg("cli_scanxar: %u checksum errors and %u extraction errors, use --debug for more info.\n",
+        cli_dbgmsg("cli_scanxar: %u checksum errors and %u extraction errors.\n",
                     cksum_fails, extract_errors);
     }
 
     return rc;
 }

Vulnerable code: static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, long * value)
            *value = atol((const char *)numstr);
            if (*value < 0) {
            *cksum = xmlStrdup(xmlval); 
            cli_dbgmsg("cli_scanxar: checksum value is %s.\n", *cksum);
static int xar_get_toc_data_values(xmlTextReaderPtr reader, long *length, long *offset, long *size, int *encoding,
        len = SHA1_HASH_SIZE;
        len = CLI_HASH_MD5;
    long length, offset, size, at;
        cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %li,\n"
                   "from xar heap offset %li length %li\n",
                unsigned long in_remaining = length;
                
                    cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
                               length, at, errbuff);
                                   length, at, errbuff);
                unsigned long write_len;
                
                    write_len = MIN((size_t)(ctx->engine->maxfilesize), (size_t)length);
                else
                    write_len = length;
                if (!(blockp = (void*)fmap_need_off_once(map, at, length))) {
                    cli_dbgmsg("cli_scanxar: Can't read %li bytes @ %li, errno:%s.\n",
                               length, at, errbuff);
                    xar_hash_update(a_hash_ctx, blockp, length, a_hash);
                if (cli_writen(fd, blockp, write_len) < 0) {
                    cli_dbgmsg("cli_scanxar: cli_writen error %li bytes @ %li.\n", length, at);
        }
        cli_warnmsg("cli_scanxar: %u checksum errors and %u extraction errors, use --debug for more info.\n",
Fixed code: static int xar_get_numeric_from_xml_element(xmlTextReaderPtr reader, size_t * value)
    ssize_t numval;

            numval = atol((const char *)numstr);
            if (numval < 0) {
            *value = numval;
            cli_dbgmsg("cli_scanxar: checksum value is %s.\n", xmlval);
            if (*hash == XAR_CKSUM_SHA1 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_SHA1 ||
                *hash == XAR_CKSUM_MD5 && xmlStrlen(xmlval) == 2 * CLI_HASHLEN_MD5)
                {
                    *cksum = xmlStrdup(xmlval); 
                } 
            else
                {
                    cli_dbgmsg("cli_scanxar: checksum type is unknown or length is invalid.\n");
                    *hash = XAR_CKSUM_OTHER;
                    *cksum = NULL;
                }
static int xar_get_toc_data_values(xmlTextReaderPtr reader, size_t *length, size_t *offset, size_t *size, int *encoding,
        len = CLI_HASHLEN_SHA1;
        len = CLI_HASHLEN_MD5;
    size_t length, offset, size, at;
    if (hdr.toc_length_decompressed != strm.total_out) {
        cli_dbgmsg("TOC decompress length %" PRIu64 " does not match amount decompressed %lu\n",
                   hdr.toc_length_decompressed, strm.total_out);
        toc[strm.total_out] = '\0';
        hdr.toc_length_decompressed = strm.total_out;
    }

        cli_dbgmsg("cli_scanxar: decompress into temp file:\n%s, size %zu,\n"
                   "from xar heap offset %zu length %zu\n",
                unsigned long in_remaining = MIN(length, map->len - at);

                if (length > in_remaining)
                    length = in_remaining;

                    cli_dbgmsg("cli_scanxar: Can't read %i bytes @ %li, errno:%s.\n",
                               CLI_LZMA_HDR_SIZE, at, errbuff);
                                   lz.avail_in, at, errbuff);
                size_t writelen = MIN(map->len - at, length);

                    writelen = MIN((size_t)(ctx->engine->maxfilesize), writelen);
                if (!(blockp = (void*)fmap_need_off_once(map, at, writelen))) {
                    cli_dbgmsg("cli_scanxar: Can't read %zu bytes @ %zu, errno:%s.\n",
                               writelen, at, errbuff);
                    xar_hash_update(a_hash_ctx, blockp, writelen, a_hash);
                if (cli_writen(fd, blockp, writelen) < 0) {
                    cli_dbgmsg("cli_scanxar: cli_writen error %zu bytes @ %li.\n", writelen, at);
        } /* end of switch */
        cli_dbgmsg("cli_scanxar: %u checksum errors and %u extraction errors.\n",