From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 28 Jun 2017 01:54:10 +0000 (-0400)
Subject: FR-GV-302 - do checks based on pointers, not on decoded data
X-Git-Tag: release_3_0_15~13
X-Git-Url: http://www.project-moonshot.org/gitweb/?a=commitdiff_plain;ds=sidebyside;h=6726c16549b131ed39f6f8886cdf5d9d922a9a97;p=freeradius.git

FR-GV-302 - do checks based on pointers, not on decoded data

because decoded data may be empty
---

diff --git a/src/lib/radius.c b/src/lib/radius.c
index 180b006..81a8066 100644
--- a/src/lib/radius.c
+++ b/src/lib/radius.c
@@ -2952,16 +2952,23 @@ static ssize_t data2vp_concat(TALLOC_CTX *ctx,
 	 *	don't care about walking off of the end of it.
 	 */
 	while (ptr < end) {
+		if (ptr[1] < 2) return -1;
+		if ((ptr + ptr[1]) > end) return -1;
+
 		total += ptr[1] - 2;
 
 		ptr += ptr[1];
 
+		if (ptr == end) break;
+
 		/*
 		 *	Attributes MUST be consecutive.
 		 */
 		if (ptr[0] != attr) break;
 	}
 
+	end = ptr;
+
 	vp = fr_pair_afrom_da(ctx, da);
 	if (!vp) return -1;
 
@@ -2974,7 +2981,7 @@ static ssize_t data2vp_concat(TALLOC_CTX *ctx,
 
 	total = 0;
 	ptr = start;
-	while (total < vp->vp_length) {
+	while (ptr < end) {
 		memcpy(p, ptr + 2, ptr[1] - 2);
 		p += ptr[1] - 2;
 		total += ptr[1] - 2;
@@ -2982,6 +2989,7 @@ static ssize_t data2vp_concat(TALLOC_CTX *ctx,
 	}
 
 	*pvp = vp;
+
 	return ptr - start;
 }
 
diff --git a/src/tests/unit/rfc.txt b/src/tests/unit/rfc.txt
index 0024794..d870975 100644
--- a/src/tests/unit/rfc.txt
+++ b/src/tests/unit/rfc.txt
@@ -178,6 +178,18 @@ data Failed to parse IPv4 address string "256/8"
 attribute PMIP6-Home-IPv4-HoA = bob/8
 data Failed to parse IPv4 address string "bob/8"
 
+#
+#  A "concat" attribute, with no data
+#
+decode 89 02
+data PKM-SS-Cert = 0x
+
+#
+#  Or with weirdly formatted data
+#
+decode 89 03 ff 89 02 89 03 fe
+data PKM-SS-Cert = 0xfffe
+
 $INCLUDE tunnel.txt
 $INCLUDE errors.txt
 $INCLUDE extended.txt