fuzz - 12166 - Fix for 4-byte out of bounds write wherein the an invalid struct pointer member variable is set to zero. The fix adds bounds checking to the Uniq storage 'add' function as well as error code checks. Included a lot of new inline documentation.

libclamav/libclamav.map

@@ -62,8 +62,8 @@ CLAMAV_PUBLIC {
 };
 CLAMAV_PRIVATE {
   global:
-    cli_sigperf_print; 
-    cli_sigperf_events_destroy; 
+    cli_sigperf_print;
+    cli_sigperf_events_destroy;
     cli_pcre_perf_print;
     cli_pcre_perf_events_destroy;
     cli_pcre_init;
@@ -101,6 +101,7 @@ CLAMAV_PRIVATE {
     cli_ppt_vba_read;
     cli_wm_readdir;
     cli_wm_decrypt_macro;
+    cli_free_vba_project;
     cli_readn;
     cli_str2hex;
     cli_hashfile;

libclamav/ole2_extract.c

@@ -810,10 +810,18 @@ handler_writefile(ole2_header_t *hdr, property_t *prop, const char *dir, cli_ctx
         return CL_SUCCESS;
     }
     name = get_property_name2(prop->name, prop->name_size);
-    if (name)
-        cnt = uniq_add(hdr->U, name, strlen(name), &hash);
-    else
-        cnt = uniq_add(hdr->U, NULL, 0, &hash);
+    if (name) {
+        if (CL_SUCCESS != uniq_add(hdr->U, name, strlen(name), &hash, &cnt)) {
+            free(name);
+            cli_dbgmsg("OLE2 [handler_writefile]: too many property names added to uniq store.\n");
+            return CL_BREAK;
+        }
+    } else {
+        if (CL_SUCCESS != uniq_add(hdr->U, NULL, 0, &hash, &cnt)) {
+            cli_dbgmsg("OLE2 [handler_writefile]: too many property names added to uniq store.\n");
+            return CL_BREAK;
+        }
+    }
     snprintf(newname, sizeof(newname), "%s" PATHSEP "%s_%u", dir, hash, cnt);
     newname[sizeof(newname) - 1] = '\0';
     cli_dbgmsg("OLE2 [handler_writefile]: Dumping '%s' to '%s'\n", name ? name : "<empty>", newname);

libclamav/scanners.c

@@ -205,10 +205,10 @@ static int cli_scandir(const char *dirname, cli_ctx *ctx)
 
 /**
  * @brief  Scan the metadata using cli_matchmeta()
- * 
+ *
  * @param metadata  unrar metadata structure
  * @param ctx       scanning context structure
- * @param files     
+ * @param files
  * @return cl_error_t  Returns CL_CLEAN if nothing found, CL_VIRUS if something found, CL_EUNPACK if encrypted.
  */
 static cl_error_t cli_unrar_scanmetadata(unrar_metadata_t *metadata, cli_ctx *ctx, unsigned int files)
@@ -340,7 +340,7 @@ static cl_error_t cli_scanrar(const char *filepath, int desc, cli_ctx *ctx)
     /*
      * Read & scan each file header.
      * Extract & scan each file.
-     * 
+     *
      * Skip files if they will exceed max filesize or max scansize.
      * Count the number of encrypted file headers and encrypted files.
      *  - Alert if there are encrypted files,
@@ -406,7 +406,7 @@ static cl_error_t cli_scanrar(const char *filepath, int desc, cli_ctx *ctx)
                     break;
                 }
             } else if (cli_checklimits("RAR", ctx, metadata.unpack_size, 0, 0)) {
-                /* File size exceeds maxfilesize, must skip extraction. 
+                /* File size exceeds maxfilesize, must skip extraction.
                 * Although we may be able to scan the metadata */
                 nTooLargeFilesFound += 1;
 
@@ -441,12 +441,12 @@ static cl_error_t cli_scanrar(const char *filepath, int desc, cli_ctx *ctx)
 
                 unrar_ret = cli_unrar_extract_file(hArchive, extract_fullpath, NULL);
                 if (unrar_ret != UNRAR_OK) {
-                    /* 
+                    /*
                      * Some other error extracting the file
                      */
                     cli_dbgmsg("RAR: Error extracting file: %s\n", metadata.filename);
 
-                    /* TODO: 
+                    /* TODO:
                      *   may need to manually skip the file depending on what, specifically, cli_unrar_extract_file() returned.
                      */
                 } else {
@@ -1124,7 +1124,8 @@ static int vba_scandata(const unsigned char *data, unsigned int len, cli_ctx *ct
 
 static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
 {
-    int ret = CL_CLEAN, i, j, fd, data_len, hasmacros = 0;
+    cl_error_t ret = CL_CLEAN;
+    int i, j, fd, data_len, hasmacros = 0;
     vba_project_t *vba_project;
     DIR *dd;
     struct dirent *dent;
@@ -1138,22 +1139,28 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
     char *fullname, vbaname[1024];
     unsigned char *data;
     char *hash;
-    uint32_t hashcnt;
+    uint32_t hashcnt           = 0;
     unsigned int viruses_found = 0;
 
     cli_dbgmsg("VBADir: %s\n", dirname);
-    hashcnt = uniq_get(U, "_vba_project", 12, NULL);
-    while (hashcnt--) {
-        if (!(vba_project = (vba_project_t *)cli_vba_readdir(dirname, U, hashcnt)))
+    if (CL_SUCCESS != (ret = uniq_get(U, "_vba_project", 12, NULL, &hashcnt))) {
+        cli_dbgmsg("VBADir: uniq_get('_vba_project') failed with ret code (%d)!\n", ret);
+        return ret;
+    }
+    while (hashcnt) {
+        if (!(vba_project = (vba_project_t *)cli_vba_readdir(dirname, U, hashcnt))) {
+            hashcnt--;
             continue;
+        }
 
         for (i = 0; i < vba_project->count; i++) {
-            for (j = 0; (unsigned int)j < vba_project->colls[i]; j++) {
+            for (j = 1; (unsigned int)j <= vba_project->colls[i]; j++) {
                 snprintf(vbaname, 1024, "%s" PATHSEP "%s_%u", vba_project->dir, vba_project->name[i], j);
                 vbaname[sizeof(vbaname) - 1] = '\0';
                 fd                           = open(vbaname, O_RDONLY | O_BINARY);
-                if (fd == -1)
+                if (fd == -1) {
                     continue;
+                }
                 cli_dbgmsg("VBADir: Decompress VBA project '%s_%u'\n", vba_project->name[i], j);
                 data = (unsigned char *)cli_vba_inflate(fd, vba_project->offset[i], &data_len);
                 close(fd);
@@ -1197,23 +1204,28 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
             }
         }
 
-        free(vba_project->name);
-        free(vba_project->colls);
-        free(vba_project->dir);
-        free(vba_project->offset);
-        free(vba_project);
+        cli_free_vba_project(vba_project);
+        vba_project = NULL;
+
         if (ret == CL_VIRUS && !SCAN_ALLMATCHES)
             break;
+
+        hashcnt--;
     }
 
-    if ((ret == CL_CLEAN || (ret == CL_VIRUS && SCAN_ALLMATCHES)) &&
-        (hashcnt = uniq_get(U, "powerpoint document", 19, &hash))) {
-        while (hashcnt--) {
+    if (ret == CL_CLEAN || (ret == CL_VIRUS && SCAN_ALLMATCHES)) {
+        if (CL_SUCCESS != (ret = uniq_get(U, "powerpoint document", 19, &hash, &hashcnt))) {
+            cli_dbgmsg("VBADir: uniq_get('powerpoint document') failed with ret code (%d)!\n", ret);
+            return ret;
+        }
+        while (hashcnt) {
             snprintf(vbaname, 1024, "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
             vbaname[sizeof(vbaname) - 1] = '\0';
             fd                           = open(vbaname, O_RDONLY | O_BINARY);
-            if (fd == -1)
+            if (fd == -1) {
+                hashcnt--;
                 continue;
+            }
             if ((fullname = cli_ppt_vba_read(fd, ctx))) {
                 if (cli_scandir(fullname, ctx) == CL_VIRUS) {
                     ret = CL_VIRUS;
@@ -1224,20 +1236,27 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
                 free(fullname);
             }
             close(fd);
+            hashcnt--;
         }
     }
 
-    if ((ret == CL_CLEAN || (ret == CL_VIRUS && SCAN_ALLMATCHES)) &&
-        (hashcnt = uniq_get(U, "worddocument", 12, &hash))) {
-        while (hashcnt--) {
+    if (ret == CL_CLEAN || (ret == CL_VIRUS && SCAN_ALLMATCHES)) {
+        if (CL_SUCCESS != (ret = uniq_get(U, "worddocument", 12, &hash, &hashcnt))) {
+            cli_dbgmsg("VBADir: uniq_get('worddocument') failed with ret code (%d)!\n", ret);
+            return ret;
+        }
+        while (hashcnt) {
             snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
             vbaname[sizeof(vbaname) - 1] = '\0';
             fd                           = open(vbaname, O_RDONLY | O_BINARY);
-            if (fd == -1)
+            if (fd == -1) {
+                hashcnt--;
                 continue;
+            }
 
             if (!(vba_project = (vba_project_t *)cli_wm_readdir(fd))) {
                 close(fd);
+                hashcnt--;
                 continue;
             }
 
@@ -1264,19 +1283,16 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
             }
 
             close(fd);
-            free(vba_project->name);
-            free(vba_project->colls);
-            free(vba_project->dir);
-            free(vba_project->offset);
-            free(vba_project->key);
-            free(vba_project->length);
-            free(vba_project);
+            cli_free_vba_project(vba_project);
+            vba_project = NULL;
+
             if (ret == CL_VIRUS) {
                 if (SCAN_ALLMATCHES)
                     viruses_found++;
                 else
                     break;
             }
+            hashcnt--;
         }
     }
 
@@ -1286,8 +1302,11 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
 #if HAVE_JSON
     /* JSON Output Summary Information */
     if (SCAN_COLLECT_METADATA && (ctx->wrkproperty != NULL)) {
-        hashcnt = uniq_get(U, "_5_summaryinformation", 21, &hash);
-        while (hashcnt--) {
+        if (CL_SUCCESS != (ret = uniq_get(U, "_5_summaryinformation", 21, &hash, &hashcnt))) {
+            cli_dbgmsg("VBADir: uniq_get('_5_summaryinformation') failed with ret code (%d)!\n", ret);
+            return ret;
+        }
+        while (hashcnt) {
             snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
             vbaname[sizeof(vbaname) - 1] = '\0';
 
@@ -1298,10 +1317,14 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
                 cli_ole2_summary_json(ctx, fd, 0);
                 close(fd);
             }
+            hashcnt--;
         }
 
-        hashcnt = uniq_get(U, "_5_documentsummaryinformation", 29, &hash);
-        while (hashcnt--) {
+        if (CL_SUCCESS != (ret = uniq_get(U, "_5_documentsummaryinformation", 29, &hash, &hashcnt))) {
+            cli_dbgmsg("VBADir: uniq_get('_5_documentsummaryinformation') failed with ret code (%d)!\n", ret);
+            return ret;
+        }
+        while (hashcnt) {
             snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
             vbaname[sizeof(vbaname) - 1] = '\0';
 
@@ -1312,13 +1335,17 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
                 cli_ole2_summary_json(ctx, fd, 1);
                 close(fd);
             }
+            hashcnt--;
         }
     }
 #endif
 
     /* Check directory for embedded OLE objects */
-    hashcnt = uniq_get(U, "_1_ole10native", 14, &hash);
-    while (hashcnt--) {
+    if (CL_SUCCESS != (ret = uniq_get(U, "_1_ole10native", 14, &hash, &hashcnt))) {
+        cli_dbgmsg("VBADir: uniq_get('_1_ole10native') failed with ret code (%d)!\n", ret);
+        return ret;
+    }
+    while (hashcnt) {
         snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
         vbaname[sizeof(vbaname) - 1] = '\0';
 
@@ -1329,6 +1356,7 @@ static int cli_vba_scandir(const char *dirname, cli_ctx *ctx, struct uniq *U)
             if (ret != CL_CLEAN && !(ret == CL_VIRUS && SCAN_ALLMATCHES))
                 return ret;
         }
+        hashcnt--;
     }
 
     /* ACAB: since we now hash filenames and handle collisions we
@@ -3667,8 +3695,8 @@ int cli_mem_scandesc(const void *buffer, size_t length, cli_ctx *ctx)
 }
 
 /**
- * @brief   The main function to initiate a scan, that may be invoked with a file descriptor or a file map. 
- * 
+ * @brief   The main function to initiate a scan, that may be invoked with a file descriptor or a file map.
+ *
  * @param desc              File descriptor of an open file. The caller must provide this or the map.
  * @param map               File map. The caller must provide this or the desc.
  * @param filepath          (optional, recommended) filepath of the open file descriptor or file map.
@@ -3735,7 +3763,7 @@ static cl_error_t scan_common(int desc, cl_fmap_t *map, const char *filepath, co
         }
     }
 
-    /* Best effort to determine the filename if not provided. 
+    /* Best effort to determine the filename if not provided.
      * May still be NULL if filename could not be determined. */
     if (filepath == NULL) {
         char *fpath = NULL;
@@ -3894,8 +3922,8 @@ int cli_found_possibly_unwanted(cli_ctx *ctx)
             return CL_VIRUS;
         }
         /* heuristic scan isn't taking precedence, keep scanning.
-     * If this is part of an archive, and 
-     * we find a real malware we report that instead of the 
+     * If this is part of an archive, and
+     * we find a real malware we report that instead of the
      * heuristic match */
         ctx->found_possibly_unwanted = 1;
     } else {

libclamav/uniq.c

@@ -47,6 +47,7 @@ struct uniq *uniq_init(uint32_t count)
         uniq_free(U);
         return NULL;
     }
+    U->max_unique_items = count;
 
     return U;
 }
@@ -57,19 +58,39 @@ void uniq_free(struct uniq *U)
     free(U);
 }
 
-uint32_t uniq_add(struct uniq *U, const char *key, uint32_t key_len, char **rhash)
+cl_error_t uniq_add(struct uniq *U, const char *item, uint32_t item_len, char **rhash, uint32_t *count)
 {
+    cl_error_t status = CL_EARG;
     unsigned int i;
     uint8_t digest[16];
     struct UNIQMD5 *m = NULL;
 
-    cl_hash_data("md5", key, key_len, digest, NULL);
+    if (!U) {
+        /* Invalid args */
+        goto done;
+    }
+
+    /* Uniq adds are limited by the maximum allocated in uniq_init(). */
+    if (U->cur_unique_items >= U->max_unique_items) {
+        /* Attempted to add more uniq items than may be stored. */
+        status = CL_EMAXSIZE;
+        goto done;
+    }
+
+    /* Make a hash of the item string */
+    if (NULL == cl_hash_data("md5", item, item_len, digest, NULL)) {
+        /* Failed to create hash of item. */
+        status = CL_EFORMAT;
+        goto done;
+    }
 
+    /* Check for md5 digest match in md5 collection */
     if (U->items && U->md5s[U->idx[*digest]].md5[0] == *digest)
         for (m = &U->md5s[U->idx[*digest]]; m; m = m->next)
             if (!memcmp(&digest[1], &m->md5[1], 15)) break;
 
     if (!m) {
+        /* No match. Add new md5 to list */
         const char HEX[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
 
         m        = &U->md5s[U->items];
@@ -88,28 +109,85 @@ uint32_t uniq_add(struct uniq *U, const char *key, uint32_t key_len, char **rhas
             m->md5[i]          = digest[i];
         }
         m->name[32] = '\0';
+
+        /* Increment # of unique items. */
+        U->cur_unique_items++;
     }
 
+    /* Increment total # of items. */
     U->items++;
+
+    /* Increment # items matching this md5 digest (probably just this 1). */
+    m->count++;
+
+    /* Pass back the ascii hash, if requested. */
     if (rhash) *rhash = m->name;
-    return m->count++;
+
+    /* Pass back the count, if requested. */
+    if (count) *count = m->count;
+
+    status = CL_SUCCESS;
+
+done:
+    return status;
 }
 
-uint32_t uniq_get(struct uniq *U, const char *key, uint32_t key_len, char **rhash)
+cl_error_t uniq_get(struct uniq *U, const char *item, uint32_t item_len, char **rhash, uint32_t *count)
 {
+    cl_error_t status = CL_EARG;
     uint8_t digest[16];
     struct UNIQMD5 *m = NULL;
+    uint32_t idx      = 0;
 
-    cl_hash_data("md5", key, key_len, digest, NULL);
+    if (!U || !count) {
+        /* Invalid args */
+        goto done;
+    }
 
-    if (!U->items || U->md5s[U->idx[*digest]].md5[0] != *digest)
-        return 0;
+    *count = 0;
 
-    for (m = &U->md5s[U->idx[*digest]]; m; m = m->next) {
-        if (memcmp(&digest[1], &m->md5[1], 15)) continue;
-        if (rhash) *rhash = m->name;
-        return m->count;
+    if (!U->items) {
+        goto not_found;
     }
 
-    return 0;
+    /* Make a hash of the item string */
+    if (NULL == cl_hash_data("md5", item, item_len, digest, NULL)) {
+        /* Failed to create hash of item. */
+        status = CL_EFORMAT;
+        goto done;
+    }
+
+    /* Get the md5s array index for the bucket list head. */
+    idx = U->idx[*digest];
+    m   = &U->md5s[idx];
+
+    if (m->md5[0] != *digest) {
+        /*
+         * If the first two bytes in the digest doesn't actually match,
+         * then the item has never been added.
+         * This is a common scenario because the idx table is initialized
+         * to 0's.
+         */
+        goto not_found;
+    }
+
+    do {
+        if (0 == memcmp(&digest[1], &m->md5[1], 15)) {
+            /* The item-hash matched.
+             * Pass back the ascii hash value (if requested).
+             * Return the count of matching items (will be 1+).
+             */
+            if (rhash)
+                *rhash = m->name;
+            *count = m->count;
+            break;
+        }
+        m = m->next;
+    } while (NULL != m);
+
+not_found:
+    status = CL_SUCCESS;
+
+done:
+    return status;
 }

libclamav/uniq.h

@@ -6,6 +6,54 @@
  *
  *  Authors: aCaB <acab@clamav.net>
  *
+ *  Uniq implements a structure that stores the count of duplicate items.
+ *  The count can be retrieved by item name (if you know it).
+ *  Additionally, you can retrieve the ascii md5 hash at the same time.
+ *
+ *  This is essentially a tiny hash table of hashes.
+ *  The hashes are in an array instead of dynamically added.
+ *  This is faster than alloc'ing for each unique item added, *  but means a max # of unique items must be defined at init.
+ *
+ *  Example where:
+ *   items = 6
+ *   max_unique_items = 5
+ *   cur_unique_items = 4
+ *   md5 #1 has been added 3 times
+ *   Two md5's start with the same 2 bytes (#0 and #3)
+ *
+ *    idx:
+ *      -00--01--02--03--04--05--06--07-...
+ *      | 0 | 0 | 0 | 2 | 1 | 0 | 0 | ...
+ *      ------------------------------...
+ *
+ *    md5s:
+ *      ------------------------------
+ *   0  | next:  Address of #3
+ *      | count: 1
+ *      | md5:   0x01,0x98,0x23,0xa8,0xfd,...
+ *      | name:  "019823a8fd..."
+ *      ------------------------------
+ *   1  | next:  NULL
+ *      | count: 3
+ *      | md5:   0x03,0x98,0x23,0xa8,0xfd,...
+ *      | name:  "019823a8fd..."
+ *      ------------------------------
+ *   2  | next:  NULL
+ *      | count: 1
+ *      | md5:   0x01,0x98,0x23,0xa8,0xfd,...
+ *      | name:  "019823a8fd..."
+ *      ------------------------------
+ *   3  | next:  NULL
+ *      | count: 1
+ *      | md5:   0x01,0xdd,0x2f,0x87,0x6a,...
+ *      | name:  "01dd2f876a..."
+ *      ------------------------------
+ *   4  | next:  NULL
+ *      | count: 0
+ *      | md5:   0x00,0x00,0x00,0x00,0x00,...
+ *      | name:  "\0\0\0\0\0..."
+ *      ------------------------------
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
  *  published by the Free Software Foundation.
@@ -24,24 +72,84 @@
 #ifndef _UNIQ_H
 #define _UNIQ_H
 
+#include "clamav.h"
 #include "clamav-types.h"
 
+/**
+ * @brief Store the count of each unique item.
+ *
+ * These elements are allocated as an array in struct uniq, but they are also
+ * linked together using the `next` pointers to form impromptu buckets,
+ * categorized using the first two bytes of each md5.
+ */
 struct UNIQMD5 {
-    struct UNIQMD5 *next;
-    uint32_t count;
-    uint8_t md5[16];
-    char name[33];
+    struct UNIQMD5 *next; /**< Pointer to next UNIQMD5 where the first two bytes are the same. */
+    uint32_t count;       /**< Number of times this item has been added. (# duplicates). */
+    uint8_t md5[16];      /**< Binary md5 hash of the item. */
+    char name[33];        /**< Ascii md5 hash of the item. */
 };
 
+/**
+ * @brief The main Uniq store structure.
+ *
+ * Includes array of uniq md5 hashes, and an index table to optimize searches
+ * into the hash array, categorized by the first two bytes of the md5.
+ */
 struct uniq {
-    struct UNIQMD5 *md5s;
-    uint32_t items;
-    uint32_t idx[256];
+    struct UNIQMD5 *md5s;      /**< Array of UNIQMD5 structs. */
+    uint32_t items;            /**< Total # of items added (including duplicates) */
+    uint32_t cur_unique_items; /**< The # of md5s currently stored in the array. */
+    uint32_t max_unique_items; /**< The # of md5s that can be stored the array. */
+    uint32_t idx[256];         /**< Array of indices into the md5s array.
+                                    Each index represents a linked-list of md5s
+                                    sharing the common trait that the first two
+                                    bytes are the same. */
 };
 
+/**
+ * @brief Initialize a Uniq store to count the number of uniq string items.
+ *
+ * The Uniq store must be free'd with uniq_free().
+ * uniq_add()'s will fail if they exceed the number of unique strings initialized with count.
+ *
+ * @param count         The max number of unique string items that may be added.
+ * @return struct uniq* A pointer to the Uniq store object. Will return NULL on failure.
+ */
 struct uniq *uniq_init(uint32_t);
+
+/**
+ * @brief Free the Uniq store and associated memory.
+ */
 void uniq_free(struct uniq *);
-uint32_t uniq_add(struct uniq *, const char *, uint32_t, char **);
-uint32_t uniq_get(struct uniq *, const char *, uint32_t, char **);
+
+/**
+ * @brief Add to the uniq (item md5) count.
+ *
+ * Adds an item to the list of known items.
+ * Increments the count if the item has been seen before.
+ * The optional rhash pointer will be valid until `uniq_free()` is called.
+ *
+ * @param U             The Uniq count store.
+ * @param item          (optional) The item to hash and count.
+ * @param item_len      The length, in bytes, of the item. May be 0.
+ * @param[out] rhash    (optional) A pointer to the item's md5 hash (in ascii).
+ * @param[out] count    (optional) The number of times this unique item has been added.
+ * @return cl_error_t   CL_SUCCESS if successful, else an error code.
+ */
+cl_error_t uniq_add(struct uniq *U, const char *item, uint32_t, char **rhash, uint32_t *count);
+
+/**
+ * @brief Retrieve the number of times an item has been added to the Uniq count store.
+ *
+ * The optional rhash pointer will be valid until `uniq_free()` is called.
+ *
+ * @param U             The Uniq count store.
+ * @param item          (optional) The item to hash and count.
+ * @param item_len      The length, in bytes, of the item. May be 0.
+ * @param[out] rhash    (optional) A pointer to the item's md5 hash (in ascii).
+ * @param[out] count    The number of times this unique item has been added.
+ * @return cl_error_t   CL_SUCCESS if successful, else an error code.
+ */
+cl_error_t uniq_get(struct uniq *U, const char *item, uint32_t, char **rhash, uint32_t *count);
 
 #endif

libclamav/vba_extract.c

@@ -290,6 +290,7 @@ cli_vba_readdir(const char *dir, struct uniq *U, uint32_t which)
     struct vba56_header v56h;
     off_t seekback;
     char fullname[1024], *hash;
+    uint32_t hashcnt = 0;
 
     cli_dbgmsg("in cli_vba_readdir()\n");
 
@@ -300,8 +301,13 @@ cli_vba_readdir(const char *dir, struct uniq *U, uint32_t which)
 	 * _VBA_PROJECT files are embedded within office documents (OLE2)
 	 */
 
-    if (!uniq_get(U, "_vba_project", 12, &hash))
+    if (CL_SUCCESS != uniq_get(U, "_vba_project", 12, &hash, &hashcnt)) {
+        cli_dbgmsg("vba_readdir: uniq_get('_vba_project') failed. Unable to check # of embedded vba proj files\n");
         return NULL;
+    }
+    if (hashcnt == 0) {
+        return NULL;
+    }
     snprintf(fullname, sizeof(fullname), "%s" PATHSEP "%s_%u", dir, hash, which);
     fullname[sizeof(fullname) - 1] = '\0';
     fd                             = open(fullname, O_RDONLY | O_BINARY);
@@ -444,7 +450,13 @@ cli_vba_readdir(const char *dir, struct uniq *U, uint32_t which)
         }
         ptr = get_unicode_name((const char *)buf, length, big_endian);
         if (ptr == NULL) break;
-        if (!(vba_project->colls[i] = uniq_get(U, ptr, strlen(ptr), &hash))) {
+        if (CL_SUCCESS != uniq_get(U, ptr, strlen(ptr), &hash, &hashcnt)) {
+            cli_dbgmsg("vba_readdir: uniq_get('%s') failed.\n", ptr);
+            free(ptr);
+            break;
+        }
+        vba_project->colls[i] = hashcnt;
+        if (0 == vba_project->colls[i]) {
             cli_dbgmsg("vba_readdir: cannot find project %s (%s)\n", ptr, hash);
             free(ptr);
             break;
@@ -1294,7 +1306,7 @@ create_vba_project(int record_count, const char *dir, struct uniq *U)
 {
     vba_project_t *ret;
 
-    ret = (vba_project_t *)cli_malloc(sizeof(struct vba_project_tag));
+    ret = (vba_project_t *)cli_calloc(1, sizeof(struct vba_project_tag));
 
     if (ret == NULL) {
         cli_errmsg("create_vba_project: Unable to allocate memory for vba project structure\n");
@@ -1306,16 +1318,8 @@ create_vba_project(int record_count, const char *dir, struct uniq *U)
     ret->dir    = cli_strdup(dir);
     ret->offset = (uint32_t *)cli_malloc(sizeof(uint32_t) * record_count);
 
-    if ((ret->name == NULL) || (ret->dir == NULL) || (ret->offset == NULL)) {
-        if (ret->dir)
-            free(ret->dir);
-        if (ret->colls)
-            free(ret->colls);
-        if (ret->name)
-            free(ret->name);
-        if (ret->offset)
-            free(ret->offset);
-        free(ret);
+    if ((ret->colls == NULL) || (ret->name == NULL) || (ret->dir == NULL) || (ret->offset == NULL)) {
+        cli_free_vba_project(ret);
         cli_errmsg("create_vba_project: Unable to allocate memory for vba project elements\n");
         return NULL;
     }
@@ -1324,3 +1328,32 @@ create_vba_project(int record_count, const char *dir, struct uniq *U)
 
     return ret;
 }
+
+/**
+ * @brief Free up the memory associated with the vba_project_t type.
+ *
+ * @param project A vba_project_t type allocated by one of these:
+ *  - create_vba_project()
+ *  - cli_wm_readdir()
+ *  - cli_vba_readdir()
+ */
+void cli_free_vba_project(vba_project_t *vba_project)
+{
+    if (vba_project) {
+        if (vba_project->dir)
+            free(vba_project->dir);
+        if (vba_project->colls)
+            free(vba_project->colls);
+        if (vba_project->name)
+            free(vba_project->name);
+        if (vba_project->offset)
+            free(vba_project->offset);
+        if (vba_project->length)
+            free(vba_project->length);
+        if (vba_project->key)
+            free(vba_project->key);
+        free(vba_project);
+    }
+
+    return;
+}

libclamav/vba_extract.h

@@ -41,6 +41,8 @@ typedef struct vba_project_tag {
 
 vba_project_t *cli_vba_readdir(const char *dir, struct uniq *U, uint32_t which);
 vba_project_t *cli_wm_readdir(int fd);
+void cli_free_vba_project(vba_project_t *vba_project);
+
 unsigned char *cli_vba_inflate(int fd, off_t offset, int *size);
 int cli_scan_ole10(int fd, cli_ctx *ctx);
 char *cli_ppt_vba_read(int fd, cli_ctx *ctx);

sigtool/vba.c

@@ -1106,8 +1106,10 @@ static int sigtool_scandir(const char *dirname, int hex_output)
 
 int sigtool_vba_scandir(const char *dirname, int hex_output, struct uniq *U)
 {
-    int ret = CL_CLEAN, i, fd, data_len;
-    vba_project_t *vba_project;
+    cl_error_t status = CL_CLEAN;
+    cl_error_t ret;
+    int i, fd, data_len;
+    vba_project_t *vba_project = NULL;
     DIR *dd;
     struct dirent *dent;
     STATBUF statbuf;
@@ -1116,15 +1118,23 @@ int sigtool_vba_scandir(const char *dirname, int hex_output, struct uniq *U)
     uint32_t hashcnt;
     unsigned int j;
 
-    hashcnt = uniq_get(U, "_vba_project", 12, NULL);
-    while (hashcnt--) {
-        if (!(vba_project = (vba_project_t *)cli_vba_readdir(dirname, U, hashcnt))) continue;
+    if (CL_SUCCESS != (ret = uniq_get(U, "_vba_project", 12, NULL, &hashcnt))) {
+        logg("!ScanDir -> uniq_get('_vba_project') failed.\n");
+        return ret;
+    }
+
+    while (hashcnt) {
+        if (!(vba_project = (vba_project_t *)cli_vba_readdir(dirname, U, hashcnt))) {
+            hashcnt--;
+            continue;
+        }
 
         for (i = 0; i < vba_project->count; i++) {
             for (j = 0; j < vba_project->colls[i]; j++) {
                 snprintf(vbaname, 1024, "%s" PATHSEP "%s_%u", vba_project->dir, vba_project->name[i], j);
                 vbaname[sizeof(vbaname) - 1] = '\0';
-                fd                           = open(vbaname, O_RDONLY | O_BINARY);
+
+                fd = open(vbaname, O_RDONLY | O_BINARY);
                 if (fd == -1) continue;
                 data = (unsigned char *)cli_vba_inflate(fd, vba_project->offset[i], &data_len);
                 close(fd);
@@ -1138,60 +1148,71 @@ int sigtool_vba_scandir(const char *dirname, int hex_output, struct uniq *U)
             }
         }
 
-        free(vba_project->name);
-        free(vba_project->colls);
-        free(vba_project->dir);
-        free(vba_project->offset);
-        free(vba_project);
+        cli_free_vba_project(vba_project);
+        vba_project = NULL;
+
+        hashcnt--;
     }
 
-    if ((hashcnt = uniq_get(U, "powerpoint document", 19, &hash))) {
-        while (hashcnt--) {
-            snprintf(vbaname, 1024, "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
-            vbaname[sizeof(vbaname) - 1] = '\0';
-            fd                           = open(vbaname, O_RDONLY | O_BINARY);
-            if (fd == -1) continue;
-            if ((fullname = cli_ppt_vba_read(fd, NULL))) {
-                sigtool_scandir(fullname, hex_output);
-                cli_rmdirs(fullname);
-                free(fullname);
-            }
-            close(fd);
+    if (CL_SUCCESS != (ret = uniq_get(U, "powerpoint document", 19, &hash, &hashcnt))) {
+        logg("!ScanDir -> uniq_get('powerpoint document') failed.\n");
+        return ret;
+    }
+
+    while (hashcnt) {
+        snprintf(vbaname, 1024, "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
+        vbaname[sizeof(vbaname) - 1] = '\0';
+
+        fd = open(vbaname, O_RDONLY | O_BINARY);
+        if (fd == -1) {
+            hashcnt--;
+            continue;
+        }
+        if ((fullname = cli_ppt_vba_read(fd, NULL))) {
+            sigtool_scandir(fullname, hex_output);
+            cli_rmdirs(fullname);
+            free(fullname);
         }
+        close(fd);
+        hashcnt--;
     }
 
-    if ((hashcnt = uniq_get(U, "worddocument", 12, &hash))) {
-        while (hashcnt--) {
-            snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
-            vbaname[sizeof(vbaname) - 1] = '\0';
-            fd                           = open(vbaname, O_RDONLY | O_BINARY);
-            if (fd == -1) continue;
+    if (CL_SUCCESS != (ret = uniq_get(U, "worddocument", 12, &hash, &hashcnt))) {
+        logg("!ScanDir -> uniq_get('worddocument') failed.\n");
+        return ret;
+    }
 
-            if (!(vba_project = (vba_project_t *)cli_wm_readdir(fd))) {
-                close(fd);
-                continue;
-            }
+    while (hashcnt) {
+        snprintf(vbaname, sizeof(vbaname), "%s" PATHSEP "%s_%u", dirname, hash, hashcnt);
+        vbaname[sizeof(vbaname) - 1] = '\0';
 
-            for (i = 0; i < vba_project->count; i++) {
-                data_len = vba_project->length[i];
-                data     = (unsigned char *)cli_wm_decrypt_macro(fd, vba_project->offset[i], data_len, vba_project->key[i]);
-                if (data) {
-                    data           = (unsigned char *)realloc(data, data_len + 1);
-                    data[data_len] = '\0';
-                    printf("-------------- start of code ------------------\n%s\n-------------- end of code ------------------\n", data);
-                    free(data);
-                }
-            }
+        fd = open(vbaname, O_RDONLY | O_BINARY);
+        if (fd == -1) {
+            hashcnt--;
+            continue;
+        }
 
+        if (!(vba_project = (vba_project_t *)cli_wm_readdir(fd))) {
             close(fd);
-            free(vba_project->name);
-            free(vba_project->colls);
-            free(vba_project->dir);
-            free(vba_project->offset);
-            free(vba_project->key);
-            free(vba_project->length);
-            free(vba_project);
+            hashcnt--;
+            continue;
+        }
+
+        for (i = 0; i < vba_project->count; i++) {
+            data_len = vba_project->length[i];
+            data     = (unsigned char *)cli_wm_decrypt_macro(fd, vba_project->offset[i], data_len, vba_project->key[i]);
+            if (data) {
+                data           = (unsigned char *)realloc(data, data_len + 1);
+                data[data_len] = '\0';
+                printf("-------------- start of code ------------------\n%s\n-------------- end of code ------------------\n", data);
+                free(data);
+            }
         }
+
+        close(fd);
+        cli_free_vba_project(vba_project);
+        vba_project = NULL;
+        hashcnt--;
     }
 
     if ((dd = opendir(dirname)) != NULL) {
@@ -1217,5 +1238,5 @@ int sigtool_vba_scandir(const char *dirname, int hex_output, struct uniq *U)
     }
 
     closedir(dd);
-    return ret;
+    return status;
 }

unit_tests/check_uniq.c

@@ -28,6 +28,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "../libclamav/clamav.h"
 #include "../libclamav/uniq.h"
 #include "checks.h"
 
@@ -60,12 +61,16 @@ START_TEST(test_uniq_known)
     fail_unless(U != 0, "uniq_init");
 
     for (i = 0; tests[i].expected; i++) {
-        u = uniq_add(U, tests[i].key, tests[i].key_len, &hash);
-        fail_unless_fmt(u == 0 && strcmp(hash, tests[i].expected) == 0, "uniq_add(%s) = %u - expected %s, got %s", tests[i].key, u, tests[i].expected, hash);
+        if (CL_SUCCESS != uniq_add(U, tests[i].key, tests[i].key_len, &hash, &u)) {
+            fail("uniq_add(%s) failed.", tests[i].key);
+        }
+        fail_unless_fmt(u == 1 && strcmp(hash, tests[i].expected) == 0, "uniq_add(%s) = %u - expected %s, got %s", tests[i].key, u, tests[i].expected, hash);
     }
 
     for (i = 0; tests[i].expected; i++) {
-        u = uniq_get(U, tests[i].key, tests[i].key_len, &hash);
+        if (CL_SUCCESS != uniq_get(U, tests[i].key, tests[i].key_len, &hash, &u)) {
+            fail("uniq_get(%s) failed.", tests[i].key);
+        }
         fail_unless_fmt(u == 1 && strcmp(hash, tests[i].expected) == 0, "uniq_get(%s) = %u - expected %s, got %s", tests[i].key, u, tests[i].expected, hash);
     }
 
@@ -83,11 +88,16 @@ START_TEST(test_uniq_colls)
     fail_unless(U != 0, "uniq_init");
 
     for (j = 4; j > 0; j--)
-        for (i = 0; i < j; i++)
-            u = uniq_add(U, tests[i], strlen(tests[i]), NULL);
+        for (i = 0; i < j; i++) {
+            if (CL_SUCCESS != uniq_add(U, tests[i], strlen(tests[i]), NULL, &u)) {
+                fail("uniq_add(%s) failed.", tests[i]);
+            }
+        }
 
     for (i = 0; i < 4; i++) {
-        u = uniq_add(U, tests[i], strlen(tests[i]), NULL);
+        if (CL_SUCCESS != uniq_get(U, tests[i], strlen(tests[i]), NULL, &u)) {
+            fail("uniq_get(%s) failed.", tests[i]);
+        }
         fail_unless_fmt(u + i == 4, "uniq_get(%s) = %u - expected %u", tests[i], u, 4 - i);
     }
 

win32/libclamav.def

@@ -179,6 +179,7 @@ EXPORTS cli_hwp5header @44381 NONAME
 EXPORTS cli_scanhwp5_stream @44382 NONAME
 EXPORTS cli_scanhwp3 @44383 NONAME
 EXPORTS cli_genhash_pe @44384 NONAME
+EXPORTS cli_free_vba_project @44385 NONAME
 
 ; compatibility layer, tommath, zlib
 EXPORTS w32_srand @44269 NONAME