diff --git a/SOURCES/freeradius-FR-GV-201-check-input-output-length-in-make_secret.patch b/SOURCES/freeradius-FR-GV-201-check-input-output-length-in-make_secret.patch new file mode 100644 index 0000000..04a8bea --- /dev/null +++ b/SOURCES/freeradius-FR-GV-201-check-input-output-length-in-make_secret.patch @@ -0,0 +1,64 @@ +From 8af41abbd3c0b078425963d88494cfa4e22627e5 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Tue, 4 Jul 2017 10:12:09 -0400 +Subject: [PATCH] FR-GV-201 - check input / output length in make_secret() + +--- + src/lib/radius.c | 17 +++++++++++------ + 1 file changed, 11 insertions(+), 6 deletions(-) + +diff --git a/src/lib/radius.c b/src/lib/radius.c +index b9f0f59c9..62ec11c7a 100644 +--- a/src/lib/radius.c ++++ b/src/lib/radius.c +@@ -542,17 +542,17 @@ static ssize_t rad_recvfrom(int sockfd, RADIUS_PACKET *packet, int flags, + * encrypting passwords to RADIUS. + */ + static void make_secret(uint8_t *digest, uint8_t const *vector, +- char const *secret, uint8_t const *value) ++ char const *secret, uint8_t const *value, size_t length) + { + FR_MD5_CTX context; +- int i; ++ size_t i; + + fr_md5_init(&context); + fr_md5_update(&context, vector, AUTH_VECTOR_LEN); + fr_md5_update(&context, (uint8_t const *) secret, strlen(secret)); + fr_md5_final(digest, &context); + +- for ( i = 0; i < AUTH_VECTOR_LEN; i++ ) { ++ for ( i = 0; i < length; i++ ) { + digest[i] ^= value[i]; + } + } +@@ -1010,8 +1010,8 @@ static ssize_t vp2data_any(RADIUS_PACKET const *packet, + * always fits. + */ + case FLAG_ENCRYPT_ASCEND_SECRET: +- if (len != 16) return 0; +- make_secret(ptr, packet->vector, secret, data); ++ if (len > AUTH_VECTOR_LEN) len = AUTH_VECTOR_LEN; ++ make_secret(ptr, packet->vector, secret, data, len); + len = AUTH_VECTOR_LEN; + break; + +@@ -3769,9 +3769,14 @@ ssize_t data2vp(TALLOC_CTX *ctx, + goto raw; + } else { + uint8_t my_digest[AUTH_VECTOR_LEN]; ++ size_t secret_len; ++ ++ secret_len = datalen; ++ if (secret_len > AUTH_VECTOR_LEN) secret_len = AUTH_VECTOR_LEN; ++ + make_secret(my_digest, + original->vector, +- secret, data); ++ secret, data, secret_len); + memcpy(buffer, my_digest, + AUTH_VECTOR_LEN ); + buffer[AUTH_VECTOR_LEN] = '\0'; +-- +2.13.2 + diff --git a/SOURCES/freeradius-FR-GV-206-decode-option-60-string-not-63-octets-and-.patch b/SOURCES/freeradius-FR-GV-206-decode-option-60-string-not-63-octets-and-.patch new file mode 100644 index 0000000..186b156 --- /dev/null +++ b/SOURCES/freeradius-FR-GV-206-decode-option-60-string-not-63-octets-and-.patch @@ -0,0 +1,28 @@ +From 46aa2d51f137ca34842dd744d17321ed7c11b386 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Mon, 3 Jul 2017 11:36:13 -0400 +Subject: [PATCH] FR-GV-206 - decode option 60 (string) not 63 (octets), and + check length + +--- + src/modules/proto_dhcp/dhcp.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/modules/proto_dhcp/dhcp.c b/src/modules/proto_dhcp/dhcp.c +index 98d87509d..a66a931cb 100644 +--- a/src/modules/proto_dhcp/dhcp.c ++++ b/src/modules/proto_dhcp/dhcp.c +@@ -1097,8 +1097,8 @@ int fr_dhcp_decode(RADIUS_PACKET *packet) + /* + * Vendor is "MSFT 98" + */ +- vp = fr_pair_find_by_num(head, 63, DHCP_MAGIC_VENDOR, TAG_ANY); +- if (vp && (strcmp(vp->vp_strvalue, "MSFT 98") == 0)) { ++ vp = fr_pair_find_by_num(head, 60, DHCP_MAGIC_VENDOR, TAG_ANY); ++ if (vp && (vp->vp_length >= 7) && (memcmp(vp->vp_octets, "MSFT 98", 7) == 0)) { + vp = fr_pair_find_by_num(head, 262, DHCP_MAGIC_VENDOR, TAG_ANY); + + /* +-- +2.13.2 + diff --git a/SOURCES/freeradius-FR-GV-301-handle-malformed-WiMAX-attributes.patch b/SOURCES/freeradius-FR-GV-301-handle-malformed-WiMAX-attributes.patch new file mode 100644 index 0000000..a427291 --- /dev/null +++ b/SOURCES/freeradius-FR-GV-301-handle-malformed-WiMAX-attributes.patch @@ -0,0 +1,270 @@ +From c4685b85f61f28ea0d5cf4e41cb8feaa5193d9dd Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +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(-) + +diff --git a/src/lib/radius.c b/src/lib/radius.c +index 7114e1650..ea4cbf06b 100644 +--- a/src/lib/radius.c ++++ b/src/lib/radius.c +@@ -3229,7 +3229,7 @@ static ssize_t data2vp_extended(TALLOC_CTX *ctx, RADIUS_PACKET *packet, + return end - data; + } + +-/** Convert a Vendor-Specific WIMAX to vps ++/** Convert a Vendor-Specific WIMAX to VPs + * + * @note Called ONLY for Vendor-Specific + */ +@@ -3241,25 +3241,54 @@ static ssize_t data2vp_wimax(TALLOC_CTX *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 ... ++ */ ++ ++ /* ++ * 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 (((size_t) (data[5] + 4)) != attrlen) return -1; ++ 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; + } + + /* +@@ -3268,61 +3297,115 @@ static ssize_t data2vp_wimax(TALLOC_CTX *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; +- } ++ while (attr < end) { ++ /* ++ * Not enough room for Attribute + length + ++ * continuation, it's bad. ++ */ ++ if ((end - attr) < 3) goto raw; + +- last_frag = ((frag[8] & 0x80) == 0); ++ /* ++ * Must have non-zero data in the attribute. ++ */ ++ if (attr[1] <= 3) goto raw; + +- fraglen += frag[7] - 3; +- frag += frag[1]; +- } ++ /* ++ * If the WiMAX attribute overflows the packet, ++ * it's bad. ++ */ ++ if ((attr + attr[1]) > end) goto raw; + +- head = tail = malloc(fraglen); +- if (!head) return -1; ++ /* ++ * 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; ++ } + + /* +- * 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; + } +diff --git a/src/tests/unit/wimax.txt b/src/tests/unit/wimax.txt +index 191b37e25..6e373d59a 100644 +--- a/src/tests/unit/wimax.txt ++++ b/src/tests/unit/wimax.txt +@@ -7,6 +7,12 @@ data 1a 0e 00 00 60 b5 01 08 00 01 05 31 2e 30 + 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 45 45 45 45 45 45 45 45 45 45 45 45 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 ++ +-- +2.13.2 + diff --git a/SOURCES/freeradius-FR-GV-302-do-checks-based-on-pointers-not-on-decoded.patch b/SOURCES/freeradius-FR-GV-302-do-checks-based-on-pointers-not-on-decoded.patch new file mode 100644 index 0000000..2c3c77b --- /dev/null +++ b/SOURCES/freeradius-FR-GV-302-do-checks-based-on-pointers-not-on-decoded.patch @@ -0,0 +1,82 @@ +From 019e35431db17661aa1d74d995fd0315af9a8dbf Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Tue, 27 Jun 2017 21:54:10 -0400 +Subject: [PATCH] FR-GV-302 - do checks based on pointers, not on decoded data + +because decoded data may be empty +--- + src/lib/radius.c | 10 +++++++++- + src/tests/unit/rfc.txt | 12 ++++++++++++ + 2 files changed, 21 insertions(+), 1 deletion(-) + +diff --git a/src/lib/radius.c b/src/lib/radius.c +index ad6b15b46..7114e1650 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 00247940b..d870975e3 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 +-- +2.13.2 + diff --git a/SOURCES/freeradius-FR-GV-303-do-memchr-of-end-p-not-q-p.patch b/SOURCES/freeradius-FR-GV-303-do-memchr-of-end-p-not-q-p.patch new file mode 100644 index 0000000..c0e0613 --- /dev/null +++ b/SOURCES/freeradius-FR-GV-303-do-memchr-of-end-p-not-q-p.patch @@ -0,0 +1,51 @@ +From af6d64cfa4f8ff64da1b5cd6cacd06ae3c095c37 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Mon, 3 Jul 2017 15:37:44 -0400 +Subject: [PATCH] FR-GV-303 - do memchr() of end-p, not q-p + +--- + src/modules/proto_dhcp/dhcp.c | 20 +++++++++----------- + 1 file changed, 9 insertions(+), 11 deletions(-) + +diff --git a/src/modules/proto_dhcp/dhcp.c b/src/modules/proto_dhcp/dhcp.c +index a66a931cb..dbfe81747 100644 +--- a/src/modules/proto_dhcp/dhcp.c ++++ b/src/modules/proto_dhcp/dhcp.c +@@ -774,25 +774,23 @@ static int fr_dhcp_attr2vp(TALLOC_CTX *ctx, VALUE_PAIR **vp_p, uint8_t const *da + * multiple additional VPs + */ + fr_cursor_init(&cursor, vp_p); +- for (;;) { +- q = memchr(p, '\0', q - p); ++ while (p < end) { ++ q = memchr(p, '\0', end - p); + /* Malformed but recoverable */ + if (!q) q = end; + + fr_pair_value_bstrncpy(vp, (char const *)p, q - p); + p = q + 1; + ++ if (p >= end) break; ++ + /* Need another VP for the next round */ +- if (p < end) { +- vp = fr_pair_afrom_da(ctx, vp->da); +- if (!vp) { +- fr_pair_list_free(vp_p); +- return -1; +- } +- fr_cursor_insert(&cursor, vp); +- continue; ++ vp = fr_pair_afrom_da(ctx, vp->da); ++ if (!vp) { ++ fr_pair_list_free(vp_p); ++ return -1; + } +- break; ++ fr_cursor_insert(&cursor, vp); + } + } + break; +-- +2.13.2 + diff --git a/SOURCES/freeradius-FR-GV-304-check-for-option-overflowing-the-packet.patch b/SOURCES/freeradius-FR-GV-304-check-for-option-overflowing-the-packet.patch new file mode 100644 index 0000000..64b68c5 --- /dev/null +++ b/SOURCES/freeradius-FR-GV-304-check-for-option-overflowing-the-packet.patch @@ -0,0 +1,41 @@ +From 4929ae5d13a2750f83cd1a7fd0191b8fca4d32d0 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Mon, 3 Jul 2017 15:42:35 -0400 +Subject: [PATCH] FR-GV-304 - check for option overflowing the packet + +--- + src/modules/proto_dhcp/dhcp.c | 18 ++++++++++++++++++ + 1 file changed, 18 insertions(+) + +diff --git a/src/modules/proto_dhcp/dhcp.c b/src/modules/proto_dhcp/dhcp.c +index dbfe81747..5fd922d03 100644 +--- a/src/modules/proto_dhcp/dhcp.c ++++ b/src/modules/proto_dhcp/dhcp.c +@@ -629,6 +629,24 @@ static int fr_dhcp_decode_suboption(TALLOC_CTX *ctx, VALUE_PAIR **tlv, uint8_t c + uint32_t attr; + + /* ++ * Not enough room for the option header, it's a ++ * bad packet. ++ */ ++ if ((p + 2) > (data + len)) { ++ fr_pair_list_free(&head); ++ return -1; ++ } ++ ++ /* ++ * Not enough room for the option header + data, ++ * it's a bad packet. ++ */ ++ if ((p + 2 + p[1]) > (data + len)) { ++ fr_pair_list_free(&head); ++ return -1; ++ } ++ ++ /* + * The initial OID string looks like: + * .0 + * +-- +2.13.2 + diff --git a/SOURCES/freeradius-make-data2vp_extended-be-more-like-data2vp_wimax.patch b/SOURCES/freeradius-make-data2vp_extended-be-more-like-data2vp_wimax.patch new file mode 100644 index 0000000..b6e4a37 --- /dev/null +++ b/SOURCES/freeradius-make-data2vp_extended-be-more-like-data2vp_wimax.patch @@ -0,0 +1,237 @@ +From 87390bfe6d266ab81312e072759203ccbb261906 Mon Sep 17 00:00:00 2001 +From: "Alan T. DeKok" +Date: Wed, 28 Jun 2017 12:13:03 -0400 +Subject: [PATCH] make data2vp_extended() be more like data2vp_wimax() + +There is no exploit, but making the code simpler is good. +--- + src/lib/radius.c | 170 ++++++++++++++++++++++++++++++-------------- + src/tests/unit/extended.txt | 2 +- + 2 files changed, 116 insertions(+), 56 deletions(-) + +diff --git a/src/lib/radius.c b/src/lib/radius.c +index ea4cbf06b..b9f0f59c9 100644 +--- a/src/lib/radius.c ++++ b/src/lib/radius.c +@@ -3161,70 +3161,143 @@ static ssize_t data2vp_extended(TALLOC_CTX *ctx, RADIUS_PACKET *packet, + VALUE_PAIR **pvp) + { + ssize_t rcode; +- size_t fraglen; ++ size_t ext_len; ++ bool more; + uint8_t *head, *tail; +- uint8_t const *frag, *end; +- uint8_t const *attr; +- int fragments; +- bool last_frag; ++ uint8_t const *attr, *end; ++ DICT_ATTR const *child; ++ ++ /* ++ * data = Ext-Attr Flag ... ++ */ ++ ++ /* ++ * Not enough room for Ext-Attr + Flag + data, it's a bad ++ * attribute. ++ */ ++ if (attrlen < 3) { ++ raw: ++ /* ++ * It's not an Extended attribute, it's unknown... ++ */ ++ child = dict_unknown_afrom_fields(ctx, (da->vendor/ FR_MAX_VENDOR) & 0xff, 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; ++ } ++ ++ /* ++ * No continued data, just decode the attribute in place. ++ */ ++ if ((data[1] & 0x80) == 0) { ++ rcode = data2vp(ctx, packet, original, secret, da, ++ data + 2, attrlen - 2, attrlen - 2, ++ pvp); + +- if (attrlen < 3) return -1; ++ if ((rcode < 0) || (((size_t) rcode + 2) != attrlen)) goto raw; /* didn't decode all of the data */ ++ return attrlen; ++ } ++ ++ /* ++ * It's continued, but there are no subsequent fragments, ++ * it's bad. ++ */ ++ if (attrlen >= packetlen) goto raw; + + /* + * Calculate the length of all of the fragments. For + * now, they MUST be contiguous in the packet, and they +- * MUST be all of the same TYPE and EXTENDED-TYPE ++ * MUST be all of the same Type and Ext-Type ++ * ++ * We skip the first fragment, which doesn't have a ++ * RADIUS attribute header. + */ +- attr = data - 2; +- fraglen = attrlen - 2; +- frag = data + attrlen; ++ ext_len = attrlen - 2; ++ attr = data + attrlen; + end = data + packetlen; +- fragments = 1; +- last_frag = false; +- +- while (frag < end) { +- if (last_frag || +- (frag[0] != attr[0]) || +- (frag[1] < 4) || /* too short for long-extended */ +- (frag[2] != attr[2]) || +- ((frag + frag[1]) > end)) { /* overflow */ +- end = frag; +- break; +- } + +- last_frag = ((frag[3] & 0x80) == 0); ++ while (attr < end) { ++ /* ++ * Not enough room for Attr + length + Ext-Attr ++ * continuation, it's bad. ++ */ ++ if ((end - attr) < 4) goto raw; ++ ++ if (attr[1] < 4) goto raw; ++ ++ /* ++ * If the attribute overflows the packet, it's ++ * bad. ++ */ ++ if ((attr + attr[1]) > end) goto raw; ++ ++ if (attr[0] != ((da->vendor / FR_MAX_VENDOR) & 0xff)) goto raw; /* not the same Extended-Attribute-X */ ++ ++ if (attr[2] != data[0]) goto raw; /* Not the same Ext-Attr */ ++ ++ /* ++ * 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. ++ */ ++ ext_len += attr[1] - 4; + +- fraglen += frag[1] - 4; +- frag += frag[1]; +- fragments++; ++ /* ++ * Go to the next attribute, and stop if there's ++ * no more. ++ */ ++ attr += attr[1]; ++ if (!more) break; + } + +- head = tail = malloc(fraglen); +- if (!head) return -1; ++ if (!ext_len) goto raw; + +- VP_TRACE("Fragments %d, total length %d\n", fragments, (int) fraglen); ++ head = tail = malloc(ext_len); ++ if (!head) goto raw; + + /* +- * And again, but faster and looser. +- * +- * We copy the first fragment, followed by the rest of +- * the fragments. ++ * Copy the data over, this time trusting the attribute ++ * contents. + */ +- frag = attr; ++ attr = data; ++ memcpy(tail, attr + 2, attrlen - 2); ++ tail += attrlen - 2; ++ attr += attrlen; + +- while (fragments > 0) { +- memcpy(tail, frag + 4, frag[1] - 4); +- tail += frag[1] - 4; +- frag += frag[1]; +- fragments--; ++ while (attr < end) { ++ memcpy(tail, attr + 4, attr[1] - 4); ++ tail += attr[1] - 4; ++ attr += attr[1]; /* skip VID+WiMax header */ + } + +- VP_HEXDUMP("long-extended fragments", head, fraglen); ++ VP_HEXDUMP("long-extended fragments", head, ext_len); + + rcode = data2vp(ctx, packet, original, secret, da, +- head, fraglen, fraglen, pvp); ++ head, ext_len, ext_len, pvp); + free(head); +- if (rcode < 0) return rcode; ++ if (rcode < 0) goto raw; + + return end - data; + } +@@ -3827,19 +3900,6 @@ ssize_t data2vp(TALLOC_CTX *ctx, + } + + /* +- * If there no more fragments, then the contents +- * have to be a well-known data type. +- * +- */ +- if ((data[1] & 0x80) == 0) { +- rcode = data2vp(ctx, packet, original, secret, child, +- data + 2, attrlen - 2, attrlen - 2, +- pvp); +- if (rcode < 0) goto raw; +- return 2 + rcode; +- } +- +- /* + * This requires a whole lot more work. + */ + return data2vp_extended(ctx, packet, original, secret, child, +diff --git a/src/tests/unit/extended.txt b/src/tests/unit/extended.txt +index 8b0e3a2c4..9810b194c 100644 +--- a/src/tests/unit/extended.txt ++++ b/src/tests/unit/extended.txt +@@ -80,7 +80,7 @@ data Attr-245.26.1.6 = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + # again, but the second one attr is not an extended attr + decode f5 ff 1a 80 00 00 00 01 06 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ab bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb bb 01 05 62 6f 62 +-data Attr-245.26.1.6 = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, User-Name = "bob" ++data Attr-245 = 0x1a800000000106aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, User-Name = "bob" + + # No data means that the attribute is an "invalid attribute" + decode f5 04 01 00 +-- +2.13.2 + diff --git a/SPECS/freeradius.spec b/SPECS/freeradius.spec index 81d16bc..33c9a72 100644 --- a/SPECS/freeradius.spec +++ b/SPECS/freeradius.spec @@ -1,7 +1,7 @@ Summary: High-performance and highly configurable free RADIUS server Name: freeradius Version: 3.0.13 -Release: 6%{?dist} +Release: 8%{?dist} License: GPLv2+ and LGPLv2+ Group: System Environment/Daemons URL: http://www.freeradius.org/ @@ -29,6 +29,13 @@ Patch5: freeradius-disable-internal-OpenSSL-cache.patch Patch6: freeradius-check-sizeof-packet-.-Found-by-PVS-Studio.patch Patch7: freeradius-parse-port.-Closes-2000.patch Patch8: freeradius-set-S_IWUSER-when-creating-the-file-not-later.patch +Patch9: freeradius-FR-GV-302-do-checks-based-on-pointers-not-on-decoded.patch +Patch10: freeradius-FR-GV-301-handle-malformed-WiMAX-attributes.patch +Patch11: freeradius-make-data2vp_extended-be-more-like-data2vp_wimax.patch +Patch12: freeradius-FR-GV-206-decode-option-60-string-not-63-octets-and-.patch +Patch13: freeradius-FR-GV-303-do-memchr-of-end-p-not-q-p.patch +Patch14: freeradius-FR-GV-304-check-for-option-overflowing-the-packet.patch +Patch15: freeradius-FR-GV-201-check-input-output-length-in-make_secret.patch %global docdir %{?_pkgdocdir}%{!?_pkgdocdir:%{_docdir}/%{name}-%{version}} @@ -193,6 +200,13 @@ This plugin provides the unixODBC support for the FreeRADIUS server project. %patch6 -p1 %patch7 -p1 %patch8 -p1 +%patch9 -p1 +%patch10 -p1 +%patch11 -p1 +%patch12 -p1 +%patch13 -p1 +%patch14 -p1 +%patch15 -p1 %build # Force compile/link options, extra security for network facing daemon @@ -793,6 +807,25 @@ exit 0 %{_libdir}/freeradius/rlm_sql_unixodbc.so %changelog +* Mon Jul 17 2017 Nikolai Kondrashov - 3.0.13-8 +- Avoid misinterpreting zero-size malloc in data2vp_extended() fix. +- Related: Bug#1469414 CVE-2017-10984 freeradius: Out-of-bounds write in + data2vp_wimax() + +* Tue Jul 11 2017 Nikolai Kondrashov - 3.0.13-7 +- Resolves: Bug#1469409 CVE-2017-10978 freeradius: Out-of-bounds read/write due + to improper output buffer size check in make_secret() +- Resolves: Bug#1469413 CVE-2017-10983 freeradius: Out-of-bounds read in + fr_dhcp_decode() when decoding option 63 +- Resolves: Bug#1469414 CVE-2017-10984 freeradius: Out-of-bounds write in + data2vp_wimax() +- Resolves: Bug#1469417 CVE-2017-10985 freeradius: Infinite loop and memory + exhaustion with 'concat' attributes +- Resolves: Bug#1469418 CVE-2017-10986 freeradius: Infinite read in + dhcp_attr2vp() +- Resolves: Bug#1469421 CVE-2017-10987 freeradius: Buffer over-read in + fr_dhcp_decode_suboptions() + * Thu Jun 15 2017 Nikolai Kondrashov - 3.0.13-6 - Avoid race condition when creating session cache file Resolves: Bug#1458746 CVE-2017-9148 freeradius: TLS resumption