[Fix] ragel: bound nested comment depth in header parsers

The Content-Disposition and SMTP date grammars parse RFC 5322 nested
comments via `fcall`, which pushes one entry onto a heap allocated
state stack per nesting level. The stack had no depth limit and grew
through realloc, so a single header could allocate memory proportional
to its own length: peak RSS scaled linearly at roughly 12 bytes per
input byte, with unbalanced opening parens as the worst case since
every byte adds a level. Bounded only by max_message (50Mb by default),
a crafted header could push a worker to around 530Mb of transient
allocation, and realloc failure hit a g_assert and aborted the process
instead of failing the parse.

Cap the nesting depth at 8 (real headers never nest more than a couple
of levels) and use fbreak to drop into the error state once it is
exceeded, so the parser stops after a fixed number of bytes rather than
consuming the whole header. Replace the g_assert with a realloc failure
path that keeps the old allocation so it can still be freed.

Note that src/ragel/content_type.rl carries the same prepush but is not
built: it is not an input to any ragel_target, and the live content type
parser is the hand written state machine in content_type.c.

diff --git a/src/libmime/smtp_parsers.h b/src/libmime/smtp_parsers.h
index 79d15903a0f4ac46611f7494093cd83ef0f817b6..7b65b54d08c34bdd9f7dc99afdafa84752a32549 100644 (file)
--- a/src/libmime/smtp_parsers.h
+++ b/src/libmime/smtp_parsers.h
@@ -23,6 +23,16 @@
 #include "message.h"
 
 
+/*
+ * Maximum nesting depth of RFC 5322 comments, e.g. `(a (b (c)))`.
+ * The Ragel grammars implement nested comments via `fcall`, which pushes one
+ * entry onto a heap allocated state stack per level, so an unbounded limit lets
+ * a single header allocate memory proportional to its own length. Real world
+ * headers never nest more than a couple of levels; anything deeper is treated
+ * as a parse error.
+ */
+#define RSPAMD_RAGEL_MAX_COMMENT_NESTING 8
+
 #ifdef __cplusplus
 extern "C" {
 #endif

diff --git a/src/ragel/content_disposition.rl b/src/ragel/content_disposition.rl
index 93d3c9d3d348abc297cc198b3e866427bfffa93d..1878988bd90696e504d640df5eb03f0ca5e58f36 100644 (file)
--- a/src/ragel/content_disposition.rl
+++ b/src/ragel/content_disposition.rl
@@ -26,10 +26,22 @@
   content_disposition = disposition_type (";" disposition_parm)*;
 
   prepush {
+    if (top >= RSPAMD_RAGEL_MAX_COMMENT_NESTING) {
+      /* Too deeply nested comments, bail out instead of growing the stack */
+      fbreak;
+    }
+
     if (top >= st_storage.size) {
-      st_storage.size = (top + 1) * 2;
-      st_storage.data = realloc (st_storage.data, st_storage.size * sizeof (int));
-      g_assert (st_storage.data != NULL);
+      gsize _new_size = (top + 1) * 2;
+      int *_new_data = realloc (st_storage.data, _new_size * sizeof (int));
+
+      if (_new_data == NULL) {
+        /* Keep the old allocation so that it can still be freed */
+        fbreak;
+      }
+
+      st_storage.size = _new_size;
+      st_storage.data = _new_data;
       stack = st_storage.data;
     }
   }

diff --git a/src/ragel/smtp_date.rl b/src/ragel/smtp_date.rl
index 125ae8a408e29586e93d18864c18aae3e543660e..33e1887ab5c27d4ab09b9954283e99f2d22b8a2a 100644 (file)
--- a/src/ragel/smtp_date.rl
+++ b/src/ragel/smtp_date.rl
@@ -192,10 +192,22 @@
     tz = -700;
   }
   prepush {
+    if (top >= RSPAMD_RAGEL_MAX_COMMENT_NESTING) {
+      /* Too deeply nested comments, bail out instead of growing the stack */
+      fbreak;
+    }
+
     if (top >= st_storage.size) {
-      st_storage.size = (top + 1) * 2;
-      st_storage.data = realloc (st_storage.data, st_storage.size * sizeof (int));
-      g_assert (st_storage.data != NULL);
+      gsize _new_size = (top + 1) * 2;
+      int *_new_data = realloc (st_storage.data, _new_size * sizeof (int));
+
+      if (_new_data == NULL) {
+        /* Keep the old allocation so that it can still be freed */
+        fbreak;
+      }
+
+      st_storage.size = _new_size;
+      st_storage.data = _new_data;
       stack = st_storage.data;
     }
   }