@@ -2818,6 +2818,7 @@ int cli_bytecode_runlsig(cli_ctx *cctx, struct cli_target_info *tinfo,

             cli_bytecode_context_clear(&ctx);

             return rc;

         } else {

+            cli_bytecode_context_clear(&ctx);

             return CL_VIRUS;

         }

     }

@@ -455,7 +455,6 @@ static int cli_elf_sh32(cli_ctx *ctx, fmap_t *map, struct cli_exe_info *elfinfo,

         section_hdr = (struct elf_section_hdr32 *)cli_calloc(shnum, shentsize);

         if (!section_hdr) {

             cli_errmsg("ELF: Can't allocate memory for section headers\n");

-            cli_exe_info_destroy(elfinfo);

             return CL_EMEM;

         }

         if (ctx) {

@@ -473,7 +472,6 @@ static int cli_elf_sh32(cli_ctx *ctx, fmap_t *map, struct cli_exe_info *elfinfo,

                 cli_dbgmsg("ELF: Possibly broken ELF file\n");

             }

             free(section_hdr);

-            cli_exe_info_destroy(elfinfo);

             if (ctx && SCAN_HEURISTIC_BROKEN) {

                 cli_append_virus(ctx, "Heuristics.Broken.Executable");

                 return CL_VIRUS;

@@ -558,7 +556,6 @@ static int cli_elf_sh64(cli_ctx *ctx, fmap_t *map, struct cli_exe_info *elfinfo,

         section_hdr = (struct elf_section_hdr64 *)cli_calloc(shnum, shentsize);

         if (!section_hdr) {

             cli_errmsg("ELF: Can't allocate memory for section headers\n");

-            cli_exe_info_destroy(elfinfo);

             return CL_EMEM;

         }

         if (ctx) {

@@ -576,7 +573,6 @@ static int cli_elf_sh64(cli_ctx *ctx, fmap_t *map, struct cli_exe_info *elfinfo,

                 cli_dbgmsg("ELF: Possibly broken ELF file\n");

             }

             free(section_hdr);

-            cli_exe_info_destroy(elfinfo);

             if (ctx && SCAN_HEURISTIC_BROKEN) {

                 cli_append_virus(ctx, "Heuristics.Broken.Executable");

                 return CL_VIRUS;

@@ -805,6 +801,12 @@ int cli_elfheader(fmap_t *map, struct cli_exe_info *elfinfo)

     cli_dbgmsg("in cli_elfheader\n");

+    // TODO This code assumes elfinfo->offset == 0, which might not always

+    // be the case.  For now just print this debug message and continue on

+    if (0 != elfinfo->offset) {

+        cli_dbgmsg("cli_elfheader: Assumption Violated: elfinfo->offset != 0\n");

+    }

+

     ret = cli_elf_fileheader(NULL, map, &file_hdr, &conv, &is64);

     if (ret != CL_CLEAN) {

         return -1;

@@ -1,5 +1,5 @@

 /*

- *  Copyright (C) 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

+ *  Copyright (C) 2018-2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

  *

  *  Authors: Andrew Williams

  *

@@ -21,19 +21,50 @@

 #include "execs.h"

 #include <string.h>

+/**

+ * Initialize a struct cli_exe_info so that it's ready to be populated

+ * by the EXE header parsing functions (cli_peheader, cli_elfheader, and

+ * cli_machoheader) and/or cli_exe_info_destroy.

+ *

+ * @param exeinfo a pointer to the struct cli_exe_info to initialize

+ * @param offset the file offset corresponding to the start of the

+ *        executable that exeinfo stores information about

+ */

 void cli_exe_info_init(struct cli_exe_info *exeinfo, uint32_t offset)

 {

     if (NULL == exeinfo) {

         return;

     }

-    // TODO the memset below might not be needed.  Instead, replace with:

-    // exeinfo->sections = NULL;

-    // memset(&exeinfo->vinfo, '\0', sizeof(exeinfo->vinfo));

+

     memset(exeinfo, '\0', sizeof(*exeinfo));

     exeinfo->offset = offset;

+

+    // TODO Replace the memset above with the following once we can make

+    // certain that this is the only initialization needed (maybe run with

+    // MemorySanitizer?)

+

+    ///* Initialize all of the members which are actually used by the matcher

+    // * and by the bytecode runtime.  The rest is executable specific and

+    // * we'll leave it to be populated by the exe parsing code. */

+    //exeinfo->offset = offset;

+    //exeinfo->sections = NULL;

+    //exeinfo->nsections = 0;

+    //exeinfo->ep = 0;

+    ///* NOTE: These are PE-specific to an extent, but we should still

+    // * initialize them for other exe types because they are used by

+    // * the matcher/bytecode runtime. */

+    //exeinfo->hdr_size = 0;

+    //exeinfo->res_addr = 0;

+    //cli_hashset_init_noalloc(&(exeinfo->vinfo));

 }

+/**

+ * Free resources associated with a struct cli_exe_info initialized

+ * via cli_exe_info_init

+ *

+ * @param exeinfo a pointer to the struct cli_exe_info to destroy

+ */

 void cli_exe_info_destroy(struct cli_exe_info *exeinfo)

 {

@@ -33,17 +33,19 @@

  *  NOTE: This is used to store PE, MachO, and ELF section information. Not

  *  all members are populated by the respective parsing functions.

  *

- *  NOTE: This structure MUST stay in-sync with the ones defined within the

- *  clamav-bytecode-compiler source at:

- *  - clang/lib/Headers/bytecode_execs.h

- *  - llvm/tools/clang/lib/Headers/bytecode_execs.h

- *  We allocate space for an array of these and pass it directly to the

- *  bytecode sig runtime for use.

+ *  NOTE: This header file originates in the clamav-devel source and gets

+ *  copied into the clamav-bytecode-compiler source through a script

+ *  (sync-clamav.sh). This is done because an array of this structure is

+ *  allocated by libclamav and passed to the bytecode sig runtime.

  *

- *  TODO Next time we are making changes to the clamav-bytecode-compiler

- *  source, modify this structure to also include the section name (here and

- *  there).  Then, populate this field in the PE/MachO/ELF header parsing

- *  functions.  Choose a length that's reasonable for all platforms

+ *  If you need to make changes to this structure, you will need to update

+ *  it in both repos.  Also, I'm not sure whether changing this structure

+ *  would require a recompile of all previous bytecode sigs.  This should

+ *  be investigated before changes are made.

+ *

+ *  TODO Modify this structure to also include the section name (in both

+ *  repos).  Then, populate this field in the libclamav PE/MachO/ELF header

+ *  parsing functions.  Choose a length that's reasonable for all platforms

 */

 struct cli_exe_section {

     uint32_t rva;  /**< Relative VirtualAddress */

@@ -61,18 +63,17 @@ struct cli_exe_section {

  *  NOTE: This is used to store PE, MachO, and ELF executable information,

  *  but it predominantly has fields for PE info.  Not all members are

  *  populated by the respective parsing functions.

- * 

- *  TODO: Document which fields are PE only (maybe put those into a union of

- *  structs containing the various type-specific members)

-

- *  NOTE: This structure is also defined in the clamav-bytecode-compiler

- *  source at:

- *  - clang/lib/Headers/bytecode_execs.h

- *  - llvm/tools/clang/lib/Headers/bytecode_execs.h

- *  Based on how we use it, though, it doesn't need to stay in-sync

  *

- *  TODO Next time we are making changes to the clamav-bytecode-compiler

- *  source, remove this structure definition there so it's not confusing

+ *  NOTE: This header file originates in the clamav-devel source and gets

+ *  copied into the clamav-bytecode-compiler source through a script

+ *  (sync-clamav.sh). This is done because an array of cli_exe_section

+ *  structs is allocated by libclamav and passed to the bytecode sig

+ *  runtime.

+ *

+ *  This structure is not used by the bytecode sig runtime, so it can be

+ *  modified in the clamav-devel repo without requiring the changes to

+ *  be propagated to the clamav-bytecode-compile repo and that code rebuilt.

+ *  It'd be nice to keep them in sync if possible, though.

 */

 struct cli_exe_info {

     /** Information about all the sections of this file. 

@@ -91,15 +92,12 @@ struct cli_exe_info {

      */

     uint16_t nsections;

-    // TODO Remove - not required

-    void *dummy; /* for compat - preserve offset */

+    /***************** Begin PE-specific Section *****************/

-    /** Resources RVA - PE ONLY */

-    // TODO Maybe get rid of this - it looks like it's only used by the

-    // icon matching code (and maybe the bytecode API more broadly?)

+    /** Resources RVA */

     uint32_t res_addr;

-    /** Size of the  header (aligned) - PE ONLY. This corresponds to

+    /** Size of the  header (aligned). This corresponds to

      *  SizeOfHeaders in the optional header

     */

     uint32_t hdr_size;

@@ -140,7 +138,7 @@ struct cli_exe_info {

     /* Raw data copied in from the EXE directly.

      *

      * NOTE: The data in the members below haven't been converted to host

-     * endianness, so all field accesses most account for this to ensure

+     * endianness, so all field accesses must account for this to ensure

      * proper functionality on big endian systems (the PE header is always

      * little-endian)

      */

@@ -159,9 +157,10 @@ struct cli_exe_info {

      * the unpopulated entries will be memset'd to zero.

      */

     struct pe_image_data_dir dirs[16];

+

+    /***************** End PE-specific Section *****************/

 };

-// TODO Document

 void cli_exe_info_init(struct cli_exe_info *exeinfo, uint32_t offset);

 void cli_exe_info_destroy(struct cli_exe_info *exeinfo);

@@ -204,9 +204,16 @@ int cli_scanmacho(cli_ctx *ctx, struct cli_exe_info *fileinfo)

     fmap_t *map = *ctx->fmap;

     ssize_t at;

-    if (fileinfo)

+    if (fileinfo) {

         matcher = 1;

+        // TODO This code assumes fileinfo->offset == 0, which might not always

+        // be the case.  For now just print this debug message and continue on

+        if (0 != fileinfo->offset) {

+            cli_dbgmsg("cli_scanmacho: Assumption Violated: fileinfo->offset != 0\n");

+        }

+    }

+

     if (fmap_readn(map, &hdr, 0, sizeof(hdr)) != sizeof(hdr)) {

         cli_dbgmsg("cli_scanmacho: Can't read header\n");

         return matcher ? -1 : CL_EFORMAT;

@@ -504,23 +504,39 @@ int cli_caloff(const char *offstr, const struct cli_target_info *info, unsigned

     return CL_SUCCESS;

 }

+/**

+ * Initialize a struct cli_target_info so that it's ready to have its exeinfo

+ * populated by the call to cli_targetinfo and/or destroyed by

+ * cli_targetinfo_destroy.

+ *

+ * @param info a pointer to the struct cli_target_info to initialize

+ */

 void cli_targetinfo_init(struct cli_target_info *info)

 {

     if (NULL == info) {

         return;

     }

-    // Things that must be initialized here:

-    // - status (set to 0)

-    // - vinfo

-    // Maybe other things.

-    // TODO Consider replacing with just the required member assignments

-    // for performance

-    memset(info, 0, sizeof(struct cli_target_info));

-

-    cli_hashset_init_noalloc(&info->exeinfo.vinfo);

+    info->status = 0;

+    cli_exe_info_init(&(info->exeinfo), 0);

 }

+/** Parse the executable headers and, if successful, populate exeinfo

+ *

+ * @param info A structure to populate with info from the exe header. This

+ *             MUST be initialized via cli_targetinfo_init prior to calling

+ * @param target the target executable file type. Possible values are:

+ *               - 1 - PE32 / PE32+

+ *               - 6 - ELF

+ *               - 9 - MachO

+ * @param map The fmap_t backing the file being scanned

+ *

+ * @return If target refers to a supported executable file type, the exe header

+ *         will be parsed and, if successful, info->status will be set to 1.

+ *         If parsing the exe header fails, info->status will be set to -1.

+ *         The caller MUST destroy info via a call to cli_targetinfo_destroy

+ *         regardless of what info->status is set to.

+ */

 void cli_targetinfo(struct cli_target_info *info, unsigned int target, fmap_t *map)

 {

     int (*einfo)(fmap_t *, struct cli_exe_info *) = NULL;

@@ -542,14 +558,21 @@ void cli_targetinfo(struct cli_target_info *info, unsigned int target, fmap_t *m

         info->status = 1;

 }

+/**

+ * Free resources associated with a struct cli_target_info initialized

+ * via cli_targetinfo_init

+ *

+ * @param info a pointer to the struct cli_target_info to destroy

+ */

 void cli_targetinfo_destroy(struct cli_target_info *info)

 {

-    if (NULL == info || info->status != 1) {

+    if (NULL == info) {

         return;

     }

     cli_exe_info_destroy(&(info->exeinfo));

+    info->status = 0;

 }

 int cli_checkfp(unsigned char *digest, size_t size, cli_ctx *ctx)

@@ -701,6 +724,9 @@ int32_t cli_bcapi_matchicon(struct cli_bc_ctx *ctx, const uint8_t *grp1, int32_t

     const char **oldvirname;

     struct cli_exe_info info;

+    // TODO This isn't a good check, since EP will be zero for DLLs and

+    // (assuming pedata->ep is populated from exeinfo->pe) non-zero for

+    // some MachO and ELF executables

     if (!ctx->hooks.pedata->ep) {

         cli_dbgmsg("bytecode: matchicon only works with PE files\n");

         return -1;

@@ -981,7 +1007,18 @@ int cli_fmap_scandesc(cli_ctx *ctx, cli_file_t ftype, uint8_t ftonly, struct cli

      * (which is broken for signed binaries within signed binaries).

      * 

      * If we want to add support for more signature parsing in the future

-     * (Ex: MachO sigs), do that here too. */

+     * (Ex: MachO sigs), do that here too.

+     *

+     * One benefit of not continuing on to scan files with trusted signatures

+     * is that the bytes associated with the exe won't get counted against the

+     * scansize limits, which means we have an increased chance of catching

+     * malware in container types (NSIS, iShield, etc.) where the file size is

+     * large.  A common case where this occurs is installers that embed one

+     * or more of the various Microsoft Redistributable Setup packages.  These

+     * can easily be 5 MB or more in size, and might appear before malware

+     * does in a given sample.

+     */

+

     if (1 == info.status && i == 1) {

         ret = cli_check_auth_header(ctx, &(info.exeinfo));

@@ -373,6 +373,10 @@ void findres(uint32_t by_type, uint32_t by_name, fmap_t *map, struct cli_exe_inf

         return;

     }

+    if (0 != peinfo->offset) {

+        cli_dbgmsg("findres: Assumption Violated: Looking for version info when peinfo->offset != 0\n");

+    }

+

     res_rva = EC32(peinfo->dirs[2].VirtualAddress);

     if (!(resdir = fmap_need_off_once(map, cli_rawaddr(res_rva, peinfo->sections, peinfo->nsections, &err, map->len, peinfo->hdr_size), 16)) || err)

@@ -3305,6 +3309,8 @@ int cli_scanpe(cli_ctx *ctx)

         uint32_t fileoffset;

         const char *tbuff;

+        // TODO shouldn't peinfo->ep be used here instead?  ep is the file

+        // offset, vep is the entry point RVA

         fileoffset = (peinfo->vep + cli_readint32(epbuff + 1) + 5);

         while (fileoffset == 0x154 || fileoffset == 0x158) {

             char *src;

@@ -4450,7 +4456,6 @@ int cli_pe_targetinfo(fmap_t *map, struct cli_exe_info *peinfo)

  * @param map The fmap_t backing the file being scanned

  * @param peinfo A structure to populate with info from the PE header. This

  *               MUST be initialized via cli_exe_info_init prior to calling

- *               being passed in.

  * @param opts A bitfield indicating various options related to PE header

  *             parsing.  The options are (prefixed with CLI_PEHEADER_OPT_):

  *              - NONE - Do default parsing

@@ -4474,9 +4479,8 @@ int cli_pe_targetinfo(fmap_t *map, struct cli_exe_info *peinfo)

  *         is returned. If it seems like the PE is broken,

  *         CLI_PEHEADER_RET_BROKEN_PE is returned.  Otherwise, one of the

  *         other error codes is returned.

- *         If CLI_PEHEADER_RET_SUCCESS is returned, it is the caller's

- *         responsibility to destroy peinfo.  Otherwise, it's safe to do

- *         so but not required.

+ *         The caller MUST destroy peinfo, regardless of what this function

+ *         returns.

  *

  * TODO What constitutes a "broken PE" seems somewhat arbitrary in places.

  * I think a PE should only be considered broken if it will not run on

@@ -4506,7 +4510,7 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     struct pe_image_file_hdr *file_hdr;

     struct pe_image_optional_hdr32 *opt32;

     struct pe_image_optional_hdr64 *opt64;

-    struct pe_image_section_hdr *section_hdrs;

+    struct pe_image_section_hdr *section_hdrs = NULL;

     unsigned int i, j, section_pe_idx;

     unsigned int err;

     uint32_t salign, falign;

@@ -4516,6 +4520,7 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     uint32_t is_exe = 0;

     int native      = 0;

     int read;

+    int ret = CLI_PEHEADER_RET_GENERIC_ERROR;

 #if HAVE_JSON

     int toval                   = 0;

     struct json_object *pe_json = NULL;

@@ -4526,7 +4531,7 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         (opts & CLI_PEHEADER_OPT_COLLECT_JSON ||

          opts & CLI_PEHEADER_OPT_DBG_PRINT_INFO)) {

         cli_errmsg("cli_peheader: ctx can't be NULL for options specified\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

 #if HAVE_JSON

@@ -4538,18 +4543,19 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     fsize = map->len - peinfo->offset;

     if (fmap_readn(map, &e_magic, peinfo->offset, sizeof(e_magic)) != sizeof(e_magic)) {

         cli_dbgmsg("cli_peheader: Can't read DOS signature\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     if (EC16(e_magic) != PE_IMAGE_DOS_SIGNATURE && EC16(e_magic) != PE_IMAGE_DOS_SIGNATURE_OLD) {

         cli_dbgmsg("cli_peheader: Invalid DOS signature\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     if (fmap_readn(map, &(peinfo->e_lfanew), peinfo->offset + 58 + sizeof(e_magic), sizeof(peinfo->e_lfanew)) != sizeof(peinfo->e_lfanew)) {

         /* truncated header? */

         cli_dbgmsg("cli_peheader: Unable to read e_lfanew - truncated header?\n");

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     peinfo->e_lfanew = EC32(peinfo->e_lfanew);

@@ -4558,20 +4564,20 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     }

     if (!peinfo->e_lfanew) {

         cli_dbgmsg("cli_peheader: Not a PE file - e_lfanew == 0\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     if (fmap_readn(map, &(peinfo->file_hdr), peinfo->offset + peinfo->e_lfanew, sizeof(struct pe_image_file_hdr)) != sizeof(struct pe_image_file_hdr)) {

         /* bad information in e_lfanew - probably not a PE file */

         cli_dbgmsg("cli_peheader: Can't read file header\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     file_hdr = &(peinfo->file_hdr);

     if (EC32(file_hdr->Magic) != PE_IMAGE_NT_SIGNATURE) {

         cli_dbgmsg("cli_peheader: Invalid PE signature (probably NE file)\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     if (EC16(file_hdr->Characteristics) & 0x2000) {

@@ -4745,7 +4751,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

                 cli_dbgmsg("cli_peheader: Invalid NumberOfSections (>%d)\n", PE_MAXSECTIONS);

             }

         }

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     timestamp    = (time_t)EC32(file_hdr->TimeDateStamp);

@@ -4753,13 +4760,14 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     if (opts & CLI_PEHEADER_OPT_DBG_PRINT_INFO) {

         cli_dbgmsg("NumberOfSections: %d\n", peinfo->nsections);

-        cli_dbgmsg("TimeDateStamp: %s\n", cli_ctime(&timestamp, timestr, sizeof(timestr)));

+        cli_dbgmsg("TimeDateStamp: %s", cli_ctime(&timestamp, timestr, sizeof(timestr)));

         cli_dbgmsg("SizeOfOptionalHeader: 0x%x\n", opt_hdr_size);

     }

 #if HAVE_JSON

     if (opts & CLI_PEHEADER_OPT_COLLECT_JSON) {

         cli_jsonint(pe_json, "NumberOfSections", peinfo->nsections);

+        /* NOTE: the TimeDateStamp value will look like "Wed Dec 31 19:00:00 1969\n" */

         cli_jsonstr(pe_json, "TimeDateStamp", cli_ctime(&timestamp, timestr, sizeof(timestr)));

         cli_jsonint(pe_json, "SizeOfOptionalHeader", opt_hdr_size);

     }

@@ -4776,13 +4784,15 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

             pe_add_heuristic_property(ctx, "BadOptionalHeaderSize");

         }

 #endif

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     at = peinfo->offset + peinfo->e_lfanew + sizeof(struct pe_image_file_hdr);

     if (fmap_readn(map, &(peinfo->pe_opt.opt32), at, sizeof(struct pe_image_optional_hdr32)) != sizeof(struct pe_image_optional_hdr32)) {

         cli_dbgmsg("cli_peheader: Can't read optional file header\n");

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     stored_opt_hdr_size = sizeof(struct pe_image_optional_hdr32);

     at += stored_opt_hdr_size;

@@ -4800,12 +4810,14 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

                 pe_add_heuristic_property(ctx, "BadOptionalHeaderSizePE32Plus");

             }

 #endif

-            return CLI_PEHEADER_RET_BROKEN_PE;

+            ret = CLI_PEHEADER_RET_BROKEN_PE;

+            goto done;

         }

         if (fmap_readn(map, (void *)(((size_t) & (peinfo->pe_opt.opt64)) + sizeof(struct pe_image_optional_hdr32)), at, OPT_HDR_SIZE_DIFF) != OPT_HDR_SIZE_DIFF) {

             cli_dbgmsg("cli_peheader: Can't read additional optional file header bytes\n");

-            return CLI_PEHEADER_RET_BROKEN_PE;

+            ret = CLI_PEHEADER_RET_BROKEN_PE;

+            goto done;

         }

         stored_opt_hdr_size += OPT_HDR_SIZE_DIFF;

@@ -4987,14 +4999,16 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     if (!native && (!salign || (salign % 0x1000))) {

         cli_dbgmsg("cli_peheader: Bad section alignment\n");

         if (opts & CLI_PEHEADER_OPT_STRICT_ON_PE_ERRORS) {

-            return CLI_PEHEADER_RET_BROKEN_PE;

+            ret = CLI_PEHEADER_RET_BROKEN_PE;

+            goto done;

         }

     }

     if (!native && (!falign || (falign % 0x200))) {

         cli_dbgmsg("cli_peheader: Bad file alignment\n");

         if (opts & CLI_PEHEADER_OPT_STRICT_ON_PE_ERRORS) {

-            return CLI_PEHEADER_RET_BROKEN_PE;

+            ret = CLI_PEHEADER_RET_BROKEN_PE;

+            goto done;

         }

     }

@@ -5023,13 +5037,14 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     if (opt_hdr_size < (stored_opt_hdr_size + data_dirs_size)) {

         cli_dbgmsg("cli_peheader: SizeOfOptionalHeader too small (doesn't include data dir size)\n");

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     read = fmap_readn(map, peinfo->dirs, at, data_dirs_size);

     if (read < 0 || (uint32_t)read != data_dirs_size) {

         cli_dbgmsg("cli_peheader: Can't read optional file header data dirs\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     at += data_dirs_size;

@@ -5060,25 +5075,21 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     if (!peinfo->sections) {

         cli_dbgmsg("cli_peheader: Can't allocate memory for section headers\n");

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     section_hdrs = (struct pe_image_section_hdr *)cli_calloc(peinfo->nsections, sizeof(struct pe_image_section_hdr));

     if (!section_hdrs) {

         cli_dbgmsg("cli_peheader: Can't allocate memory for section headers\n");

-        free(peinfo->sections);

-        peinfo->sections = NULL;

-        return CLI_PEHEADER_RET_GENERIC_ERROR;

+        goto done;

     }

     read = fmap_readn(map, section_hdrs, at, peinfo->nsections * sizeof(struct pe_image_section_hdr));

     if (read < 0 || (uint32_t)read != peinfo->nsections * sizeof(struct pe_image_section_hdr)) {

         cli_dbgmsg("cli_peheader: Can't read section header - possibly broken PE file\n");

-        free(section_hdrs);

-        free(peinfo->sections);

-        peinfo->sections = NULL;

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

     at += sizeof(struct pe_image_section_hdr) * peinfo->nsections;

@@ -5122,10 +5133,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

             if (section->raw >= fsize || section->uraw >= fsize) {

                 cli_dbgmsg("cli_peheader: Broken PE file - Section %d starts or exists beyond the end of file (Offset@ %lu, Total filesize %lu)\n", section_pe_idx, (unsigned long)section->raw, (unsigned long)fsize);

                 if (peinfo->nsections == 1) {

-                    free(section_hdrs);

-                    free(peinfo->sections);

-                    peinfo->sections = NULL;

-                    return CLI_PEHEADER_RET_BROKEN_PE;

+                    ret = CLI_PEHEADER_RET_BROKEN_PE;

+                    goto done;

                 }

                 for (j = i; j < peinfo->nsections - 1; j++)

@@ -5161,10 +5170,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

             add_section_info(ctx, &peinfo->sections[i]);

             if (cli_json_timeout_cycle_check(ctx, &toval) != CL_SUCCESS) {

-                free(section_hdrs);

-                free(peinfo->sections);

-                peinfo->sections = NULL;

-                return CLI_PEHEADER_RET_JSON_TIMEOUT;

+                ret = CLI_PEHEADER_RET_JSON_TIMEOUT;

+                goto done;

             }

         }

 #endif

@@ -5207,10 +5214,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         if (!salign || (section->urva % salign)) { /* Bad section alignment */

             cli_dbgmsg("cli_peheader: Broken PE - section's VirtualAddress is misaligned\n");

             if (opts & CLI_PEHEADER_OPT_STRICT_ON_PE_ERRORS) {

-                free(section_hdrs);

-                free(peinfo->sections);

-                peinfo->sections = NULL;

-                return CLI_PEHEADER_RET_BROKEN_PE;

+                ret = CLI_PEHEADER_RET_BROKEN_PE;

+                goto done;

             }

         }

@@ -5218,22 +5223,16 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         // section? Why the exception for uraw?

         if (section->urva >> 31 || section->uvsz >> 31 || (section->rsz && section->uraw >> 31) || peinfo->sections[i].ursz >> 31) {

             cli_dbgmsg("cli_peheader: Found PE values with sign bit set\n");

-

-            free(section_hdrs);

-            free(peinfo->sections);

-            peinfo->sections = NULL;

-

-            return CLI_PEHEADER_RET_BROKEN_PE;

+            ret = CLI_PEHEADER_RET_BROKEN_PE;

+            goto done;

         }

         if (!i) {

             if (section->urva != peinfo->hdr_size) { /* Bad first section RVA */

                 cli_dbgmsg("cli_peheader: First section doesn't start immediately after the header\n");

                 if (opts & CLI_PEHEADER_OPT_STRICT_ON_PE_ERRORS) {

-                    free(section_hdrs);

-                    free(peinfo->sections);

-                    peinfo->sections = NULL;

-                    return CLI_PEHEADER_RET_BROKEN_PE;

+                    ret = CLI_PEHEADER_RET_BROKEN_PE;

+                    goto done;

                 }

             }

@@ -5243,10 +5242,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

             if (section->urva - peinfo->sections[i - 1].urva != peinfo->sections[i - 1].vsz) { /* No holes, no overlapping, no virtual disorder */

                 cli_dbgmsg("cli_peheader: Virtually misplaced section (wrong order, overlapping, non contiguous)\n");

                 if (opts & CLI_PEHEADER_OPT_STRICT_ON_PE_ERRORS) {

-                    free(section_hdrs);

-                    free(peinfo->sections);

-                    peinfo->sections = NULL;

-                    return CLI_PEHEADER_RET_BROKEN_PE;

+                    ret = CLI_PEHEADER_RET_BROKEN_PE;

+                    goto done;

                 }

             }

@@ -5268,15 +5265,12 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     peinfo->overlay_size = fsize - peinfo->overlay_start;

-    free(section_hdrs);

-

     // NOTE: For DLLs the entrypoint is likely to be zero

-    // TODO ^^^ Does this mean this doesn't work for DLLs?

+    // TODO Should this offset include peinfo->offset?

     if (!(peinfo->ep = cli_rawaddr(peinfo->vep, peinfo->sections, peinfo->nsections, &err, fsize, peinfo->hdr_size)) && err) {

         cli_dbgmsg("cli_peheader: Broken PE file - Can't map EntryPoint to a file offset\n");

-        free(peinfo->sections);

-        peinfo->sections = NULL;

-        return CLI_PEHEADER_RET_BROKEN_PE;

+        ret = CLI_PEHEADER_RET_BROKEN_PE;

+        goto done;

     }

 #if HAVE_JSON

@@ -5284,9 +5278,8 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         cli_jsonint(pe_json, "EntryPointOffset", peinfo->ep);

         if (cli_json_timeout_cycle_check(ctx, &toval) != CL_SUCCESS) {

-            free(peinfo->sections);

-            peinfo->sections = NULL;

-            return CLI_PEHEADER_RET_JSON_TIMEOUT;

+            ret = CLI_PEHEADER_RET_JSON_TIMEOUT;

+            goto done;

         }

     }

 #endif

@@ -5306,6 +5299,12 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         const uint8_t *vptr, *baseptr;

         uint32_t rva, res_sz;

+        // TODO This code assumes peinfo->offset == 0, which might not always

+        // be the case.

+        if (0 != peinfo->offset) {

+            cli_dbgmsg("cli_peheader: Assumption Violated: Looking for version info when peinfo->offset != 0\n");

+        }

+

         memset(&vlist, 0, sizeof(vlist));

         findres(0x10, 0xffffffff, map, peinfo, versioninfo_cb, &vlist);

         if (!vlist.count)

@@ -5313,9 +5312,7 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

         if (cli_hashset_init(&peinfo->vinfo, 32, 80)) {

             cli_errmsg("cli_peheader: Unable to init vinfo hashset\n");

-            free(peinfo->sections);

-            peinfo->sections = NULL;

-            return CLI_PEHEADER_RET_GENERIC_ERROR;

+            goto done;

         }

         err = 0;

@@ -5446,10 +5443,7 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

                             if (cli_hashset_addkey(&peinfo->vinfo, (uint32_t)(vptr - baseptr + 6))) {

                                 cli_errmsg("cli_peheader: Unable to add rva to vinfo hashset\n");

-                                cli_hashset_destroy(&peinfo->vinfo);

-                                free(peinfo->sections);

-                                peinfo->sections = NULL;

-                                return CLI_PEHEADER_RET_GENERIC_ERROR;

+                                goto done;

                             }

                             if (cli_debug_flag) {

@@ -5487,7 +5481,16 @@ int cli_peheader(fmap_t *map, struct cli_exe_info *peinfo, uint32_t opts, cli_ct

     // Do final preperations for peinfo to be passed back

     peinfo->is_dll = is_dll;

-    return CLI_PEHEADER_RET_SUCCESS;

+    ret = CLI_PEHEADER_RET_SUCCESS;

+

+done:

+    /* In the fail case, peinfo will get destroyed by the caller */

+

+    if (NULL != section_hdrs) {

+        free(section_hdrs);

+    }

+

+    return ret;

 }

 // TODO We should sort based on VirtualAddress instead, since PointerToRawData

@@ -1,6 +1,6 @@

 /*

- *  Copyright (C) 2015, 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

- *  Copyright (C) 2007-2008 Sourcefire, Inc.

+ *  Copyright (C) 2013-2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.

+ *  Copyright (C) 2007-2013 Sourcefire, Inc.

  *

  *  Authors: Alberto Wu, Tomasz Kojm, Andrew Williams

  * 

@@ -4529,7 +4529,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

     if ((dd = opendir(dirname)) == NULL) {

         cli_errmsg("cli_loaddbdir(): Can't open directory %s\n", dirname);

         ret = CL_EOPEN;

-        goto cleanup;

+        goto done;

     }

     dirname_len = strlen(dirname);

@@ -4564,7 +4564,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

         if (!dbfile) {

             cli_errmsg("cli_loaddbdir(): dbfile == NULL\n");

             ret = CL_EMEM;

-            goto cleanup;

+            goto done;

         }

         if (ends_with_sep)

             sprintf(dbfile, "%s%s", dirname, dent->d_name);

@@ -4585,6 +4585,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

         } else if (!strcmp(dent->d_name, "daily.cld")) {

             /* the daily db must be loaded before main */

+            // TODO Why is this the case?

             load_priority = DB_LOAD_PRIORITY_DAILY_CLD;

             have_daily_cld = !access(dbfile, R_OK);

@@ -4593,7 +4594,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

                 if (!daily_cld) {

                     cli_errmsg("cli_loaddbdir(): error parsing header of %s\n", dbfile);

                     ret = CL_EMALFDB;

-                    goto cleanup;

+                    goto done;

                 }

             }

@@ -4606,7 +4607,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

                 if (!daily_cvd) {

                     cli_errmsg("cli_loaddbdir(): error parsing header of %s\n", dbfile);

                     ret = CL_EMALFDB;

-                    goto cleanup;

+                    goto done;

                 }

             }

@@ -4639,7 +4640,7 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

         if (NULL == entry) {

             cli_errmsg("cli_loaddbdir(): entry == NULL\n");

             ret = CL_EMEM;

-            goto cleanup;

+            goto done;

         }

         entry->path          = dbfile;

@@ -4651,18 +4652,20 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

     /* The list entries are stored in priority order, so now just loop through

      * and load everything.

      * NOTE: If there's a daily.cld and a daily.cvd, we'll only load whichever

-     * has the highest version number. */

-

-    // TODO Should we treat all cld/cvd pairs like we do the daily ones?

+     * has the highest version number.  If they have the same version number

+     * we load daily.cld, since that will load faster (it won't attempt to

+     * verify the digital signature of the db).

+     * TODO It'd be ideal if we treated all cld/cvd pairs like we do the daily

+     * ones, and only loaded the one with the highest version. */

     for (iter = head; iter != NULL; iter = iter->next) {

         if (DB_LOAD_PRIORITY_DAILY_CLD == iter->load_priority && have_daily_cvd) {

-            if (daily_cld->version <= daily_cvd->version) {

+            if (daily_cld->version < daily_cvd->version) {

                 continue;

             }

         } else if (DB_LOAD_PRIORITY_DAILY_CVD == iter->load_priority && have_daily_cld) {

-            if (daily_cld->version > daily_cvd->version) {

+            if (daily_cld->version >= daily_cvd->version) {

                 continue;

             }

         }

@@ -4670,11 +4673,11 @@ static int cli_loaddbdir(const char *dirname, struct cl_engine *engine, unsigned

         ret = cli_load(iter->path, engine, signo, options, NULL);

         if (ret) {

             cli_errmsg("cli_loaddbdir(): error loading database %s\n", iter->path);

-            goto cleanup;

+            goto done;

         }

     }

-cleanup:

+done:

     for (iter = head; iter != NULL; iter = next) {

         next = iter->next;

         free(iter->path);

@@ -4763,7 +4766,7 @@ int cl_load(const char *path, struct cl_engine *engine, unsigned int *signo, uns

         cli_dbgmsg("Bytecode engine disabled\n");

     }

-    if (cli_cache_init(engine))

+    if (!engine->cache && cli_cache_init(engine))

         return CL_EMEM;

     engine->dboptions |= dboptions;

@@ -5085,6 +5088,11 @@ int cl_engine_free(struct cl_engine *engine)

         mpool_free(engine->mempool, root);

     }

+    if ((root = engine->hm_imp)) {

+        hm_free(root);

+        mpool_free(engine->mempool, root);

+    }

+

     if ((root = engine->hm_fp)) {

         hm_free(root);

         mpool_free(engine->mempool, root);

@@ -5255,6 +5263,9 @@ int cl_engine_compile(struct cl_engine *engine)

     if (engine->hm_mdb)

         hm_flush(engine->hm_mdb);

+    if (engine->hm_imp)

+        hm_flush(engine->hm_imp);

+

     if (engine->hm_fp)

         hm_flush(engine->hm_fp);

@@ -28,19 +28,14 @@

 #include "str.h"

 #include "cvd.h"

-// TODO Is it safe to remove .db2 and .db3 from here?  These strings aren't

-// referenced anywhere else in the source.

-

-// NOTE: We don't include .info in CLI_DBEXT because they are only used for

-// one specific purpose - verifying the contents of database container files.

-// This list is geared towards file extensions of files that users can provide

-// to ClamAV directly.

+/* NOTE: We don't include .info in CLI_DBEXT because they are only used for

+ * one specific purpose - verifying the contents of database container files.

+ * This list is geared towards file extensions of files that users can provide

+ * to ClamAV directly. */

 #ifdef HAVE_YARA

 #define CLI_DBEXT(ext)                   \

     (                                    \

         cli_strbcasestr(ext, ".db") ||   \

-        cli_strbcasestr(ext, ".db2") ||  \