From: hshoexer <hshoexer@yerbouti.franken.de>
Subject: isakmpd(8): Validate proposal and transform sizes
To: tech@openbsd.org
Date: Wed, 22 Apr 2026 15:01:20 +0200

Check
- that a proposal payload fits within the outer SA payload,
- that the provided SPI and the following transform header fit
  within the proposal, and
- transforms fit within the outer proposal payload.

It's already ensured that we never read outside the message.  However,
within the message the mentioned payloads might be misinterpreted
due to malformed size values.

ok?

Take care,
HJ.

------------------------------------------------------------------------------
diff --git a/sbin/isakmpd/message.c b/sbin/isakmpd/message.c
index ee0df3a4282..c49a14d67b7 100644
--- a/sbin/isakmpd/message.c
+++ b/sbin/isakmpd/message.c
@@ -335,7 +335,7 @@ next_payload:
  * Parse a proposal payload found in message MSG.  PAYLOAD is always
  * ISAKMP_PAYLOAD_PROPOSAL and ignored in here.  It's needed as the API for
  * message_parse_payloads requires it.  BUF points to the proposal's
- * generic payload header.
+ * generic payload header.  P is the outer SA payload.
  */
 static int
 message_parse_proposal(struct message *msg, struct payload *p,
@@ -343,6 +343,19 @@ message_parse_proposal(struct message *msg, struct payload *p,
 {
 	set	payload_set;
 
+	/* Proposal must lie within outer SA payload. */
+	if (buf + GET_ISAKMP_GEN_LENGTH(buf) >
+	    p->p + GET_ISAKMP_GEN_LENGTH(p->p)) {
+		message_drop(msg, ISAKMP_NOTIFY_PAYLOAD_MALFORMED, 0, 1, 1);
+		return -1;
+	}
+
+	if ((ISAKMP_PROP_SPI_OFF + GET_ISAKMP_PROP_SPI_SZ(buf) +
+	    ISAKMP_GEN_SZ) > GET_ISAKMP_GEN_LENGTH(buf)) {
+		message_drop(msg, ISAKMP_NOTIFY_PAYLOAD_MALFORMED, 0, 1, 1);
+		return -1;
+	}
+
 	/* Put the proposal into the proposal bucket.  */
 	if (message_index_payload(msg, p, payload, buf) == -1)
 		return -1;
@@ -363,6 +376,13 @@ static int
 message_parse_transform(struct message *msg, struct payload *p,
     u_int8_t payload, u_int8_t *buf)
 {
+	/* Transform must lie within outer proposal payload. */
+	if (buf + GET_ISAKMP_GEN_LENGTH(buf) >
+	    p->p + GET_ISAKMP_GEN_LENGTH(p->p)) {
+		message_drop(msg, ISAKMP_NOTIFY_PAYLOAD_MALFORMED, 0, 1, 1);
+		return -1;
+	}
+
 	/* Put the transform into the transform bucket.  */
 	if (message_index_payload(msg, p, payload, buf) == -1)
 		return -1;