From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Sat, 4 Jul 2026 16:04:04 +0000 (+0000)
Subject: [PATCH] imap-login: Fix excessive memory growth with pre-login ID command
X-Git-Tag: archive/raspbian/1%2.4.1+dfsg1-6+rpi1+deb13u7^2~105
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=a6df4fe6b8663dd4db25715c7d5c66eddfe70ebb;p=dovecot.git

[PATCH] imap-login: Fix excessive memory growth with pre-login ID command

The pre-login ID parser reset the per-argument line and list limits after
every argument, so nothing bounded the number of key/value pairs. Each
external key grew log_reply and added a distinct event field (a linear
scan, making it O(N^2)), allowing a pre-auth CPU/memory amplification.

Account at most 30 pairs (per RFC 2971) into the logging and client_id
bookkeeping. Internal x-* parameter handlers still run for pairs beyond
the limit, since a trusted proxy may legitimately forward more fields.

Gbp-Pq: Name 0001-imap-login-Fix-excessive-memory-growth-with-pre-logi.patch
---

diff --git a/src/imap-login/imap-login-client.h b/src/imap-login/imap-login-client.h
index 34ea226..be4f16f 100644
--- a/src/imap-login/imap-login-client.h
+++ b/src/imap-login/imap-login-client.h
@@ -53,6 +53,10 @@ struct imap_client_cmd_id {
 	struct event *params_event;
 	struct imap_id_params *params;
 	string_t *log_reply;
+	/* Number of external (non internal x-*) key/value pairs accounted so
+	   far. Used to bound the pre-login accounting (logging, event fields,
+	   client_id) done per external pair. */
+	unsigned int processed_pairs_count;
 };
 
 struct imap_client {
diff --git a/src/imap-login/imap-login-cmd-id.c b/src/imap-login/imap-login-cmd-id.c
index be12744..d86817d 100644
--- a/src/imap-login/imap-login-cmd-id.c
+++ b/src/imap-login/imap-login-cmd-id.c
@@ -9,6 +9,12 @@
 #include "imap-login-settings.h"
 #include "imap-login-client.h"
 
+/* RFC 2971 says a client SHOULD NOT send more than 30 field-value pairs.
+   Allow that many to be accounted for (logged / kept in client_id); pairs
+   beyond this are still parsed and internal x-* handlers still run, but they
+   are not added to the logging/client_id accounting that grows per pair. */
+#define IMAP_ID_MAX_ACCOUNTED_PAIRS 30
+
 struct imap_id_params_forward {
 	const char *key;
 	const char *value;
@@ -183,6 +189,16 @@ static bool cmd_id_handle_keyvalue(struct imap_client *client,
 		imap_id_param_handler_find(key);
 	bool is_login_id_param = handler != NULL;
 
+	/* Only external pairs are logged / kept in client_id, and only those
+	   grow per-pair with count, so only they need bounding to avoid a
+	   pre-login CPU/memory DoS. Internal x-* handlers must keep running
+	   regardless (e.g. a proxy may legitimately forward more than 30
+	   fields) and are not counted against the limit. */
+	bool account_pair = !is_login_id_param &&
+		client->cmd_id->processed_pairs_count < IMAP_ID_MAX_ACCOUNTED_PAIRS;
+	if (account_pair)
+		client->cmd_id->processed_pairs_count++;
+
 	if (!is_login_id_param) {
 		/* not an internal key */
 	} else if (client->id_logged) {
@@ -197,7 +213,7 @@ static bool cmd_id_handle_keyvalue(struct imap_client *client,
 		}
 	}
 
-	if (client->set->imap_id_retain && !is_login_id_param &&
+	if (account_pair && client->set->imap_id_retain &&
 	    (client->common.client_id == NULL ||
 	     str_len(client->common.client_id) + kvlen < LOGIN_MAX_CLIENT_ID_LEN)) {
 		if (client->common.client_id == NULL) {
@@ -213,7 +229,7 @@ static bool cmd_id_handle_keyvalue(struct imap_client *client,
 			imap_append_quoted(client->common.client_id, value);
 	}
 
-	if (!is_login_id_param)
+	if (account_pair)
 		imap_id_add_log_entry(log_entry, key, value);
 	return TRUE;
 }