Fwd: Huge header detection

Bron Gondwana brong at fastmail.fm
Mon Feb 9 06:38:13 EST 2009


On Mon, Feb 09, 2009 at 10:37:16PM +1100, Bron Gondwana wrote:
> On Mon, Feb 09, 2009 at 10:02:37PM +1100, Bron Gondwana wrote:
> > Ditto.  Gosh.  That makes 3 tunables.  The gods of tunable
> > non-proliferation will want my head for doing this:
> > 
> > maxcacheheaders_warn = 500
> > maxcacheheaders_skip = 1000 (same as the current patch)
> > maxcacheheaders_reject = 2000
> > 
> > Sound like reasonable defaults?  I'm tempted to make the _reject be '0'
> > (don't reject) by default.
> 
> Yeah, something like this completely untested code!
> 
> Bron ( there doesn't seem to be an easy way to signal "abort this
>        thing" back up the chain while parsing the message in the
>        general case, but at least I've caught the top level message )

Bah, it's too late to think!
-------------- next part --------------
diff --git a/imap/message.c b/imap/message.c
index 2d9afd2..efd7c32 100644
--- a/imap/message.c
+++ b/imap/message.c
@@ -412,8 +412,9 @@ int message_parse_binary_file(FILE *infile, struct body **body)
     }
 
     if (!*body) *body = (struct body *) xmalloc(sizeof(struct body));
-    message_parse_body(&msg, MAILBOX_FORMAT_NORMAL, *body,
-		       DEFAULT_CONTENT_TYPE, (struct boundary *)0);
+    if (message_parse_body(&msg, MAILBOX_FORMAT_NORMAL, *body,
+			   DEFAULT_CONTENT_TYPE, (struct boundary *)0))
+	return IMAP_IOERROR;
 
     lseek(fd, 0L, SEEK_SET);
     n = retry_write(fd, msg.base, msg.len);
@@ -442,8 +443,9 @@ int message_parse_mapped(const char *msg_base, unsigned long msg_len,
     msg.offset = 0;
     msg.encode = 0;
 
-    message_parse_body(&msg, MAILBOX_FORMAT_NORMAL, body,
-		       DEFAULT_CONTENT_TYPE, (struct boundary *)0);
+    if (message_parse_body(&msg, MAILBOX_FORMAT_NORMAL, body,
+		       DEFAULT_CONTENT_TYPE, (struct boundary *)0))
+	return IMAP_IOERROR;
 
     message_guid_generate(&body->guid, msg_base, msg_len);
 
@@ -601,6 +603,8 @@ struct boundary *boundaries;
     sawboundary = message_parse_headers(msg, format, body, defaultContentType,
 					boundaries);
 
+    if (sawboundary < 0) return IMAP_IOERROR; /* too many headers! */
+
     /* Recurse according to type */
     if (strcmp(body->type, "MULTIPART") == 0) {
 	if (!sawboundary) {
@@ -660,6 +664,10 @@ struct boundary *boundaries;
     int left, len;
     char *next;
     int sawboundary = 0;
+    int max_warn = config_getint(IMAPOPT_MAXHEADERLINES_WARN);
+    int max_skip = config_getint(IMAPOPT_MAXHEADERLINES_SKIP);
+    int max_reject = config_getint(IMAPOPT_MAXHEADERLINES_REJECT);
+    int cur_level = 0;
 
     body->header_offset = msg->offset;
 
@@ -716,6 +724,27 @@ struct boundary *boundaries;
 	if (*next == '\n') {
 	    body->header_lines++;
 
+	    /* if we're skipping, skip now */
+	    if (cur_level >= 2) continue;
+
+	    /* check if we've hit a limit and flag it */
+	    if (cur_level < 3 && max_reject && body->header_lines > max_reject) {
+		syslog(LOG_ERR, "ERROR: message has %d header lines, rejecting",
+		       max_reject);
+		return -1;
+	    }
+	    if (cur_level < 2 && max_skip && body->header_lines > max_skip) {
+		syslog(LOG_ERR, "ERROR: message has %d header lines, not caching any more",
+		       max_skip);
+		cur_level = 2;
+		continue;
+	    }
+	    if (cur_level < 1 && max_warn && body->header_lines > max_warn) {
+		syslog(LOG_NOTICE, "WARNING: message has over %d header lines",
+		       max_warn);
+		cur_level = 1;
+	    }
+
 	    /* Check for headers in generic cache */
 	    if (body->cacheheaders.start &&
 		(next[1] != ' ') && (next[1] != '\t') &&
diff --git a/lib/imapoptions b/lib/imapoptions
index b17d831..7b4ec19 100644
--- a/lib/imapoptions
+++ b/lib/imapoptions
@@ -566,6 +566,22 @@ are listed with ``<none>''.
 /* Notifyd(8) method to use for "MAIL" notifications.  If not set, "MAIL"
    notifications are disabled. */
 
+{ "maxheaderlines_warn", 500, INT }
+/* Number of lines in a message header before a warning is logged
+   to syslog.  Default 500.  If set to zero, it is unlimited. 
+   If set higher than maxheaderlines_skip, then you'll only get the
+   single warning when headers start getting skipped. */
+
+{ "maxheaderlines_skip", 1000, INT }
+/* Maximum number of lines of header that will be processed into cache
+   records.  Default 1000.  If set to zero, it is unlimited.
+   If set higher than maxheaderlines_reject, then you will only
+   get the warning as the message is rejected. */
+
+{ "maxheaderlines_reject", 0, INT }
+/* Number of lines in a message header before the message is rejected
+   by cyrus.  The default is zero - unlimited. */
+
 { "maxmessagesize", 0, INT }
 /* Maximum incoming LMTP message size.  If non-zero, lmtpd will reject
    messages larger than \fImaxmessagesize\fR bytes.  If set to 0, this


More information about the Info-cyrus mailing list