Add length check for Proxy LCP and Authentication AVPs.

2 files changed, 35 insertions, 19 deletions

diff --git a/usr.sbin/npppd/l2tp/l2tp_call.c b/usr.sbin/npppd/l2tp/l2tp_call.c
index 562ca713840..51b470d8ece 100644
--- a/usr.sbin/npppd/l2tp/l2tp_call.c
+++ b/usr.sbin/npppd/l2tp/l2tp_call.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: l2tp_call.c,v 1.14 2012/09/18 13:14:08 yasuoka Exp $	*/
+/*	$OpenBSD: l2tp_call.c,v 1.15 2013/09/20 07:26:23 yasuoka Exp $	*/
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -25,7 +25,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-/* $Id: l2tp_call.c,v 1.14 2012/09/18 13:14:08 yasuoka Exp $ */
+/* $Id: l2tp_call.c,v 1.15 2013/09/20 07:26:23 yasuoka Exp $ */
 /**@file L2TP LNS call */
 #include <sys/types.h>
 #include <sys/param.h>
@@ -525,39 +525,44 @@ l2tp_call_recv_ICCN(l2tp_call *_this, u_char *pkt, int pktlen,
 			_this->seq_required = 1;
 			_this->use_seq = 1;
 			continue;
-#ifndef	L2TPD_TEST
 	    /*
 	     * AVP's for Proxy-LCP and Proxy-Authen
 	     */
 		case L2TP_AVP_TYPE_LAST_SENT_LCP_CONFREQ:
+			AVP_MAXLEN_CHECK(avp, sizeof(dpi->last_sent_lcp.data));
 			memcpy(dpi->last_sent_lcp.data, avp->attr_value,
 			    avp_attr_length(avp));
 			dpi->last_sent_lcp.ldata = avp_attr_length(avp);
 			break;
 		case L2TP_AVP_TYPE_LAST_RECV_LCP_CONFREQ:
+			AVP_MAXLEN_CHECK(avp, sizeof(dpi->last_recv_lcp.data));
 			memcpy(dpi->last_recv_lcp.data, avp->attr_value,
 			    avp_attr_length(avp));
 			dpi->last_recv_lcp.ldata = avp_attr_length(avp);
 			break;
 		case L2TP_AVP_TYPE_PROXY_AUTHEN_CHALLENGE:
+			AVP_MAXLEN_CHECK(avp, sizeof(dpi->auth_chall));
 			memcpy(dpi->auth_chall, avp->attr_value,
-			    MIN(avp_attr_length(avp), sizeof(dpi->auth_chall)));
+			    avp_attr_length(avp));
 			dpi->lauth_chall = avp_attr_length(avp);
 			break;
 		case L2TP_AVP_TYPE_PROXY_AUTHEN_ID:
+			AVP_SIZE_CHECK(avp, ==, 8);
 			dpi->auth_id = avp_get_val16(avp);
 			break;
 		case L2TP_AVP_TYPE_PROXY_AUTHEN_NAME:
+			AVP_MAXLEN_CHECK(avp, sizeof(dpi->username) - 1);
 			memcpy(dpi->username, avp->attr_value,
-			    MIN(sizeof(dpi->username) - 1,
-			    avp_attr_length(avp)));
+			    avp_attr_length(avp));
 			break;
 		case L2TP_AVP_TYPE_PROXY_AUTHEN_RESPONSE:
+			AVP_MAXLEN_CHECK(avp, sizeof(dpi->auth_resp));
 			memcpy(dpi->auth_resp, avp->attr_value,
-			    MIN(avp_attr_length(avp), sizeof(dpi->auth_resp)));
+			    avp_attr_length(avp));
 			dpi->lauth_resp = avp_attr_length(avp);
 			break;
 		case L2TP_AVP_TYPE_PROXY_AUTHEN_TYPE:
+			AVP_SIZE_CHECK(avp, ==, 8);
 			switch (avp_get_val16(avp)) {
 			default:
 				l2tp_call_log(_this, LOG_WARNING,
@@ -578,7 +583,6 @@ l2tp_call_recv_ICCN(l2tp_call *_this, u_char *pkt, int pktlen,
 				break;
 			}
 			break;
-#endif
 		default:
 			if (avp->is_mandatory != 0) {
 				l2tp_call_log(_this, LOG_WARNING,
diff --git a/usr.sbin/npppd/l2tp/l2tp_subr.h b/usr.sbin/npppd/l2tp/l2tp_subr.h
index dfa61cb3eef..a49963a9801 100644
--- a/usr.sbin/npppd/l2tp/l2tp_subr.h
+++ b/usr.sbin/npppd/l2tp/l2tp_subr.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: l2tp_subr.h,v 1.5 2012/05/08 13:15:11 yasuoka Exp $	*/
+/*	$OpenBSD: l2tp_subr.h,v 1.6 2013/09/20 07:26:23 yasuoka Exp $	*/
 
 /*-
  * Copyright (c) 2009 Internet Initiative Japan Inc.
@@ -27,7 +27,7 @@
  */
 #ifndef	L2TP_SUBR_H
 #define	L2TP_SUBR_H	1
-/* $Id: l2tp_subr.h,v 1.5 2012/05/08 13:15:11 yasuoka Exp $ */
+/* $Id: l2tp_subr.h,v 1.6 2013/09/20 07:26:23 yasuoka Exp $ */
 
 /**
  * structure of L2TP Attribute Value Pair (AVP) packet header
@@ -94,17 +94,29 @@ short_hash(const void *v, int sz)
 
 /*
  * macro to check AVP size.
- * Prepare 1) char emes[256] for error message, 2) size_check_failed label
+ * Prepare 1) char array `emes' for error message, 2) size_check_failed label
  * before use this macro.
  */
-#define	AVP_SIZE_CHECK(avp, op, exp)					\
-	    if (!((avp)->length op (exp))) {				\
-		    snprintf(emes, sizeof(emes),			\
-			"invalid packet size %s %d" #op "%d)",		\
-			avp_attr_type_string((avp)->attr_type),		\
-			(avp)->length, (exp));				\
-		    goto size_check_failed;				\
-	    }
+#define	AVP_SIZE_CHECK(_avp, _op, _exp)					\
+	do {								\
+		if (!((_avp)->length _op (_exp))) {			\
+			snprintf(emes, sizeof(emes),			\
+			    "invalid packet size %s %d" #_op "%d)",	\
+			    avp_attr_type_string((_avp)->attr_type),	\
+			    (_avp)->length, (_exp));			\
+			goto size_check_failed;				\
+		}							\
+	} while (/* CONSTCOND */0)
+#define AVP_MAXLEN_CHECK(_avp, _maxlen)					\
+	do {								\
+		if ((_avp)->length > (_maxlen) + 6) {			\
+			snprintf(emes, sizeof(emes),			\
+			    "Attribute value is too long %s %d > %d",	\
+			    avp_attr_type_string((_avp)->attr_type),	\
+			    (_avp)->length - 6, (int)(_maxlen));	\
+			goto size_check_failed;				\
+		}							\
+	} while (/* CONSTCOND */0)
 
 #ifdef __cplusplus
 extern "C" {