Make all TLV parsing functions look the same for consistency.

Also, add one more safety check in recv_init().

4 files changed, 45 insertions, 33 deletions

diff --git a/usr.sbin/ldpd/hello.c b/usr.sbin/ldpd/hello.c
index 5d8cd202f4a..2128ef72fcb 100644
--- a/usr.sbin/ldpd/hello.c
+++ b/usr.sbin/ldpd/hello.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: hello.c,v 1.45 2016/06/09 17:26:32 renato Exp $ */
+/*	$OpenBSD: hello.c,v 1.46 2016/06/11 01:55:35 renato Exp $ */
 
 /*
  * Copyright (c) 2013, 2016 Renato Westphal <renato@openbsd.org>
@@ -472,8 +472,12 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 	 * given address family".
 	 */
 	while (len >= sizeof(tlv)) {
-		memcpy(&tlv, buf, sizeof(tlv));
+		memcpy(&tlv, buf, TLV_HDR_LEN);
+		buf += TLV_HDR_LEN;
+		len -= TLV_HDR_LEN;
+		total += TLV_HDR_LEN;
 		tlv_len = ntohs(tlv.length);
+
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_IPV4TRANSADDR:
 			if (tlv_len != sizeof(addr->v4))
@@ -482,7 +486,7 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 				return (-1);
 			if (*tlvs_rcvd & F_HELLO_TLV_RCVD_ADDR)
 				break;
-			memcpy(&addr->v4, buf + TLV_HDR_LEN, sizeof(addr->v4));
+			memcpy(&addr->v4, buf, sizeof(addr->v4));
 			*tlvs_rcvd |= F_HELLO_TLV_RCVD_ADDR;
 			break;
 		case TLV_TYPE_IPV6TRANSADDR:
@@ -492,14 +496,13 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 				return (-1);
 			if (*tlvs_rcvd & F_HELLO_TLV_RCVD_ADDR)
 				break;
-			memcpy(&addr->v6, buf + TLV_HDR_LEN, sizeof(addr->v6));
+			memcpy(&addr->v6, buf, sizeof(addr->v6));
 			*tlvs_rcvd |= F_HELLO_TLV_RCVD_ADDR;
 			break;
 		case TLV_TYPE_CONFIG:
 			if (tlv_len != sizeof(uint32_t))
 				return (-1);
-			memcpy(conf_number, buf + TLV_HDR_LEN,
-			    sizeof(uint32_t));
+			memcpy(conf_number, buf, sizeof(uint32_t));
 			*tlvs_rcvd |= F_HELLO_TLV_RCVD_CONF;
 			break;
 		case TLV_TYPE_DUALSTACK:
@@ -515,12 +518,11 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 				break;
 			/* Shame on you, Cisco! */
 			if (leconf->flags & F_LDPD_DS_CISCO_INTEROP) {
-				memcpy(trans_pref, buf + TLV_HDR_LEN +
-				    sizeof(uint16_t), sizeof(uint16_t));
+				memcpy(trans_pref, buf + sizeof(uint16_t),
+				    sizeof(uint16_t));
 				*trans_pref = ntohs(*trans_pref);
 			} else {
-				memcpy(trans_pref, buf + TLV_HDR_LEN,
-				    sizeof(uint16_t));
+				memcpy(trans_pref, buf , sizeof(uint16_t));
 				*trans_pref = ntohs(*trans_pref) >> 12;
 			}
 			*tlvs_rcvd |= F_HELLO_TLV_RCVD_DS;
@@ -531,9 +533,9 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af,
 				return (-1);
 			break;
 		}
-		buf += TLV_HDR_LEN + tlv_len;
-		len -= TLV_HDR_LEN + tlv_len;
-		total += TLV_HDR_LEN + tlv_len;
+		buf += tlv_len;
+		len -= tlv_len;
+		total += tlv_len;
 	}
 
 	return (total);
diff --git a/usr.sbin/ldpd/init.c b/usr.sbin/ldpd/init.c
index d4b7433860c..7832cd1671e 100644
--- a/usr.sbin/ldpd/init.c
+++ b/usr.sbin/ldpd/init.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: init.c,v 1.28 2016/06/08 21:28:09 renato Exp $ */
+/*	$OpenBSD: init.c,v 1.29 2016/06/11 01:55:35 renato Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -54,6 +54,7 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len)
 	struct ldp_msg		init;
 	struct sess_prms_tlv	sess;
 	uint16_t		max_pdu_len;
+	int			r;
 
 	log_debug("%s: lsr-id %s", __func__, inet_ntoa(nbr->id));
 
@@ -88,7 +89,8 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len)
 	len -= SESS_PRMS_SIZE;
 
 	/* just ignore all optional TLVs for now */
-	if (tlv_decode_opt_init_prms(buf, len) == -1) {
+	r = tlv_decode_opt_init_prms(buf, len);
+	if (r == -1 || r != len) {
 		session_shutdown(nbr, S_BAD_TLV_VAL, init.msgid, init.type);
 		return (-1);
 	}
@@ -138,8 +140,12 @@ tlv_decode_opt_init_prms(char *buf, uint16_t len)
 	int		total = 0;
 
 	 while (len >= sizeof(tlv)) {
-		memcpy(&tlv, buf, sizeof(tlv));
+		memcpy(&tlv, buf, TLV_HDR_LEN);
+		buf += TLV_HDR_LEN;
+		len -= TLV_HDR_LEN;
+		total += TLV_HDR_LEN;
 		tlv_len = ntohs(tlv.length);
+
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_ATMSESSIONPAR:
 			log_warnx("ATM session parameter present");
@@ -153,9 +159,9 @@ tlv_decode_opt_init_prms(char *buf, uint16_t len)
 				return (-1);
 			break;
 		}
-		buf += TLV_HDR_LEN + tlv_len;
-		len -= TLV_HDR_LEN + tlv_len;
-		total += TLV_HDR_LEN + tlv_len;
+		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 3aa63d7fd25..48bb19414b4 100644
--- a/usr.sbin/ldpd/labelmapping.c
+++ b/usr.sbin/ldpd/labelmapping.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: labelmapping.c,v 1.48 2016/06/11 01:52:33 renato Exp $ */
+/*	$OpenBSD: labelmapping.c,v 1.49 2016/06/11 01:55:35 renato Exp $ */
 
 /*
  * Copyright (c) 2014, 2015 Renato Westphal <renato@openbsd.org>
@@ -232,6 +232,7 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 	/* Optional Parameters */
 	while (len > 0) {
 		struct tlv 	tlv;
+		uint16_t	tlv_len;
 		uint32_t	reqbuf, labelbuf, statusbuf;
 
 		if (len < sizeof(tlv)) {
@@ -240,16 +241,17 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 			goto err;
 		}
 
-		memcpy(&tlv, buf, sizeof(tlv));
+		memcpy(&tlv, buf, TLV_HDR_LEN);
 		buf += TLV_HDR_LEN;
 		len -= TLV_HDR_LEN;
+		tlv_len = ntohs(tlv.length);
 
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_LABELREQUEST:
 			switch (type) {
 			case MSG_TYPE_LABELMAPPING:
 			case MSG_TYPE_LABELREQUEST:
-				if (ntohs(tlv.length) != 4) {
+				if (tlv_len != 4) {
 					session_shutdown(nbr, S_BAD_TLV_LEN,
 					    lm.msgid, lm.type);
 					goto err;
@@ -272,7 +274,7 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 			switch (type) {
 			case MSG_TYPE_LABELWITHDRAW:
 			case MSG_TYPE_LABELRELEASE:
-				if (ntohs(tlv.length) != 4) {
+				if (tlv_len != 4) {
 					session_shutdown(nbr, S_BAD_TLV_LEN,
 					    lm.msgid, lm.type);
 					goto err;
@@ -304,7 +306,7 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 		case TLV_TYPE_PW_STATUS:
 			switch (type) {
 			case MSG_TYPE_LABELMAPPING:
-				if (ntohs(tlv.length) != 4) {
+				if (tlv_len != 4) {
 					session_shutdown(nbr, S_BAD_TLV_LEN,
 					    lm.msgid, lm.type);
 					goto err;
@@ -327,8 +329,8 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type)
 			/* ignore unknown tlv */
 			break;
 		}
-		buf += ntohs(tlv.length);
-		len -= ntohs(tlv.length);
+		buf += tlv_len;
+		len -= tlv_len;
 	}
 
 	/* notify lde about the received message. */
diff --git a/usr.sbin/ldpd/notification.c b/usr.sbin/ldpd/notification.c
index 87b2929b1f5..06233cf9e86 100644
--- a/usr.sbin/ldpd/notification.c
+++ b/usr.sbin/ldpd/notification.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: notification.c,v 1.34 2016/06/11 01:52:33 renato Exp $ */
+/*	$OpenBSD: notification.c,v 1.35 2016/06/11 01:55:35 renato Exp $ */
 
 /*
  * Copyright (c) 2009 Michele Marchetto <michele@openbsd.org>
@@ -122,6 +122,7 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 	/* Optional Parameters */
 	while (len > 0) {
 		struct tlv 	tlv;
+		uint16_t	tlv_len;
 
 		if (len < sizeof(tlv)) {
 			session_shutdown(nbr, S_BAD_TLV_LEN, not.msgid,
@@ -129,9 +130,10 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 			return (-1);
 		}
 
-		memcpy(&tlv, buf, sizeof(tlv));
+		memcpy(&tlv, buf, TLV_HDR_LEN);
 		buf += TLV_HDR_LEN;
 		len -= TLV_HDR_LEN;
+		tlv_len = ntohs(tlv.length);
 
 		switch (ntohs(tlv.type)) {
 		case TLV_TYPE_EXTSTATUS:
@@ -140,7 +142,7 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 			/* TODO is there any use for this? */
 			break;
 		case TLV_TYPE_PW_STATUS:
-			if (ntohs(tlv.length) != 4) {
+			if (tlv_len != 4) {
 				session_shutdown(nbr, S_BAD_TLV_LEN,
 				    not.msgid, not.type);
 				return (-1);
@@ -151,10 +153,10 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 			break;
 		case TLV_TYPE_FEC:
 			if ((tlen = tlv_decode_fec_elm(nbr, &not, buf,
-			    ntohs(tlv.length), &nm.fec)) == -1)
+			    tlv_len, &nm.fec)) == -1)
 				return (-1);
 			/* allow only one fec element */
-			if (tlen != ntohs(tlv.length)) {
+			if (tlen != tlv_len) {
 				session_shutdown(nbr, S_BAD_TLV_VAL,
 				    not.msgid, not.type);
 				return (-1);
@@ -169,8 +171,8 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len)
 			/* ignore unknown tlv */
 			break;
 		}
-		buf += ntohs(tlv.length);
-		len -= ntohs(tlv.length);
+		buf += tlv_len;
+		len -= tlv_len;
 	}
 
 	if (nm.status == S_PW_STATUS) {