fix/improve envelope_validate():

- return an informative error string if the envelope is invalid.
- take the envelope id as a parameter and make sure it matches.
- do not expect the errorline to start with an SMTP response code,
  as this is not always the case: a temporary failure with mda would
  cause the envelope to be marked as corrupted. Instead, just make sure
  that all string fields are actual strings to prevent overflows later.

ok gilles@ chl@

1 files changed, 27 insertions, 21 deletions

diff --git a/usr.sbin/smtpd/queue_backend.c b/usr.sbin/smtpd/queue_backend.c
index 45a3ff56b0c..c4b90c4b4b4 100644
--- a/usr.sbin/smtpd/queue_backend.c
+++ b/usr.sbin/smtpd/queue_backend.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: queue_backend.c,v 1.16 2011/12/16 17:35:00 eric Exp $	*/
+/*	$OpenBSD: queue_backend.c,v 1.17 2011/12/19 19:57:25 eric Exp $	*/
 
 /*
  * Copyright (c) 2011 Gilles Chehade <gilles@openbsd.org>
@@ -36,7 +36,7 @@
 #include "smtpd.h"
 #include "log.h"
 
-static int envelope_validate(struct envelope *);
+static const char* envelope_validate(struct envelope *, uint64_t);
 
 /* fsqueue backend */
 extern struct queue_backend	queue_backend_fs;
@@ -114,9 +114,14 @@ queue_envelope_delete(enum queue_kind qkind, struct envelope *ep)
 int
 queue_envelope_load(enum queue_kind qkind, u_int64_t evpid, struct envelope *ep)
 {
+	const char	*e;
+
 	ep->id = evpid;
-	if (env->sc_queue->envelope(qkind, QOP_LOAD, ep))
-		return envelope_validate(ep);
+	if (env->sc_queue->envelope(qkind, QOP_LOAD, ep)) {
+		if ((e = envelope_validate(ep, evpid)) == NULL)
+			return 1;
+		log_debug("invalid envelope %016" PRIx64 ": %s", ep->id, e);
+	}
 	return 0;
 }
 
@@ -173,29 +178,30 @@ queue_generate_evpid(u_int32_t msgid)
 
 
 /**/
-static int
-envelope_validate(struct envelope *ep)
+static const char*
+envelope_validate(struct envelope *ep, uint64_t id)
 {
 	if (ep->version != SMTPD_ENVELOPE_VERSION)
-		return 0;
+		return "version mismatch";
+
+	if ((ep->id & 0xffffffff) == 0 || ((ep->id >> 32) & 0xffffffff) == 0)
+		return "invalid id";
 
-	if ((ep->id & 0xffffffff) == 0 ||
-	    ((ep->id >> 32) & 0xffffffff) == 0)
-		return 0;
+	if (ep->id != id)
+		return "id mismatch";
 
+	if (memchr(ep->helo, '\0', sizeof(ep->helo)) == NULL)
+		return "invalid helo";
 	if (ep->helo[0] == '\0')
-		return 0;
+		return "empty helo";
 
+	if (memchr(ep->hostname, '\0', sizeof(ep->hostname)) == NULL)
+		return "invalid hostname";
 	if (ep->hostname[0] == '\0')
-		return 0;
-
-	if (ep->errorline[0] != '\0') {
-		if (! isdigit(ep->errorline[0]) ||
-		    ! isdigit(ep->errorline[1]) ||
-		    ! isdigit(ep->errorline[2]) ||
-		    ep->errorline[3] != ' ')
-			return 0;
-	}
+		return "empty hostname";
+
+	if (memchr(ep->errorline, '\0', sizeof(ep->errorline)) == NULL)
+		return "invalid error line";
 
-	return 1;
+	return NULL;
 }