Merge pull request #1264 from ragusaa/CLAM-2602-ValidateMaxRecursion

Added checks for MaxRecursion
Cisco-Talos · Sep 9, 2024 · 51e2390 · 51e2390
2 parents c17a0c7 + e7cb0ff
commit 51e2390
Show file tree

Hide file tree

Showing 11 changed files with 32 additions and 22 deletions.
diff --git a/clamd/server-th.c b/clamd/server-th.c
@@ -50,6 +50,7 @@
 #include "clamav.h"
 #include "others.h"
 #include "readdb.h"
+#include "default.h"
 
 // common
 #include "output.h"
@@ -981,6 +982,12 @@ int recvloop(int *socketds, unsigned nsockets, struct cl_engine *engine, unsigne
 #endif
 
     if ((opt = optget(opts, "MaxRecursion"))->active) {
+        if ((0 == opt->numarg) || (opt->numarg > CLI_MAX_MAXRECLEVEL)) {
+            logg(LOGG_ERROR, "MaxRecursion set to %zu, but cannot be larger than %u, and cannot be 0.\n",
+                 (size_t)opt->numarg, CLI_MAX_MAXRECLEVEL);
+            cl_engine_free(engine);
+            return 1;
+        }
         if ((ret = cl_engine_set_num(engine, CL_ENGINE_MAX_RECURSION, opt->numarg))) {
             logg(LOGG_ERROR, "cl_engine_set_num(CL_ENGINE_MAX_RECURSION) failed: %s\n", cl_strerror(ret));
             cl_engine_free(engine);

diff --git a/clamscan/manager.c b/clamscan/manager.c
@@ -56,6 +56,7 @@
 #include "matcher-pcre.h"
 #include "str.h"
 #include "readdb.h"
+#include "default.h"
 
 // common
 #include "optparser.h"
@@ -1388,6 +1389,13 @@ int scanmanager(const struct optstruct *opts)
     }
 
     if ((opt = optget(opts, "max-recursion"))->active) {
+        uint32_t opt_value = opt->numarg;
+        if ((0 == opt_value) || (opt_value > CLI_MAX_MAXRECLEVEL)) {
+            logg(LOGG_ERROR, "max-recursion set to %u, but  cannot be larger than %u, and cannot be 0.\n",
+                 opt_value, CLI_MAX_MAXRECLEVEL);
+            ret = 2;
+            goto done;
+        }
         if ((ret = cl_engine_set_num(engine, CL_ENGINE_MAX_RECURSION, opt->numarg))) {
             logg(LOGG_ERROR, "cli_engine_set_num(CL_ENGINE_MAX_RECURSION) failed: %s\n", cl_strerror(ret));
             ret = 2;

diff --git a/docs/man/clamscan.1.in b/docs/man/clamscan.1.in
@@ -232,7 +232,7 @@ Extract and scan at most #n bytes from each archive. The size the archive plus t
 Extract at most #n files from each scanned file (when this is an archive, a document or another kind of container). This option protects your system against DoS attacks (default: 10000)
 .TP
 \fB\-\-max\-recursion=#n\fR
-Set archive recursion level limit. This option protects your system against DoS attacks (default: 17).
+Set archive recursion level limit. This option protects your system against DoS attacks (default: 17) (maximum: 100).
 .TP
 \fB\-\-max\-dir\-recursion=#n\fR
 Maximum depth directories are scanned at (default: 15).

diff --git a/etc/clamd.conf.sample b/etc/clamd.conf.sample
@@ -585,6 +585,7 @@ Example
 # deeply the process should be continued.
 # Note: setting this limit too high may result in severe damage to the system.
 # Default: 17
+# Maximum: 100
 #MaxRecursion 10
 
 # Number of files to be scanned within an archive, a document, or any other

diff --git a/libclamav/default.h b/libclamav/default.h
@@ -58,6 +58,8 @@
 #define CLI_DEFAULT_PCRE_RECMATCH_LIMIT 2000
 #define CLI_DEFAULT_PCRE_MAX_FILESIZE   (1024 * 1024 * 100)   // 100 MB
 
+/* Maximums */
+#define CLI_MAX_MAXRECLEVEL     100
 // clang-format on
 
 #endif
diff --git a/libclamav/readdb.h b/libclamav/readdb.h
@@ -81,9 +81,9 @@ struct cli_matcher;
         cli_strbcasestr(ext, ".ign") ||  \
         cli_strbcasestr(ext, ".ign2") || \
         cli_strbcasestr(ext, ".imp"))
-#define CLI_DBEXT_SIGNATURE(ext)         \
-    (                                    \
-        cli_strbcasestr(ext, ".cvd") ||  \
+#define CLI_DBEXT_SIGNATURE(ext)        \
+    (                                   \
+        cli_strbcasestr(ext, ".cvd") || \
         cli_strbcasestr(ext, ".cld"))
 #else
 #define CLI_DBEXT(ext)                   \
@@ -124,9 +124,9 @@ struct cli_matcher;
         cli_strbcasestr(ext, ".ign") ||  \
         cli_strbcasestr(ext, ".ign2") || \
         cli_strbcasestr(ext, ".imp"))
-#define CLI_DBEXT_SIGNATURE(ext)         \
-    (                                    \
-        cli_strbcasestr(ext, ".cvd") ||  \
+#define CLI_DBEXT_SIGNATURE(ext)        \
+    (                                   \
+        cli_strbcasestr(ext, ".cvd") || \
         cli_strbcasestr(ext, ".cld"))
 #endif
 

diff --git a/libclamav/scanners.c b/libclamav/scanners.c
@@ -4212,9 +4212,9 @@ static inline bool result_should_goto_done(cli_ctx *ctx, cl_error_t result_in, c
 
 cl_error_t cli_magic_scan(cli_ctx *ctx, cli_file_t type)
 {
-    cl_error_t ret = CL_CLEAN;
+    cl_error_t ret                = CL_CLEAN;
     cl_error_t cache_check_result = CL_VIRUS;
-    bool cache_enabled = true;
+    bool cache_enabled            = true;
     cl_error_t verdict_at_this_level;
     cli_file_t dettype              = 0;
     uint8_t typercg                 = 1;

diff --git a/libclamav/special.c b/libclamav/special.c
@@ -72,7 +72,7 @@ int cli_check_mydoom_log(cli_ctx *ctx)
     while (blocks) { /* This wasn't probably intended but that's what the current code does anyway */
         const uint32_t marker_ff = 0xffffffff;
 
-	if (!memcmp(&ptr[--blocks], &marker_ff, sizeof(uint32_t)))
+        if (!memcmp(&ptr[--blocks], &marker_ff, sizeof(uint32_t)))
             return CL_CLEAN;
     }
 

diff --git a/libclamav/udf.h b/libclamav/udf.h
@@ -66,7 +66,6 @@ typedef struct __attribute__((packed)) {
 
 } lb_addr;
 
-
 // Long allocation descriptor
 typedef struct __attribute__((packed)) {
     uint32_t length; // 4/14.14.1.1
@@ -211,10 +210,7 @@ static uint32_t getFileIdentifierDescriptorPaddingLength(const FileIdentifierDes
 
 static inline size_t getFileIdentifierDescriptorSize(const FileIdentifierDescriptor* fid)
 {
-    return FILE_IDENTIFIER_DESCRIPTOR_SIZE_KNOWN
-        + le16_to_host(fid->implementationLength)
-        + fid->fileIdentifierLength
-        + getFileIdentifierDescriptorPaddingLength(fid);
+    return FILE_IDENTIFIER_DESCRIPTOR_SIZE_KNOWN + le16_to_host(fid->implementationLength) + fid->fileIdentifierLength + getFileIdentifierDescriptorPaddingLength(fid);
 }
 
 typedef struct __attribute__((packed)) {
@@ -521,7 +517,7 @@ typedef struct __attribute__((packed)) {
 
 } FileSetDescriptor;
 
-typedef struct  __attribute__((packed)) {
+typedef struct __attribute__((packed)) {
     uint8_t structType;
     char standardIdentifier[5];
     uint8_t structVersion;

diff --git a/libclamav_rust/build.rs b/libclamav_rust/build.rs
@@ -58,12 +58,7 @@ const BINDGEN_FUNCTIONS: &[&str] = &[
 ];
 
 // Generate bindings for these types (structs, enums):
-const BINDGEN_TYPES: &[&str] = &[
-    "cli_matcher",
-    "cli_ac_data",
-    "cli_ac_result",
-    "onedump_t",
-];
+const BINDGEN_TYPES: &[&str] = &["cli_matcher", "cli_ac_data", "cli_ac_result", "onedump_t"];
 
 // Find the required functions and types in these headers:
 const BINDGEN_HEADERS: &[&str] = &[

diff --git a/win32/conf_examples/clamd.conf.sample b/win32/conf_examples/clamd.conf.sample
@@ -557,6 +557,7 @@ TCPAddr localhost
 # deeply the process should be continued.
 # Note: setting this limit too high may result in severe damage to the system.
 # Default: 17
+# Maximum: 100
 #MaxRecursion 10
 
 # Number of files to be scanned within an archive, a document, or any other