From: renato <renato@openbsd.org>
Date: Sat, 16 Jul 2016 19:20:16 +0000 (+0000)
Subject: Fix parsing of mal-formed optional TLVs/Sub-TLVs.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a78ea73f827928c1e19cfa5f76567b253c6e1213;p=openbsd

Fix parsing of mal-formed optional TLVs/Sub-TLVs.

We must detect if a TLV's length extends beyond the end of the containing
message. And, if so, send a fatal "Bad TLV Length" notification message.

Found with the Mu Dynamics Mu-8000 protocol fuzzer.
---

diff --git a/usr.sbin/ldpd/hello.c b/usr.sbin/ldpd/hello.c
index 63c93b7b115..f6d6245b3bd 100644
--- a/usr.sbin/ldpd/hello.c
+++ b/usr.sbin/ldpd/hello.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: hello.c,v 1.56 2016/07/01 23:36:38 renato Exp $ */
+/*	$OpenBSD: hello.c,v 1.57 2016/07/16 19:20:16 renato Exp $ */
 
 /*
  * Copyright (c) 2013, 2016 Renato Westphal <renato@openbsd.org>
@@ -503,10 +503,12 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 	 */
 	while (len >= sizeof(tlv)) {
 		memcpy(&tlv, buf, TLV_HDR_SIZE);
+		tlv_len = ntohs(tlv.length);
+		if (tlv_len + TLV_HDR_SIZE > len)
+			return (-1);
 		buf += TLV_HDR_SIZE;
 		len -= TLV_HDR_SIZE;
 		total += TLV_HDR_SIZE;
-		tlv_len = ntohs(tlv.length);
 
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_IPV4TRANSADDR:
diff --git a/usr.sbin/ldpd/init.c b/usr.sbin/ldpd/init.c
index 56cc14dcda0..f3bdf7b0648 100644
--- a/usr.sbin/ldpd/init.c
+++ b/usr.sbin/ldpd/init.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: init.c,v 1.32 2016/07/01 23:36:38 renato Exp $ */
+/*	$OpenBSD: init.c,v 1.33 2016/07/16 19:20:16 renato Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -25,7 +25,6 @@
 #include "log.h"
 
 static int	gen_init_prms_tlv(struct ibuf *, struct nbr *);
-static int	tlv_decode_opt_init_prms(char *, uint16_t);
 
 void
 send_init(struct nbr *nbr)
@@ -59,7 +58,6 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len)
 	struct ldp_msg		msg;
 	struct sess_prms_tlv	sess;
 	uint16_t		max_pdu_len;
-	int			r;
 
 	log_debug("%s: lsr-id %s", __func__, inet_ntoa(nbr->id));
 
@@ -93,11 +91,41 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len)
 	buf += SESS_PRMS_SIZE;
 	len -= SESS_PRMS_SIZE;
 
-	/* just ignore all optional TLVs for now */
-	r = tlv_decode_opt_init_prms(buf, len);
-	if (r == -1 || r != len) {
-		session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type);
-		return (-1);
+	/* Optional Parameters */
+	while (len > 0) {
+		struct tlv 	tlv;
+		uint16_t	tlv_len;
+
+		if (len < sizeof(tlv)) {
+			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type);
+			return (-1);
+		}
+
+		memcpy(&tlv, buf, TLV_HDR_SIZE);
+		tlv_len = ntohs(tlv.length);
+		if (tlv_len + TLV_HDR_SIZE > len) {
+			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type);
+			return (-1);
+		}
+		buf += TLV_HDR_SIZE;
+		len -= TLV_HDR_SIZE;
+
+		switch (ntohs(tlv.type)) {
+		case TLV_TYPE_ATMSESSIONPAR:
+			session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type);
+			return (-1);
+		case TLV_TYPE_FRSESSION:
+			session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type);
+			return (-1);
+		default:
+			if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
+				send_notification_nbr(nbr, S_UNKNOWN_TLV,
+				    msg.id, msg.type);
+			/* ignore unknown tlv */
+			break;
+		}
+		buf += tlv_len;
+		len -= tlv_len;
 	}
 
 	nbr->keepalive = min(nbr_get_keepalive(nbr->af, nbr->id),
@@ -136,38 +164,3 @@ gen_init_prms_tlv(struct ibuf *buf, struct nbr *nbr)
 
 	return (ibuf_add(buf, &parms, SESS_PRMS_SIZE));
 }
-
-static int
-tlv_decode_opt_init_prms(char *buf, uint16_t len)
-{
-	struct tlv	tlv;
-	uint16_t	tlv_len;
-	int		total = 0;
-
-	 while (len >= sizeof(tlv)) {
-		memcpy(&tlv, buf, TLV_HDR_SIZE);
-		buf += TLV_HDR_SIZE;
-		len -= TLV_HDR_SIZE;
-		total += TLV_HDR_SIZE;
-		tlv_len = ntohs(tlv.length);
-
-		switch (ntohs(tlv.type)) {
-		case TLV_TYPE_ATMSESSIONPAR:
-			log_warnx("ATM session parameter present");
-			return (-1);
-		case TLV_TYPE_FRSESSION:
-			log_warnx("FR session parameter present");
-			return (-1);
-		default:
-			/* if unknown flag set, ignore TLV */
-			if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
-				return (-1);
-			break;
-		}
-		buf += tlv_len;
-		len -= tlv_len;
-		total += tlv_len;
-	}
-
-	return (total);
-}
diff --git a/usr.sbin/ldpd/labelmapping.c b/usr.sbin/ldpd/labelmapping.c
index a1ec305f0ca..f8261e6d6d8 100644
--- a/usr.sbin/ldpd/labelmapping.c
+++ b/usr.sbin/ldpd/labelmapping.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: labelmapping.c,v 1.57 2016/07/15 17:09:25 renato Exp $ */
+/*	$OpenBSD: labelmapping.c,v 1.58 2016/07/16 19:20:16 renato Exp $ */
 
 /*
  * Copyright (c) 2014, 2015 Renato Westphal <renato@openbsd.org>
@@ -246,9 +246,13 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 		}
 
 		memcpy(&tlv, buf, TLV_HDR_SIZE);
+		tlv_len = ntohs(tlv.length);
+		if (tlv_len + TLV_HDR_SIZE > len) {
+			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type);
+			goto err;
+		}
 		buf += TLV_HDR_SIZE;
 		len -= TLV_HDR_SIZE;
-		tlv_len = ntohs(tlv.length);
 
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_LABELREQUEST:
@@ -334,10 +338,9 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 			}
 			break;
 		default:
-			if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) {
+			if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
 				send_notification_nbr(nbr, S_UNKNOWN_TLV,
 				    msg.id, msg.type);
-			}
 			/* ignore unknown tlv */
 			break;
 		}
@@ -708,6 +711,12 @@ tlv_decode_fec_elm(struct nbr *nbr, struct ldp_msg *msg, char *buf,
 			}
 
 			memcpy(&stlv, buf + off, sizeof(stlv));
+			if (stlv.length > pw_len) {
+				session_shutdown(nbr, S_BAD_TLV_LEN, msg->id,
+				    msg->type);
+				return (-1);
+			}
+
 			switch (stlv.type) {
 			case SUBTLV_IFMTU:
 				if (stlv.length != FEC_SUBTLV_IFMTU_SIZE) {
diff --git a/usr.sbin/ldpd/notification.c b/usr.sbin/ldpd/notification.c
index 73788da9133..489001289a1 100644
--- a/usr.sbin/ldpd/notification.c
+++ b/usr.sbin/ldpd/notification.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: notification.c,v 1.39 2016/07/01 23:36:38 renato Exp $ */
+/*	$OpenBSD: notification.c,v 1.40 2016/07/16 19:20:16 renato Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -128,15 +128,18 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 		uint16_t	tlv_len;
 
 		if (len < sizeof(tlv)) {
-			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id,
-			    msg.type);
+			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type);
 			return (-1);
 		}
 
 		memcpy(&tlv, buf, TLV_HDR_SIZE);
+		tlv_len = ntohs(tlv.length);
+		if (tlv_len + TLV_HDR_SIZE > len) {
+			session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type);
+			return (-1);
+		}
 		buf += TLV_HDR_SIZE;
 		len -= TLV_HDR_SIZE;
-		tlv_len = ntohs(tlv.length);
 
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_EXTSTATUS:
@@ -167,10 +170,9 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 			nm.flags |= F_NOTIF_FEC;
 			break;
 		default:
-			if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) {
+			if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
 				send_notification_nbr(nbr, S_UNKNOWN_TLV,
 				    msg.id, msg.type);
-			}
 			/* ignore unknown tlv */
 			break;
 		}