[Fix] archives: bound metadata resource usage

diff --git a/rules/archives.lua b/rules/archives.lua
index c582b93bd224c58f9fb3415dea234d68456b8071..f33361094b78ea534d267ddbbdc6d02c53f2abbc 100644 (file)
--- a/rules/archives.lua
+++ b/rules/archives.lua
@@ -44,8 +44,8 @@ local id = rspamd_config:register_symbol {
           end
         end
 
-        local files = arc:get_files_full()
-        local max_check = math.min(#files, 10)
+        local files = arc:get_files_full(10)
+        local max_check = #files
 
         for i = 1, max_check do
           local info = files[i]

diff --git a/src/libmime/archives.c b/src/libmime/archives.c
index e9ffa3cac19c32676969918d4af79818c85be73f..9949a5552951763067f3e796f49444d952657875 100644 (file)
--- a/src/libmime/archives.c
+++ b/src/libmime/archives.c
@@ -40,6 +40,8 @@
 
 INIT_LOG_MODULE(archive)
 
+static const unsigned int max_archive_files = 100000;
+
 static GQuark
 rspamd_archives_err_quark(void)
 {
@@ -51,6 +53,16 @@ rspamd_archives_err_quark(void)
        return q;
 }
 
+static void
+rspamd_archive_file_free(struct rspamd_archive_file *f)
+{
+       if (f->fname) {
+               g_string_free(f->fname, TRUE);
+       }
+
+       g_free(f);
+}
+
 static void
 rspamd_archive_dtor(gpointer p)
 {
@@ -60,15 +72,58 @@ rspamd_archive_dtor(gpointer p)
 
        for (i = 0; i < arch->files->len; i++) {
                f = g_ptr_array_index(arch->files, i);
+               rspamd_archive_file_free(f);
+       }
 
-               if (f->fname) {
-                       g_string_free(f->fname, TRUE);
+       g_ptr_array_free(arch->files, TRUE);
+}
+
+static gboolean
+rspamd_archive_file_limit_check(struct rspamd_task *task,
+                                                               struct rspamd_archive *arch)
+{
+       if (MESSAGE_FIELD(task, narchive_files) >= max_archive_files) {
+               arch->flags |= RSPAMD_ARCHIVE_FILES_TRUNCATED;
+
+               if (!MESSAGE_FIELD(task, archive_files_limit_reached)) {
+                       msg_warn_task("archive file metadata limit of %ud is reached; "
+                                                 "ignoring the remaining entries",
+                                                 max_archive_files);
+                       MESSAGE_FIELD(task, archive_files_limit_reached) = TRUE;
                }
 
-               g_free(f);
+               return FALSE;
        }
 
-       g_ptr_array_free(arch->files, TRUE);
+       return TRUE;
+}
+
+static gboolean
+rspamd_archive_file_add(struct rspamd_task *task,
+                                               struct rspamd_archive *arch,
+                                               struct rspamd_archive_file *f)
+{
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               rspamd_archive_file_free(f);
+
+               return FALSE;
+       }
+
+       MESSAGE_FIELD(task, narchive_files) += 1;
+       g_ptr_array_add(arch->files, f);
+
+       return TRUE;
+}
+
+static void
+rspamd_archive_files_reset(struct rspamd_task *task,
+                                                  struct rspamd_archive *arch)
+{
+       g_assert(MESSAGE_FIELD(task, narchive_files) >= arch->files->len);
+       MESSAGE_FIELD(task, narchive_files) -= arch->files->len;
+       rspamd_archive_dtor(arch);
+       arch->files = g_ptr_array_new();
+       arch->flags &= ~RSPAMD_ARCHIVE_FILES_TRUNCATED;
 }
 
 
@@ -278,6 +333,9 @@ rspamd_archive_process_zip(struct rspamd_task *task,
        }
        rspamd_mempool_add_destructor(task->task_pool, rspamd_archive_dtor,
                                                                  arch);
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               goto set;
+       }
 
        while (cd < start + cd_offset + cd_size) {
                uint16_t flags;
@@ -324,7 +382,9 @@ rspamd_archive_process_zip(struct rspamd_task *task,
                                arch->flags |= RSPAMD_ARCHIVE_HAS_OBFUSCATED_FILES;
                        }
 
-                       g_ptr_array_add(arch->files, f);
+                       if (!rspamd_archive_file_add(task, arch, f)) {
+                               break;
+                       }
                        msg_debug_archive("found file in zip archive: %v", f->fname);
                }
                else {
@@ -359,6 +419,7 @@ rspamd_archive_process_zip(struct rspamd_task *task,
                cd += fname_len + comment_len + extra_len + cd_basic_len;
        }
 
+set:
        part->part_type = RSPAMD_MIME_PART_ARCHIVE;
        part->specific.arch = arch;
 
@@ -480,6 +541,9 @@ rspamd_archive_process_rar_v4(struct rspamd_task *task, const unsigned char *sta
        }
        rspamd_mempool_add_destructor(task->task_pool, rspamd_archive_dtor,
                                                                  arch);
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               goto end;
+       }
 
        while (p < end) {
                /* Crc16 */
@@ -597,7 +661,9 @@ rspamd_archive_process_rar_v4(struct rspamd_task *task, const unsigned char *sta
                                if (f->flags & RSPAMD_ARCHIVE_FILE_OBFUSCATED) {
                                        arch->flags |= RSPAMD_ARCHIVE_HAS_OBFUSCATED_FILES;
                                }
-                               g_ptr_array_add(arch->files, f);
+                               if (!rspamd_archive_file_add(task, arch, f)) {
+                                       goto end;
+                               }
                        }
                        else {
                                g_free(f);
@@ -662,6 +728,9 @@ rspamd_archive_process_rar(struct rspamd_task *task,
        }
        rspamd_mempool_add_destructor(task->task_pool, rspamd_archive_dtor,
                                                                  arch);
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               goto end;
+       }
 
        /* Now we can have either encryption header or archive header */
        /* Crc 32 */
@@ -795,7 +864,10 @@ rspamd_archive_process_rar(struct rspamd_task *task,
 
                                if (f->fname) {
                                        msg_debug_archive("added rarv5 file: %v", f->fname);
-                                       g_ptr_array_add(arch->files, f);
+                                       if (!rspamd_archive_file_add(task, arch, f)) {
+                                               f = NULL;
+                                               goto end;
+                                       }
                                        if (f->flags & RSPAMD_ARCHIVE_FILE_OBFUSCATED) {
                                                arch->flags |= RSPAMD_ARCHIVE_HAS_OBFUSCATED_FILES;
                                        }
@@ -1690,7 +1762,10 @@ rspamd_7zip_read_files_info(struct rspamd_task *task,
                                        if (res != NULL) {
                                                fentry = g_malloc0(sizeof(*fentry));
                                                fentry->fname = res;
-                                               g_ptr_array_add(arch->files, fentry);
+                                               if (!rspamd_archive_file_add(task, arch, fentry)) {
+                                                       p = NULL;
+                                                       goto end;
+                                               }
                                                msg_debug_archive("7zip: found file %v", res);
                                        }
                                        else {
@@ -1756,8 +1831,7 @@ rspamd_7zip_read_next_section(struct rspamd_task *task,
                        }
 
                        /* Clean the existing files if any */
-                       rspamd_archive_dtor(arch);
-                       arch->files = g_ptr_array_new();
+                       rspamd_archive_files_reset(task, arch);
 
                        struct archive_entry *ae;
 
@@ -1767,7 +1841,9 @@ rspamd_7zip_read_next_section(struct rspamd_task *task,
                                        msg_debug_archive("7zip: found file %s", name);
                                        struct rspamd_archive_file *f = g_malloc0(sizeof(*f));
                                        f->fname = g_string_new(name);
-                                       g_ptr_array_add(arch->files, f);
+                                       if (!rspamd_archive_file_add(task, arch, f)) {
+                                               break;
+                                       }
                                }
                                archive_read_data_skip(a);
                        }
@@ -1831,6 +1907,9 @@ rspamd_archive_process_7zip(struct rspamd_task *task,
        arch->type = RSPAMD_ARCHIVE_7ZIP;
        rspamd_mempool_add_destructor(task->task_pool, rspamd_archive_dtor,
                                                                  arch);
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               goto end;
+       }
 
        /* Magic (6 bytes) + version (2 bytes) + crc32 (4 bytes) */
        p += sizeof(uint64_t) + sizeof(uint32_t);
@@ -1858,6 +1937,7 @@ rspamd_archive_process_7zip(struct rspamd_task *task,
 
        while ((p = rspamd_7zip_read_next_section(task, p, end, arch, part)) != NULL);
 
+end:
        part->part_type = RSPAMD_MIME_PART_ARCHIVE;
        part->specific.arch = arch;
        if (part->cd != NULL) {
@@ -1893,6 +1973,9 @@ rspamd_archive_process_gzip(struct rspamd_task *task,
        }
        rspamd_mempool_add_destructor(task->task_pool, rspamd_archive_dtor,
                                                                  arch);
+       if (!rspamd_archive_file_limit_check(task, arch)) {
+               goto set;
+       }
 
        flags = p[3];
 
@@ -1940,7 +2023,9 @@ rspamd_archive_process_gzip(struct rspamd_task *task,
                                                                                                fname_start, p - fname_start);
 
                                        if (f->fname) {
-                                               g_ptr_array_add(arch->files, f);
+                                               if (!rspamd_archive_file_add(task, arch, f)) {
+                                                       goto set;
+                                               }
 
                                                if (f->flags & RSPAMD_ARCHIVE_FILE_OBFUSCATED) {
                                                        arch->flags |= RSPAMD_ARCHIVE_HAS_OBFUSCATED_FILES;
@@ -1987,7 +2072,9 @@ rspamd_archive_process_gzip(struct rspamd_task *task,
                                msg_debug_archive("fallback to gzip filename based on cd: %v",
                                                                  f->fname);
 
-                               g_ptr_array_add(arch->files, f);
+                               if (!rspamd_archive_file_add(task, arch, f)) {
+                                       goto set;
+                               }
 
                                goto set;
                        }
@@ -2012,7 +2099,9 @@ rspamd_archive_process_gzip(struct rspamd_task *task,
                                msg_debug_archive("fallback to gzip filename based on cd: %v",
                                                                  f->fname);
 
-                               g_ptr_array_add(arch->files, f);
+                               if (!rspamd_archive_file_add(task, arch, f)) {
+                                       goto set;
+                               }
 
                                goto set;
                        }

diff --git a/src/libmime/archives.h b/src/libmime/archives.h
index 6e05cd53fae7e7b421c551bb297dae728f51a242..76ceb540f6fea650e78b691f3bd372cef3ffe570 100644 (file)
--- a/src/libmime/archives.h
+++ b/src/libmime/archives.h
@@ -33,6 +33,7 @@ enum rspamd_archive_flags {
        RSPAMD_ARCHIVE_ENCRYPTED = (1u << 0u),
        RSPAMD_ARCHIVE_CANNOT_READ = (1u << 1u),
        RSPAMD_ARCHIVE_HAS_OBFUSCATED_FILES = (1u << 2u),
+       RSPAMD_ARCHIVE_FILES_TRUNCATED = (1u << 3u),
 };
 
 enum rspamd_archive_file_flags {

diff --git a/src/libmime/message.h b/src/libmime/message.h
index 0c8479cb629d335c31c011d62ee356614a8eae7b..b69c809f23d220c64dc7141a8c5c3d2cbf06b130 100644 (file)
--- a/src/libmime/message.h
+++ b/src/libmime/message.h
@@ -198,6 +198,8 @@ struct rspamd_message {
        struct rspamd_mime_header *headers_order;      /**< order of raw headers                                                        */
        unsigned int nheaders;                         /**< total parsed headers across all MIME parts */
        gboolean headers_limit_reached;
+       unsigned int narchive_files; /**< total archive file metadata entries */
+       gboolean archive_files_limit_reached;
        struct rspamd_task *task;
        GPtrArray *rcpt_mime;
        GPtrArray *from_mime;

diff --git a/src/lua/lua_task.c b/src/lua/lua_task.c
index eeb0e3c37b4def0f750932d6ad503ca2a3ac814d..fd375712da849957161f7cf5c1284f2b580f6bba 100644 (file)
--- a/src/lua/lua_task.c
+++ b/src/lua/lua_task.c
@@ -757,6 +757,7 @@ LUA_FUNCTION_DEF(task, get_images);
  * * `get_files` - return list of strings with filenames inside archive
  * * `get_files_full` - return list of tables with all information about files
  * * `is_encrypted` - return true if an archive is encrypted
+ * * `is_truncated` - return true if the archive file list hit the task limit
  * * `get_type` - return string representation of image's type (e.g. 'zip')
  * * `get_filename` - return string with archive's file name
  * * `get_size` - return size in bytes
@@ -1566,6 +1567,7 @@ LUA_FUNCTION_DEF(archive, get_files_full);
 LUA_FUNCTION_DEF(archive, is_encrypted);
 LUA_FUNCTION_DEF(archive, is_obfuscated);
 LUA_FUNCTION_DEF(archive, is_unreadable);
+LUA_FUNCTION_DEF(archive, is_truncated);
 LUA_FUNCTION_DEF(archive, get_filename);
 LUA_FUNCTION_DEF(archive, get_size);
 
@@ -1576,6 +1578,7 @@ static const struct luaL_reg archivelib_m[] = {
        LUA_INTERFACE_DEF(archive, is_encrypted),
        LUA_INTERFACE_DEF(archive, is_obfuscated),
        LUA_INTERFACE_DEF(archive, is_unreadable),
+       LUA_INTERFACE_DEF(archive, is_truncated),
        LUA_INTERFACE_DEF(archive, get_filename),
        LUA_INTERFACE_DEF(archive, get_size),
        {"__tostring", rspamd_lua_class_tostring},
@@ -8397,6 +8400,22 @@ lua_archive_is_unreadable(lua_State *L)
        return 1;
 }
 
+static int
+lua_archive_is_truncated(lua_State *L)
+{
+       LUA_TRACE_POINT;
+       struct rspamd_archive *arch = lua_check_archive(L);
+
+       if (arch != NULL) {
+               lua_pushboolean(L, (arch->flags & RSPAMD_ARCHIVE_FILES_TRUNCATED) ? true : false);
+       }
+       else {
+               return luaL_error(L, "invalid arguments");
+       }
+
+       return 1;
+}
+
 static int
 lua_archive_get_size(lua_State *L)
 {

diff --git a/test/lua/unit/task.lua b/test/lua/unit/task.lua
index 5b19d21bb2201f74e100cd057c196a2f9d506e77..98687ea7afea5011a395aaefa5c2251843e0fa36 100644 (file)
--- a/test/lua/unit/task.lua
+++ b/test/lua/unit/task.lua
@@ -25,6 +25,18 @@ From: <>
 To: <nobody@example.com>
 Subject: test
 ]]
+  local function make_many_file_zip(nfiles)
+    local rspamd_util = require("rspamd_util")
+    local local_header = "PK\3\4" .. string.rep("\0", 26)
+    local cd_record = "PK\1\2" .. string.rep("\0", 24) ..
+        "\1\0" .. string.rep("\0", 16) .. "x"
+    local cd = string.rep(cd_record, nfiles)
+    local eocd = "PK\5\6" .. string.rep("\0", 8) ..
+        rspamd_util.pack("<I4", #cd) ..
+        rspamd_util.pack("<I4", #local_header) .. "\0\0"
+
+    return local_header .. cd .. eocd
+  end
   local mpart = [[
 Content-Type: multipart/mixed; boundary=XXX
 ]]
@@ -252,6 +264,72 @@ Thank you,
     task:destroy()
   end)
 
+  test("Archive file metadata count is bounded", function()
+    local msg = table.concat {
+      hdrs,
+      "Content-Type: application/zip\n",
+      "Content-Disposition: attachment; filename=many.zip\n",
+      "Content-Transfer-Encoding: binary\n",
+      "\n",
+      make_many_file_zip(100001),
+    }
+    local res, task = rspamd_task.load_from_string(msg, rspamd_config)
+    assert_true(res, "failed to load message")
+    task:process_message()
+
+    local archives = task:get_archives()
+    assert_equal(1, #archives, "archive was not detected")
+    local files = archives[1]:get_files()
+    assert_equal(100000, #files,
+      string.format("unexpected archive file count: %d", #files))
+    assert_true(archives[1]:is_truncated(),
+      "archive metadata truncation was not exposed")
+    task:destroy()
+  end)
+
+  test("Archive file metadata count is bounded across parts", function()
+    local zip = make_many_file_zip(1000)
+    local body = {}
+
+    for i = 1, 101 do
+      body[#body + 1] = table.concat {
+        "--MANY-ARCHIVES\n",
+        "Content-Type: application/zip\n",
+        string.format(
+          "Content-Disposition: attachment; filename=archive-%d.zip\n", i),
+        "Content-Transfer-Encoding: binary\n",
+        "\n",
+        zip,
+        "\n",
+      }
+    end
+    body[#body + 1] = "--MANY-ARCHIVES--\n"
+
+    local msg = table.concat {
+      hdrs,
+      "Content-Type: multipart/mixed; boundary=MANY-ARCHIVES\n",
+      "\n",
+      table.concat(body),
+    }
+    local res, task = rspamd_task.load_from_string(msg, rspamd_config)
+    assert_true(res, "failed to load message")
+    task:process_message()
+
+    local archives = task:get_archives()
+    assert_equal(101, #archives, "archives were not detected")
+    local nfiles = 0
+    for _, archive in ipairs(archives) do
+      nfiles = nfiles + #archive:get_files()
+    end
+    assert_equal(100000, nfiles,
+      string.format("unexpected archive file count across parts: %d", nfiles))
+    assert_false(archives[100]:is_truncated(),
+      "archive within the task budget was marked truncated")
+    assert_true(archives[101]:is_truncated(),
+      "archive exceeding the task budget was not marked truncated")
+    task:destroy()
+  end)
+
   test("Part URLs are not deduplicated across MIME parts", function()
     local msg = table.concat {
       hdrs,