Fix payload handling flaws found by cloder@. Based on initial patch by

cloder@.  Testing by markus@ cloder@ hshoexer@.

ok ho@

5 files changed, 45 insertions, 36 deletions

diff --git a/sbin/isakmpd/doi.h b/sbin/isakmpd/doi.h
index 201b4d818f7..bf5b4b69b2c 100644
--- a/sbin/isakmpd/doi.h
+++ b/sbin/isakmpd/doi.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: doi.h,v 1.11 2003/06/03 14:28:16 ho Exp $	*/
+/*	$OpenBSD: doi.h,v 1.12 2004/03/10 23:08:48 hshoexer Exp $	*/
 /*	$EOM: doi.h,v 1.29 2000/07/02 18:47:15 provos Exp $	*/
 
 /*
@@ -83,7 +83,7 @@ struct doi {
   int (*validate_key_information) (u_int8_t *, size_t);
   int (*validate_notification) (u_int16_t);
   int (*validate_proto) (u_int8_t);
-  int (*validate_situation) (u_int8_t *, size_t *);
+  int (*validate_situation) (u_int8_t *, size_t *, size_t);
   int (*validate_transform_id) (u_int8_t, u_int8_t);
   int (*initiator) (struct message *msg);
   int (*responder) (struct message *msg);
diff --git a/sbin/isakmpd/ipsec.c b/sbin/isakmpd/ipsec.c
index c32928ad88a..6a547a6c64a 100644
--- a/sbin/isakmpd/ipsec.c
+++ b/sbin/isakmpd/ipsec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ipsec.c,v 1.86 2004/02/27 19:14:57 hshoexer Exp $	*/
+/*	$OpenBSD: ipsec.c,v 1.87 2004/03/10 23:08:48 hshoexer Exp $	*/
 /*	$EOM: ipsec.c,v 1.143 2000/12/11 23:57:42 niklas Exp $	*/
 
 /*
@@ -122,7 +122,7 @@ static int ipsec_validate_id_information (u_int8_t, u_int8_t *, u_int8_t *,
 static int ipsec_validate_key_information (u_int8_t *, size_t);
 static int ipsec_validate_notification (u_int16_t);
 static int ipsec_validate_proto (u_int8_t);
-static int ipsec_validate_situation (u_int8_t *, size_t *);
+static int ipsec_validate_situation (u_int8_t *, size_t *, size_t);
 static int ipsec_validate_transform_id (u_int8_t, u_int8_t);
 
 static struct doi ipsec_doi = {
@@ -851,34 +851,22 @@ ipsec_validate_proto (u_int8_t proto)
 }
 
 static int
-ipsec_validate_situation (u_int8_t *buf, size_t *sz)
+ipsec_validate_situation (u_int8_t *buf, size_t *sz, size_t len)
 {
-  int sit = GET_IPSEC_SIT_SIT (buf);
-  int off;
-
-  if (sit & (IPSEC_SIT_SECRECY | IPSEC_SIT_INTEGRITY))
+  if (len < IPSEC_SIT_SIT_OFF + IPSEC_SIT_SIT_LEN)
     {
-      /*
-       * XXX All the roundups below, round up to 32 bit boundaries given
-       * that the situation field is aligned.  This is not necessarily so,
-       * but I interpret the drafts as this is like this they want it.
-       */
-      off = ROUNDUP_32 (GET_IPSEC_SIT_SECRECY_LENGTH (buf));
-      off += ROUNDUP_32 (GET_IPSEC_SIT_SECRECY_CAT_LENGTH (buf + off));
-      off += ROUNDUP_32 (GET_IPSEC_SIT_INTEGRITY_LENGTH (buf + off));
-      off += ROUNDUP_32 (GET_IPSEC_SIT_INTEGRITY_CAT_LENGTH (buf + off));
-      *sz = off + IPSEC_SIT_SZ;
+      log_print ("ipsec_validate_situation: payload too short: %u",
+	         (unsigned int)len);
+      return -1;
     }
-  else
-    *sz = IPSEC_SIT_SIT_LEN;
 
   /* Currently only "identity only" situations are supported.  */
-#ifdef notdef
-  return
-    sit & ~(IPSEC_SIT_IDENTITY_ONLY | IPSEC_SIT_SECRECY | IPSEC_SIT_INTEGRITY);
-#else
-  return sit & ~IPSEC_SIT_IDENTITY_ONLY;
-#endif
+  if (GET_IPSEC_SIT_SIT (buf) != IPSEC_SIT_IDENTITY_ONLY)
+    return 1;
+
+  *sz = IPSEC_SIT_SIT_LEN;
+
+  return 0;
 }
 
 static int
diff --git a/sbin/isakmpd/isakmp_doi.c b/sbin/isakmpd/isakmp_doi.c
index 5d6de9a6f07..0aab1c83712 100644
--- a/sbin/isakmpd/isakmp_doi.c
+++ b/sbin/isakmpd/isakmp_doi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: isakmp_doi.c,v 1.17 2003/10/14 14:29:15 ho Exp $	*/
+/*	$OpenBSD: isakmp_doi.c,v 1.18 2004/03/10 23:08:49 hshoexer Exp $	*/
 /*	$EOM: isakmp_doi.c,v 1.42 2000/09/12 16:29:41 ho Exp $	*/
 
 /*
@@ -67,7 +67,7 @@ static int isakmp_validate_id_information (u_int8_t, u_int8_t *, u_int8_t *,
 static int isakmp_validate_key_information (u_int8_t *, size_t);
 static int isakmp_validate_notification (u_int16_t);
 static int isakmp_validate_proto (u_int8_t);
-static int isakmp_validate_situation (u_int8_t *, size_t *);
+static int isakmp_validate_situation (u_int8_t *, size_t *, size_t);
 static int isakmp_validate_transform_id (u_int8_t, u_int8_t);
 
 static struct doi isakmp_doi = {
@@ -197,7 +197,7 @@ isakmp_validate_proto (u_int8_t proto)
 }
 
 static int
-isakmp_validate_situation (u_int8_t *buf, size_t *sz)
+isakmp_validate_situation (u_int8_t *buf, size_t *sz, size_t len)
 {
   /* There are no situations in the ISAKMP DOI.  */
   *sz = 0;
diff --git a/sbin/isakmpd/message.c b/sbin/isakmpd/message.c
index 1d814a5467b..1ffa957d5f5 100644
--- a/sbin/isakmpd/message.c
+++ b/sbin/isakmpd/message.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: message.c,v 1.68 2004/03/10 16:10:57 hshoexer Exp $	*/
+/*	$OpenBSD: message.c,v 1.69 2004/03/10 23:08:49 hshoexer Exp $	*/
 /*	$EOM: message.c,v 1.156 2000/10/10 12:36:39 provos Exp $	*/
 
 /*
@@ -110,6 +110,13 @@ static struct field *fields[] = {
   isakmp_vendor_fld, isakmp_attribute_fld
 };
 
+static u_int16_t min_payload_lengths[] = {
+  0, ISAKMP_SA_SZ, ISAKMP_PROP_SZ, ISAKMP_TRANSFORM_SZ, ISAKMP_KE_SZ,
+  ISAKMP_ID_SZ, ISAKMP_CERT_SZ, ISAKMP_CERTREQ_SZ, ISAKMP_HASH_SZ,
+  ISAKMP_SIG_SZ, ISAKMP_NONCE_SZ, ISAKMP_NOTIFY_SZ, ISAKMP_DELETE_SZ,
+  ISAKMP_VENDOR_SZ, ISAKMP_ATTRIBUTE_SZ
+};
+
 /*
  * Fields used for checking monotonic increasing of proposal and transform
  * numbers.
@@ -283,10 +290,25 @@ message_parse_payloads (struct message *msg, struct payload *p, u_int8_t next,
 	}
 
       /*
-       * Decode the payload length field.
+       * Decode and validate the payload length field.
        */
       len = GET_ISAKMP_GEN_LENGTH (buf);
 
+      if ((payload < ISAKMP_PAYLOAD_RESERVED_MIN)
+	   && (len < min_payload_lengths[payload]))
+	{
+	  log_print ("message_parse_payloads: payload too short: %u", len);
+	  message_drop (msg, ISAKMP_NOTIFY_PAYLOAD_MALFORMED, 0, 1, 1);
+	  return -1;
+	}
+  
+      if (buf + len > (u_int8_t *)msg->iov[0].iov_base + msg->iov[0].iov_len)
+	{
+	  log_print ("message_parse_payloads: payload too long: %u", len);
+	  message_drop (msg, ISAKMP_NOTIFY_PAYLOAD_MALFORMED, 0, 1, 1);
+	  return -1;
+	}
+
       /* Ignore private payloads.  */
       if (next >= ISAKMP_PAYLOAD_PRIVATE_MIN)
 	{
@@ -874,7 +896,8 @@ message_validate_sa (struct message *msg, struct payload *p)
    * Let the DOI validate the situation, at the same time it tells us what
    * the length of the situation field is.
    */
-  if (exchange->doi->validate_situation (p->p + ISAKMP_SA_SIT_OFF, &len))
+  if (exchange->doi->validate_situation (p->p + ISAKMP_SA_SIT_OFF, &len,
+      GET_ISAKMP_GEN_LENGTH (p->p) - ISAKMP_SA_SIT_OFF))
     {
       log_print ("message_validate_sa: situation not supported");
       message_drop (msg, ISAKMP_NOTIFY_SITUATION_NOT_SUPPORTED, 0, 1, 1);
diff --git a/sbin/isakmpd/util.h b/sbin/isakmpd/util.h
index 7c5547c0d35..d3d94461f30 100644
--- a/sbin/isakmpd/util.h
+++ b/sbin/isakmpd/util.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.h,v 1.16 2003/12/14 14:50:23 ho Exp $	*/
+/*	$OpenBSD: util.h,v 1.17 2004/03/10 23:08:49 hshoexer Exp $	*/
 /*	$EOM: util.h,v 1.10 2000/10/24 13:33:39 niklas Exp $	*/
 
 /*
@@ -35,8 +35,6 @@
 
 #include <sys/types.h>
 
-#define ROUNDUP_32(x) (((x) + 3) & ~4)
-
 extern int allow_name_lookups;
 extern int regrand;
 extern unsigned long seed;