Re: [evolution-patches] [Fwd: Use of invalid HELO.]

[David Woodhouse <dwmw2 infradead org>]

On Tue, 2006-04-04 at 16:58 -0400, Jeffrey Stedfast wrote: 
> your patch will crash if name is NULL, also:

Yeah, I didn't think that was possible. It's easier to code it
defensively than to prove that though, so I've changed to check.

> static gboolean smtp_helo_is_valid(char *name)
> 
> should be:
> 
> static gboolean
> hostname_is_valid (const char *name)

Ick, but it matches the rest of the code, so OK. I don't care about the
function name so I've changed that too.

> I'd also prefer the following:

Yeah. Actually we can do even better than that. Below... and if this
message gets out then it's tested :)

Thanks for the feedback.

--- evolution-data-server-1.6.0/camel/providers/smtp/camel-smtp-transport.c.validatehelo	2005-08-31 05:26:04.000000000 +0100
+++ evolution-data-server-1.6.0/camel/providers/smtp/camel-smtp-transport.c	2006-04-04 23:27:23.000000000 +0100
@@ -873,6 +873,43 @@ smtp_set_exception (CamelSmtpTransport *
 }
 
 static gboolean
+hostname_is_valid(const char *name)
+{
+	enum { ALNUM, DASH, DOT } state = DOT;
+	int dotseen = 0;
+
+	if (!name)
+		return FALSE;
+
+	while (*name) {
+		switch(state) {
+		case ALNUM:
+			if (*name == '-') {
+				state = DASH;
+				break;
+			} else if (*name == '.') {
+				dotseen = 1;
+				state = DOT;
+				break;
+			} /* else ... */
+		case DOT:
+		case DASH:
+			if (!isalnum(*name))
+				return FALSE;
+			state = ALNUM;
+			break;
+		}
+		name++;
+	}
+
+	/* If it didn't end with an alphanumeric character, or there were no dots, it's invalid */
+	if (state != ALNUM || !dotseen)
+		return FALSE;
+	else
+		return TRUE;
+}
+
+static gboolean
 smtp_helo (CamelSmtpTransport *transport, CamelException *ex)
 {
 	/* say hello to the server */
@@ -893,8 +930,14 @@ smtp_helo (CamelSmtpTransport *transport
 	
 	camel_operation_start_transient (NULL, _("SMTP Greeting"));
 
-	/* force name resolution first, fallback to numerical, we need to know when it falls back */
-	if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NAMEREQD, NULL) != 0) {
+	/* force name resolution first, but check the resulting name is valid according to RFC2821 */
+	if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NAMEREQD, NULL) ||
+	    !hostname_is_valid(name)) {
+		g_free(name);
+		name = NULL;
+	}
+	/* If the name lookup failed, or the name wasn't acceptable, then try to use a numeric HELO domain */
+	if (!name) {
 		if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NUMERICHOST, NULL) != 0)
 			name = g_strdup("localhost.localdomain");
 		else {

-- 
dwmw2