Valgrind suppression for false positive in Rust png-decoder crate

If the current layer has a file descriptor, ClamAV is passing the path
for that file to the UnRAR module, even if the RAR we want to scan is
just some small embedded bit (e.g. detected by RARSFX signature).

We need to drop the RAR portion to a new file for the UnRAR module
because it does not accept file buffers to be scanned, only file paths.

CLAM-2900

Note this commit also changes `scanners.c` to use `access()` on Windows
instead of `_access_s()`. ClamAV defines `access()` to map to a custom
`access_w32()` function on Windows. We already use it everywhere else.

libclamav/matcher-ac.c

@@ -833,8 +833,9 @@ int cli_ac_chklsig(const char *expr, const char *end, uint32_t *lsigcnt, unsigne
                     return -1;
                 }
 
-                for (i += 2; i + 1 < len && (isdigit(expr[i + 1]) || expr[i + 1] == ','); i++)
+                for (i += 2; i + 1 < len && (isdigit(expr[i + 1]) || expr[i + 1] == ','); i++) {
-                    ;
+                    continue;
+                }
             }
 
             if (&expr[i + 1] == rend)
@@ -1625,19 +1626,23 @@ void cli_ac_freedata(struct cli_ac_data *data)
     }
 }
 
-/* returns only CL_SUCCESS or CL_EMEM */
+/**
-inline static int ac_addtype(struct cli_matched_type **list, cli_file_t type, off_t offset, const cli_ctx *ctx)
+ * @brief Add a match for an object type to the list of matched types.
+ *
+ * Important: The caller is responsible for checking limits!
+ *
+ * @param list Pointer to the list of matched types. *list may be NULL if no types have been added yet.
+ * @param type The type of the embedded object.
+ * @param offset The offset of the embedded object.
+ * @param ctx The context information. May be NULL.
+ * @return cl_error_t CL_SUCCESS regardless if added, or CL_EMEM if memory allocation failed.
+ */
+inline static cl_error_t ac_addtype(struct cli_matched_type **list, cli_file_t type, off_t offset, const cli_ctx *ctx)
 {
-    struct cli_matched_type *tnode, *tnode_last;
+    struct cli_matched_type *tnode;
 
-    if (type == CL_TYPE_ZIPSFX) {
+    tnode = calloc(1, sizeof(struct cli_matched_type));
-        if (*list && ctx && ctx->engine->maxfiles && (*list)->cnt > ctx->engine->maxfiles)
+    if (NULL == tnode) {
-            return CL_SUCCESS;
-    } else if (*list && (*list)->cnt >= MAX_EMBEDDED_OBJ) {
-        return CL_SUCCESS;
-    }
-
-    if (!(tnode = calloc(1, sizeof(struct cli_matched_type)))) {
         cli_errmsg("cli_ac_addtype: Can't allocate memory for new type node\n");
         return CL_EMEM;
     }
@@ -1645,16 +1650,25 @@ inline static int ac_addtype(struct cli_matched_type **list, cli_file_t type, of
     tnode->type   = type;
     tnode->offset = offset;
 
-    tnode_last = *list;
+    if (*list) {
-    while (tnode_last && tnode_last->next)
+        // Add to end of existing list.
-        tnode_last = tnode_last->next;
+        struct cli_matched_type *tnode_last = *list;
 
-    if (tnode_last)
+        while (tnode_last && tnode_last->next) {
+            tnode_last = tnode_last->next;
+        }
         tnode_last->next = tnode;
-    else
+    } else {
+        // First type in the list.
         *list = tnode;
+    }
 
     (*list)->cnt++;
+
+    if (UNLIKELY(cli_get_debug_flag())) {
+        cli_dbgmsg("ac_addtype: added %s embedded object at offset " STDi64 ". Embedded object count: %d\n", cli_ftname(type), (uint64_t)offset, (*list)->cnt);
+    }
+
     return CL_SUCCESS;
 }
 
@@ -1999,14 +2013,65 @@ cl_error_t cli_ac_scanbuff(
 
                                         cli_dbgmsg("Matched signature for file type %s\n", pt->virname);
                                         type = pt->type;
-                                        if ((ftoffset != NULL) &&
+
-                                            ((*ftoffset == NULL) || (*ftoffset)->cnt < MAX_EMBEDDED_OBJ || type == CL_TYPE_ZIPSFX) && (type >= CL_TYPE_SFX || ((ftype == CL_TYPE_MSEXE || ftype == CL_TYPE_ZIP || ftype == CL_TYPE_MSOLE2) && type == CL_TYPE_MSEXE))) {
+                                        if (ftoffset != NULL) {
+                                            // Caller provided a pointer to record matched types.
+                                            bool too_many_types = false;
+                                            bool supported_type = false;
+
+                                            if (*ftoffset != NULL) {
+                                                // Have some type matches already. Check limits.
+
+                                                if (ctx && ((type == CL_TYPE_ZIPSFX) ||
+                                                            (type == CL_TYPE_MSEXE && ftype == CL_TYPE_MSEXE))) {
+                                                    // When ctx present, limit the number of type matches using ctx->engine->maxfiles for specific types.
+                                                    // Reasoning:
+                                                    //   ZIP local file header entries likely to be numerous if a single ZIP appended to the scanned file.
+                                                    //   MSEXE can contain many embedded MSEXE entries and MSEXE type false positives matches.
+
+                                                    if (ctx->engine->maxfiles == 0) {
+                                                        // Max-files limit is disabled.
+                                                    } else if ((*ftoffset)->cnt >= ctx->engine->maxfiles) {
+                                                        if (UNLIKELY(cli_get_debug_flag())) {
+                                                            cli_dbgmsg("ac_addtype: Can't add %s type at offset " STDu64 " to list of embedded type matches. Reached maxfiles limit of %u\n", cli_ftname(type), (*ftoffset)->offset, ctx->engine->maxfiles);
+                                                        }
+                                                        too_many_types = true;
+                                                    }
+                                                } else {
+                                                    // Limit the number of type matches using MAX_EMBEDDED_OBJ.
+                                                    if ((*ftoffset)->cnt >= MAX_EMBEDDED_OBJ) {
+                                                        if (UNLIKELY(cli_get_debug_flag())) {
+                                                            cli_dbgmsg("ac_addtype: Can't add %s type at offset " STDu64 " to list of embedded type matches. Reached MAX_EMBEDDED_OBJ limit of %u\n", cli_ftname(type), (*ftoffset)->offset, MAX_EMBEDDED_OBJ);
+                                                        }
+                                                        too_many_types = true;
+                                                    }
+                                                }
+                                            }
+
+                                            // Filter to supported types.
+                                            if (
+                                                // Found type is MBR.
+                                                type == CL_TYPE_MBR ||
+                                                // Found type is any SFX type (i.e., ZIPSFX, RARSFX, 7ZSSFX, etc.).
+                                                type >= CL_TYPE_SFX ||
+                                                // Found type is an MSEXE, but only if host file type is one of MSEXE, ZIP, or MSOLE2.
+                                                (type == CL_TYPE_MSEXE && (ftype == CL_TYPE_MSEXE || ftype == CL_TYPE_ZIP || ftype == CL_TYPE_MSOLE2))) {
+
+                                                supported_type = true;
+                                            }
+
+                                            if (supported_type && !too_many_types) {
                                                 /* FIXME: the first offset in the array is most likely the correct one but
                                                  * it may happen it is not
+                                                 * Until we're certain and can fix this, we add all offsets in the list.
                                                  */
-                                            for (j = 1; j <= CLI_DEFAULT_AC_TRACKLEN + 1 && offmatrix[0][j] != (uint32_t)-1; j++)
+                                                for (j = 1; j <= CLI_DEFAULT_AC_TRACKLEN + 1 && offmatrix[0][j] != (uint32_t)-1; j++) {
-                                                if (ac_addtype(ftoffset, type, offmatrix[pt->parts - 1][j], ctx))
+                                                    ret = ac_addtype(ftoffset, type, offmatrix[pt->parts - 1][j], ctx);
-                                                    return CL_EMEM;
+                                                    if (CL_SUCCESS != ret) {
+                                                        return ret;
+                                                    }
+                                                }
+                                            }
                                         }
 
                                         memset(offmatrix[0], (uint32_t)-1, pt->parts * (CLI_DEFAULT_AC_TRACKLEN + 2) * sizeof(uint32_t));
@@ -2066,11 +2131,59 @@ cl_error_t cli_ac_scanbuff(
 
                                     cli_dbgmsg("Matched signature for file type %s at %u\n", pt->virname, realoff);
                                     type = pt->type;
-                                    if ((ftoffset != NULL) &&
-                                        ((*ftoffset == NULL) || (*ftoffset)->cnt < MAX_EMBEDDED_OBJ || type == CL_TYPE_ZIPSFX) && (type == CL_TYPE_MBR || type >= CL_TYPE_SFX || ((ftype == CL_TYPE_MSEXE || ftype == CL_TYPE_ZIP || ftype == CL_TYPE_MSOLE2) && type == CL_TYPE_MSEXE))) {
 
-                                        if (ac_addtype(ftoffset, type, realoff, ctx))
+                                    if (ftoffset != NULL) {
-                                            return CL_EMEM;
+                                        // Caller provided a pointer to record matched types.
+                                        bool too_many_types = false;
+                                        bool supported_type = false;
+
+                                        if (*ftoffset != NULL) {
+                                            // Have some type matches already. Check limits.
+
+                                            if (ctx && ((type == CL_TYPE_ZIPSFX) ||
+                                                        (type == CL_TYPE_MSEXE && ftype == CL_TYPE_MSEXE))) {
+                                                // When ctx present, limit the number of type matches using ctx->engine->maxfiles for specific types.
+                                                // Reasoning:
+                                                //   ZIP local file header entries likely to be numerous if a single ZIP appended to the scanned file.
+                                                //   MSEXE can contain many embedded MSEXE entries and MSEXE type false positives matches.
+
+                                                if (ctx->engine->maxfiles == 0) {
+                                                    // Max-files limit is disabled.
+                                                } else if ((*ftoffset)->cnt >= ctx->engine->maxfiles) {
+                                                    if (UNLIKELY(cli_get_debug_flag())) {
+                                                        cli_dbgmsg("ac_addtype: Can't add %s type at offset " STDu64 " to list of embedded type matches. Reached maxfiles limit of %u\n", cli_ftname(type), (*ftoffset)->offset, ctx->engine->maxfiles);
+                                                    }
+                                                    too_many_types = true;
+                                                }
+                                            } else {
+                                                // Limit the number of type matches using MAX_EMBEDDED_OBJ.
+                                                if ((*ftoffset)->cnt >= MAX_EMBEDDED_OBJ) {
+                                                    if (UNLIKELY(cli_get_debug_flag())) {
+                                                        cli_dbgmsg("ac_addtype: Can't add %s type at offset " STDu64 " to list of embedded type matches. Reached MAX_EMBEDDED_OBJ limit of %u\n", cli_ftname(type), (*ftoffset)->offset, MAX_EMBEDDED_OBJ);
+                                                    }
+                                                    too_many_types = true;
+                                                }
+                                            }
+                                        }
+
+                                        // Filter to supported types.
+                                        if (
+                                            // Found type is MBR.
+                                            type == CL_TYPE_MBR ||
+                                            // Found type is any SFX type (i.e., ZIPSFX, RARSFX, 7ZSSFX, etc.).
+                                            type >= CL_TYPE_SFX ||
+                                            // Found type is an MSEXE, but only if host file type is one of MSEXE, ZIP, or MSOLE2.
+                                            (type == CL_TYPE_MSEXE && (ftype == CL_TYPE_MSEXE || ftype == CL_TYPE_ZIP || ftype == CL_TYPE_MSOLE2))) {
+
+                                            supported_type = true;
+                                        }
+
+                                        if (supported_type && !too_many_types) {
+                                            ret = ac_addtype(ftoffset, type, realoff, ctx);
+                                            if (CL_SUCCESS != ret) {
+                                                return ret;
+                                            }
+                                        }
                                     }
                                 }
                             } else {

libclamav/matcher.h

@@ -390,6 +390,6 @@ cl_error_t cli_matchmeta(cli_ctx *ctx, const char *fname, size_t fsizec, size_t
  *               - 9 - MachO
  * @param ctx The current scan context
  */
-void cli_targetinfo(struct cli_target_info *info, unsigned int target, cli_ctx *ctx);
+void cli_targetinfo(struct cli_target_info *info, cli_target_t target, cli_ctx *ctx);
 
 #endif

libclamav/scanners.c

@@ -418,11 +418,7 @@ static cl_error_t cli_scanrar_file(const char *filepath, int desc, cli_ctx *ctx)
                      * File should be extracted...
                      * ... make sure we have read permissions to the file.
                      */
-#ifdef _WIN32
-                    if (0 != _access_s(extract_fullpath, R_OK)) {
-#else
                     if (0 != access(extract_fullpath, R_OK)) {
-#endif
                         cli_dbgmsg("RAR: Don't have read permissions, attempting to change file permissions to make it readable..\n");
 #ifdef _WIN32
                         if (0 != _chmod(extract_fullpath, _S_IREAD)) {
@@ -533,11 +529,11 @@ static cl_error_t cli_scanrar(cli_ctx *ctx)
     char *tmpname = NULL;
     int tmpfd     = -1;
 
-#ifdef _WIN32
+    if ((SCAN_UNPRIVILEGED) ||
-    if ((SCAN_UNPRIVILEGED) || (NULL == ctx->fmap->path) || (0 != _access_s(ctx->fmap->path, R_OK))) {
+        (NULL == ctx->fmap->path) ||
-#else
+        (0 != access(ctx->fmap->path, R_OK)) ||
-    if ((SCAN_UNPRIVILEGED) || (NULL == ctx->fmap->path) || (0 != access(ctx->fmap->path, R_OK))) {
+        (ctx->fmap->nested_offset > 0) || (ctx->fmap->len < ctx->fmap->real_len)) {
-#endif
+
         /* If map is not file-backed have to dump to file for scanrar. */
         status = fmap_dump_to_file(ctx->fmap, ctx->fmap->path, ctx->this_layer_tmpdir, &tmpname, &tmpfd, 0, SIZE_MAX);
         if (status != CL_SUCCESS) {
@@ -1665,7 +1661,7 @@ static cl_error_t cli_ole2_tempdir_scan_vba_new(const char *dir, cli_ctx *ctx, s
                 goto done;
             }
 
-            ret = cli_scan_desc(tempfd, ctx, CL_TYPE_SCRIPT, false, NULL, AC_SCAN_VIR, NULL, "extracted-vba-project", tempfile, LAYER_ATTRIBUTES_NORMALIZED);
+            ret = cli_scan_desc(tempfd, ctx, CL_TYPE_SCRIPT, false, NULL, AC_SCAN_VIR, NULL, "extracted-vba-project", tempfile, LAYER_ATTRIBUTES_NONE);
             if (CL_SUCCESS != ret) {
                 goto done;
             }
@@ -3670,6 +3666,8 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
     // In allmatch-mode, ret will never be CL_VIRUS, so ret may be used exclusively for file type detection and for terminal errors.
     // When not in allmatch-mode, it's more important to return right away if ret is CL_VIRUS, so we don't care if file type matches were found.
     if (ret >= CL_TYPENO) {
+        size_t last_offset = 0;
+
         // Matched 1+ file type signatures. Handle them.
         found_type = (cli_file_t)ret;
 
@@ -3678,11 +3676,16 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
         fpt = ftoffset;
 
         while (fpt) {
-            if (fpt->offset > 0) {
+            if ((fpt->offset > 0) &&
+                // Only handle each offset once to prevent duplicate processing like if two signatures are found at the same offset.
+                ((size_t)fpt->offset > last_offset)) {
+
                 bool type_has_been_handled = true;
                 bool ancestor_was_embedded = false;
                 size_t i;
 
+                last_offset = (size_t)fpt->offset;
+
                 /*
                  * First, use "embedded type recognition" to identify a file's actual type.
                  * (a.k.a. not embedded files, but file type detection corrections)
@@ -3873,84 +3876,10 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
                     break;
                 }
 
-                /*
-                 * Only scan embedded files if we are not already in an embedded context.
-                 * That is, if this or a previous layer was identified with embedded file type recognition, then we do
-                 * not scan for embedded files again.
-                 *
-                 * This restriction will prevent detecting the same embedded content more than once when recursing with
-                 * embedded file type recognition deeper within the same buffer.
-                 *
-                 * This is necessary because we have no way of knowing the length of a file for many formats and cannot
-                 * prevent a search for embedded files from finding the same embedded content multiple times (like a LOT
-                 * of times).
-                 *
-                 * E.g. if the file is like this:
-                 *
-                 *          [ data ] [ embedded file ] [ data ] [ embedded file ]
-                 *
-                 * The first time we do it we'll find "two" embedded files, like this:
-                 *
-                 *  Emb. File #1:    [ embedded file ] [ data ] [ embedded file ]
-                 *  Emb. File #2:                               [ embedded file ]
-                 *
-                 * We must not scan Emb. File #1 again for embedded files, because it would double-extract Emb. File #2.
-                 *
-                 * There is a flaw in this logic, though. Suppose that we actually have:
-                 *
-                 *          [ data ] [ compressed file w. recognizable magic bytes ]
-                 *
-                 * A first pass of the above will again identify "two" embedded files:
-                 *
-                 *  Emb. File #1:    [ compressed archive w. recognizable magic bytes ]
-                 *  Emb. File #2:                               [ magic bytes         ] <- Compressed data/Not real file
-                 *
-                 * In this case, the magic bytes of a contained, compressed file is somehow still identifiable despite
-                 * compression. The result is the Emb. File #2 will fail to be parsed and when we decompress Emb. File
-                 * #1, then we maybe get something like this:
-                 *
-                 *  Decompressed:    [ data                   ] [ embedded file ]
-                 *
-                 * So if this happened... then we WOULD want to scan the decompressed file for embedded files.
-                 * The problem is, we have no way of knowing how long embedded files are.
-                 * We don't know if we have:
-                 *
-                 * A.       [ data ] [ embedded file ] [ data ] [ embedded file ]
-                 *  or
-                 * B.       [ data ] [ embedded compressed archive w. recognizable magic bytes ]
-                 *  or
-                 * C.       [ data ] [ embedded uncompressed archive w. multiple file entries [ file 1 ] [ file 2 ] [ file 2 ] ]
-                 *
-                 * Some ideas for a more accurate solution:
-                 *
-                 * 1. Record the offset and size of each file extracted by the parsers.
-                 *    Then, when we do embedded file type recognition, we can check if the offset and size of the
-                 *    embedded file matches the offset and size of a file that was extracted by a parser.
-                 *    This falls apart a little bit for multiple layers of archives unless we also compare offsets within
-                 *    each layer. We could do that, but it would be a lot of work. And we'd probably want to take into
-                 *    consideration if files were decompressed or decrypted. ... I don't know a clean solution.
-                 *
-                 * 2. Have all parsers to run before embedded file type recognition and they each determine the length
-                 *    of the file they parsed, so we can differentiate between embedded files and appended files.
-                 *    For appended files, we would know they weren't extracted by a parser module and the parser for
-                 *    each of those would report the length of the file it parsed so we can use that to mitigate
-                 *    overlapping embedded file type recognition.
-                 *    But I highly doubt all file types can be parsed to determine the correct length of the file.
-                 */
-                for (i = ctx->recursion_level; i > 0; i--) {
-                    if (ctx->recursion_stack[i].attributes & LAYER_ATTRIBUTES_EMBEDDED) {
-                        // Found an ancestor that was embedded.
-                        // Do not scan embedded files again.
-                        ancestor_was_embedded = true;
-                        break;
-                    }
-                }
-
                 /*
                  * Next, check for actual embedded files.
                  */
-                if ((false == ancestor_was_embedded) &&
+                if (false == type_has_been_handled) {
-                    (false == type_has_been_handled)) {
                     cli_dbgmsg("%s signature found at %u\n", cli_ftname(fpt->type), (unsigned int)fpt->offset);
 
                     type_has_been_handled = true;
@@ -4003,6 +3932,9 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
                                     break;
                                 }
 
+                                // Increment last_offset to ignore any file type matches that occured within this legitimate archive.
+                                last_offset += zip_size - 1; // Note: size is definitely > 0 because header_check succeeded.
+
                                 nret = cli_magic_scan_nested_fmap_type(
                                     ctx->fmap,
                                     fpt->offset,
@@ -4025,6 +3957,9 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
                                     break;
                                 }
 
+                                // Increment last_offset to ignore any file type matches that occured within this legitimate archive.
+                                last_offset += cab_size - 1; // Note: size is definitely > 0 because header_check succeeded.
+
                                 nret = cli_magic_scan_nested_fmap_type(
                                     ctx->fmap,
                                     fpt->offset,
@@ -4048,6 +3983,9 @@ static cl_error_t scanraw(cli_ctx *ctx, cli_file_t type, uint8_t typercg, cli_fi
                                     break;
                                 }
 
+                                // Increment last_offset to ignore any file type matches that occured within this legitimate archive.
+                                last_offset += arj_size - 1; // Note: size is definitely > 0 because header_check succeeded.
+
                                 nret = cli_magic_scan_nested_fmap_type(
                                     ctx->fmap,
                                     fpt->offset,

libclamav/xlm_extract.c

@@ -4950,7 +4950,7 @@ cl_error_t cli_extract_xlm_macros_and_images(const char *dir, cli_ctx *ctx, char
     }
 
     if (CL_VIRUS == cli_scan_desc(out_fd, ctx, CL_TYPE_SCRIPT, false, NULL, AC_SCAN_VIR,
-                                  NULL, "xlm-macro", tempfile, LAYER_ATTRIBUTES_NORMALIZED)) {
+                                  NULL, "xlm-macro", tempfile, LAYER_ATTRIBUTES_NONE)) {
         status = CL_VIRUS;
         goto done;
     }

unit_tests/valgrind.supp

@@ -376,3 +376,12 @@
    fun:start_thread
    fun:clone
 }
+{
+   <image-fuzzy-hash-png-decoder>
+   Memcheck:Cond
+   ..
+   fun:read_header_info<std::io::cursor::Cursor<&[u8]>>
+   ...
+   fun:fuzzy_hash_calculate_image
+   ...
+}