Package: freeradius / 3.0.12+dfsg-5+deb9u1

fr-gv-301.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
From 931850e5d2f65193520c2d9c9878148c0cdc16a6 Mon Sep 17 00:00:00 2001
From: "Alan T. DeKok" <aland@freeradius.org>
Date: Tue, 27 Jun 2017 21:49:20 -0400
Subject: [PATCH] FR-GV-301 - handle malformed WiMAX attributes

---
 src/lib/radius.c         | 177 ++++++++++++++++++++++++++++++++++-------------
 src/tests/unit/wimax.txt |  12 ++++
 2 files changed, 142 insertions(+), 47 deletions(-)

Index: freeradius-3.0.12+dfsg/src/lib/radius.c
===================================================================
--- freeradius-3.0.12+dfsg.orig/src/lib/radius.c
+++ freeradius-3.0.12+dfsg/src/lib/radius.c
@@ -3203,7 +3203,7 @@ static ssize_t data2vp_extended(TALLOC_C
 	return end - data;
 }
 
-/** Convert a Vendor-Specific WIMAX to vps
+/** Convert a Vendor-Specific WIMAX to VPs
  *
  * @note Called ONLY for Vendor-Specific
  */
@@ -3215,25 +3215,54 @@ static ssize_t data2vp_wimax(TALLOC_CTX
 			     VALUE_PAIR **pvp)
 {
 	ssize_t rcode;
-	size_t fraglen;
-	bool last_frag;
+	size_t wimax_len;
+	bool more;
 	uint8_t *head, *tail;
-	uint8_t const *frag, *end;
+	uint8_t const *attr, *end;
 	DICT_ATTR const *child;
 
-	if (attrlen < 8) return -1;
+	/*
+	 *	data = VID VID VID VID WiMAX-Attr WimAX-Len Continuation ...
+	 */
 
-	if (((size_t) (data[5] + 4)) != attrlen) return -1;
+	/*
+	 *	Not enough room for WiMAX Vendor + Wimax attr + length
+	 *	+ continuation, it's a bad attribute.
+	 */
+	if (attrlen < 8) {
+	raw:		
+		/*
+		 *	It's not a Vendor-Specific, it's unknown...
+		 */
+		child = dict_unknown_afrom_fields(ctx, PW_VENDOR_SPECIFIC, 0);
+		if (!child) {
+			fr_strerror_printf("Internal sanity check %d", __LINE__);
+			return -1;
+		}
+
+		rcode = data2vp(ctx, packet, original, secret, child,
+				data, attrlen, attrlen, pvp);
+		if (rcode < 0) return rcode;
+		return attrlen;
+	}
+
+	if (data[5] < 3) goto raw;		/* WiMAX-Length is too small */
 
 	child = dict_attrbyvalue(data[4], vendor);
-	if (!child) return -1;
+	if (!child) goto raw;
 
+	/*
+	 *	No continued data, just decode the attribute in place.
+	 */
 	if ((data[6] & 0x80) == 0) {
+		if ((data[5] + 4) != attrlen) goto raw; /* WiMAX attribute doesn't fill Vendor-Specific */
+
 		rcode = data2vp(ctx, packet, original, secret, child,
 				data + 7, data[5] - 3, data[5] - 3,
 				pvp);
-		if (rcode < 0) return -1;
-		return 7 + rcode;
+
+		if ((rcode < 0) || (((size_t) rcode + 7) != attrlen)) goto raw; /* didn't decode all of the data */
+		return attrlen;
 	}
 
 	/*
@@ -3242,61 +3271,115 @@ static ssize_t data2vp_wimax(TALLOC_CTX
 	 *	MUST be all of the same VSA, WiMAX, and WiMAX-attr.
 	 *
 	 *	The first fragment doesn't have a RADIUS attribute
-	 *	header, so it needs to be treated a little special.
+	 *	header.
 	 */
-	fraglen = data[5] - 3;
-	frag = data + attrlen;
+	wimax_len = 0;
+	attr = data + 4;
 	end = data + packetlen;
-	last_frag = false;
-
-	while (frag < end) {
-		if (last_frag ||
-		    (frag[0] != PW_VENDOR_SPECIFIC) ||
-		    (frag[1] < 9) ||		       /* too short for wimax */
-		    ((frag + frag[1]) > end) ||		/* overflow */
-		    (memcmp(frag + 2, data, 4) != 0) || /* not wimax */
-		    (frag[6] != data[4]) || /* not the same wimax attr */
-		    ((frag[7] + 6) != frag[1])) { /* doesn't fill the attr */
-			end = frag;
-			break;
-		}
-
-		last_frag = ((frag[8] & 0x80) == 0);
 
-		fraglen += frag[7] - 3;
-		frag += frag[1];
+	while (attr < end) {
+		/*
+		 *	Not enough room for Attribute + length +
+		 *	continuation, it's bad.
+		 */
+		if ((end - attr) < 3) goto raw;
+
+		/*
+		 *	Must have non-zero data in the attribute.
+		 */
+		if (attr[1] <= 3) goto raw;
+
+		/*
+		 *	If the WiMAX attribute overflows the packet,
+		 *	it's bad.
+		 */
+		if ((attr + attr[1]) > end) goto raw;
+
+		/*
+		 *	Check the continuation flag.
+		 */
+		more = ((attr[2] & 0x80) != 0);
+
+		/*
+		 *	Or, there's no more data, in which case we
+		 *	shorten "end" to finish at this attribute.
+		 */
+		if (!more) end = attr + attr[1];
+
+		/*
+		 *	There's more data, but we're at the end of the
+		 *	packet.  The attribute is malformed!
+		 */
+		if (more && ((attr + attr[1]) == end)) goto raw;
+
+		/*
+		 *	Add in the length of the data we need to
+		 *	concatenate together.
+		 */
+		wimax_len += attr[1] - 3;
+
+		/*
+		 *	Go to the next attribute, and stop if there's
+		 *	no more.
+		 */
+		attr += attr[1];
+		if (!more) break;
+
+		/*
+		 *	data = VID VID VID VID WiMAX-Attr WimAX-Len Continuation ...
+		 *
+		 *	attr = Vendor-Specific VSA-Length VID VID VID VID WiMAX-Attr WimAX-Len Continuation ...
+		 *
+		 */
+
+		/*
+		 *	No room for Vendor-Specific + length +
+		 *	Vendor(4) + attr + length + continuation + data
+		 */
+		if ((end - attr) < 9) goto raw;
+
+		if (attr[0] != PW_VENDOR_SPECIFIC) goto raw;
+		if (attr[1] < 9) goto raw;
+		if ((attr + attr[1]) > end) goto raw;
+		if (memcmp(data, attr + 2, 4) != 0) goto raw; /* not WiMAX Vendor ID */
+
+		if (attr[1] != (attr[7] + 6)) goto raw; /* WiMAX attr doesn't exactly fill the VSA */
+
+		if (data[4] != attr[6]) goto raw; /* different WiMAX attribute */
+
+		/*
+		 *	Skip over the Vendor-Specific header, and
+		 *	continue with the WiMAX attributes.
+		 */
+		attr += 6;
 	}
 
-	head = tail = malloc(fraglen);
-	if (!head) return -1;
-
 	/*
-	 *	And again, but faster and looser.
-	 *
-	 *	We copy the first fragment, followed by the rest of
-	 *	the fragments.
+	 *	No data in the WiMAX attribute, make a "raw" one.
 	 */
-	frag = data;
+	if (!wimax_len) goto raw;
 
-	memcpy(tail, frag + 4 + 3, frag[4 + 1] - 3);
-	tail += frag[4 + 1] - 3;
-	frag += attrlen;	/* should be frag[1] - 7 */
+	head = tail = malloc(wimax_len);
+	if (!head) return -1;
 
 	/*
-	 *	frag now points to RADIUS attributes
+	 *	Copy the data over, this time trusting the attribute
+	 *	contents.
 	 */
-	do {
-		memcpy(tail, frag + 2 + 4 + 3, frag[2 + 4 + 1] - 3);
-		tail += frag[2 + 4 + 1] - 3;
-		frag += frag[1];
-	} while (frag < end);
+	attr = data;
+	while (attr < end) {
+		memcpy(tail, attr + 4 + 3, attr[4 + 1] - 3);
+		tail += attr[4 + 1] - 3;
+		attr += 4 + attr[4 + 1]; /* skip VID+WiMax header */
+		attr += 2;		 /* skip Vendor-Specific header */
+	}
 
-	VP_HEXDUMP("wimax fragments", head, fraglen);
+	VP_HEXDUMP("wimax fragments", head, wimax_len);
 
 	rcode = data2vp(ctx, packet, original, secret, child,
-			head, fraglen, fraglen, pvp);
+			head, wimax_len, wimax_len, pvp);
 	free(head);
-	if (rcode < 0) return rcode;
+	if (rcode < 0) goto raw;
 
 	return end - data;
 }
Index: freeradius-3.0.12+dfsg/src/tests/unit/wimax.txt
===================================================================
--- freeradius-3.0.12+dfsg.orig/src/tests/unit/wimax.txt
+++ freeradius-3.0.12+dfsg/src/tests/unit/wimax.txt
@@ -7,6 +7,12 @@ data 1a 0e 00 00 60 b5 01 08 00 01 05 31
 decode -
 data WiMAX-Release = "1.0"
 
+decode 1a 08 00 00 60 b5 01 02
+data Attr-26 = 0x000060b50102
+
+decode 1a 0a 00 00 60 b5 01 04 00 01
+data Attr-26.24757.1 = 0x01
+
 encode WiMAX-Accounting-Capabilities = 1
 data 1a 0c 00 00 60 b5 01 06 00 02 03 01
 
@@ -143,3 +149,9 @@ data 1a ff 00 00 60 b5 01 f9 80 01 ff 45
 
 decode -
 data WiMAX-Release = "EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE", WiMAX-Idle-Mode-Notification-Cap = Supported
+
+#
+#  Continuation is set, but there's no continued data.
+decode 1a 0b 00 00 60 b5 31 05 80 00 00
+data Attr-26 = 0x000060b53105800000
+