|
 |
9700c1 |
From: Antonio Torres <antorres@redhat.com>
|
|
|
9700c1 |
Date: Fri, 09 Dec 2022
|
|
|
9700c1 |
Subject: Fix crash on unknown option in EAP-SIM
|
|
|
9700c1 |
|
|
|
9700c1 |
When an EAP-SIM supplicant sends an unknown SIM option, the server will try to
|
|
|
9700c1 |
look that option up in the internal dictionaries. This lookup will fail, but the
|
|
|
9700c1 |
SIM code will not check for that failure. Instead, it will dereference a NULL
|
|
|
9700c1 |
pointer, and cause the server to crash.
|
|
|
9700c1 |
|
|
|
9700c1 |
Backport of:
|
|
|
9700c1 |
https://github.com/FreeRADIUS/freeradius-server/commit/f1cdbb33ec61c4a64a
|
|
|
9700c1 |
https://github.com/FreeRADIUS/freeradius-server/commit/71128cac3ee236a88a05cc7bddd43e43a88a3089
|
|
|
9700c1 |
|
|
|
9700c1 |
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151705
|
|
|
9700c1 |
Signed-off-by: Antonio Torres <antorres@redhat.com>
|
|
|
9700c1 |
---
|
|
|
9700c1 |
diff --git a/src/modules/rlm_eap/libeap/eapsimlib.c b/src/modules/rlm_eap/libeap/eapsimlib.c
|
|
|
9700c1 |
index cf1e8a7dd92..e438a844eab 100644
|
|
|
9700c1 |
--- a/src/modules/rlm_eap/libeap/eapsimlib.c
|
|
|
9700c1 |
+++ b/src/modules/rlm_eap/libeap/eapsimlib.c
|
|
|
9700c1 |
@@ -307,42 +307,77 @@ int unmap_eapsim_basictypes(RADIUS_PACKET *r,
|
|
|
9700c1 |
newvp->vp_length = 1;
|
|
|
9700c1 |
fr_pair_add(&(r->vps), newvp);
|
|
|
9700c1 |
|
|
|
9700c1 |
+ /*
|
|
|
9700c1 |
+ * EAP-SIM has a 1 octet of subtype, and 2 octets
|
|
|
9700c1 |
+ * reserved.
|
|
|
9700c1 |
+ */
|
|
|
9700c1 |
attr += 3;
|
|
|
9700c1 |
attrlen -= 3;
|
|
|
9700c1 |
|
|
|
9700c1 |
- /* now, loop processing each attribute that we find */
|
|
|
9700c1 |
- while(attrlen > 0) {
|
|
|
9700c1 |
+ /*
|
|
|
9700c1 |
+ * Loop over each attribute. The format is:
|
|
|
9700c1 |
+ *
|
|
|
9700c1 |
+ * 1 octet of type
|
|
|
9700c1 |
+ * 1 octet of length (value 1..255)
|
|
|
9700c1 |
+ * ((4 * length) - 2) octets of data.
|
|
|
9700c1 |
+ */
|
|
|
9700c1 |
+ while (attrlen > 0) {
|
|
|
9700c1 |
uint8_t *p;
|
|
|
9700c1 |
|
|
|
9700c1 |
- if(attrlen < 2) {
|
|
|
9700c1 |
+ if (attrlen < 2) {
|
|
|
9700c1 |
fr_strerror_printf("EAP-Sim attribute %d too short: %d < 2", es_attribute_count, attrlen);
|
|
|
9700c1 |
return 0;
|
|
|
9700c1 |
}
|
|
|
9700c1 |
|
|
|
9700c1 |
+ if (!attr[1]) {
|
|
|
9700c1 |
+ fr_strerror_printf("EAP-Sim attribute %d (no.%d) has no data", attr[0],
|
|
|
9700c1 |
+ es_attribute_count);
|
|
|
9700c1 |
+ return 0;
|
|
|
9700c1 |
+ }
|
|
|
9700c1 |
+
|
|
|
9700c1 |
eapsim_attribute = attr[0];
|
|
|
9700c1 |
eapsim_len = attr[1] * 4;
|
|
|
9700c1 |
|
|
|
9700c1 |
+ /*
|
|
|
9700c1 |
+ * The length includes the 2-byte header.
|
|
|
9700c1 |
+ */
|
|
|
9700c1 |
if (eapsim_len > attrlen) {
|
|
|
9700c1 |
fr_strerror_printf("EAP-Sim attribute %d (no.%d) has length longer than data (%d > %d)",
|
|
|
9700c1 |
eapsim_attribute, es_attribute_count, eapsim_len, attrlen);
|
|
|
9700c1 |
return 0;
|
|
|
9700c1 |
}
|
|
|
9700c1 |
|
|
|
9700c1 |
- if(eapsim_len > MAX_STRING_LEN) {
|
|
|
9700c1 |
- eapsim_len = MAX_STRING_LEN;
|
|
|
9700c1 |
- }
|
|
|
9700c1 |
- if (eapsim_len < 2) {
|
|
|
9700c1 |
- fr_strerror_printf("EAP-Sim attribute %d (no.%d) has length too small", eapsim_attribute,
|
|
|
9700c1 |
- es_attribute_count);
|
|
|
9700c1 |
- return 0;
|
|
|
9700c1 |
- }
|
|
|
9700c1 |
+ newvp = fr_pair_afrom_num(r, eapsim_attribute + PW_EAP_SIM_BASE, 0);
|
|
|
9700c1 |
+ if (!newvp) {
|
|
|
9700c1 |
+ /*
|
|
|
9700c1 |
+ * RFC 4186 Section 8.1 says 0..127 are
|
|
|
9700c1 |
+ * "non-skippable". If one such
|
|
|
9700c1 |
+ * attribute is found and we don't
|
|
|
9700c1 |
+ * understand it, the server has to send:
|
|
|
9700c1 |
+ *
|
|
|
9700c1 |
+ * EAP-Request/SIM/Notification packet with an
|
|
|
9700c1 |
+ * (AT_NOTIFICATION code, which implies general failure ("General
|
|
|
9700c1 |
+ * failure after authentication" (0), or "General failure" (16384),
|
|
|
9700c1 |
+ * depending on the phase of the exchange), which terminates the
|
|
|
9700c1 |
+ * authentication exchange.
|
|
|
9700c1 |
+ */
|
|
|
9700c1 |
+ if (eapsim_attribute <= 127) {
|
|
|
9700c1 |
+ fr_strerror_printf("Unknown mandatory attribute %d, failing",
|
|
|
9700c1 |
+ eapsim_attribute);
|
|
|
9700c1 |
+ return 0;
|
|
|
9700c1 |
+ }
|
|
|
9700c1 |
|
|
|
9700c1 |
- newvp = fr_pair_afrom_num(r, eapsim_attribute+PW_EAP_SIM_BASE, 0);
|
|
|
9700c1 |
- newvp->vp_length = eapsim_len-2;
|
|
|
9700c1 |
- newvp->vp_octets = p = talloc_array(newvp, uint8_t, newvp->vp_length);
|
|
|
9700c1 |
- memcpy(p, &attr[2], eapsim_len-2);
|
|
|
9700c1 |
- fr_pair_add(&(r->vps), newvp);
|
|
|
9700c1 |
- newvp = NULL;
|
|
|
9700c1 |
+ } else {
|
|
|
9700c1 |
+ /*
|
|
|
9700c1 |
+ * It's known, ccount for header, and
|
|
|
9700c1 |
+ * copy the value over.
|
|
|
9700c1 |
+ */
|
|
|
9700c1 |
+ newvp->vp_length = eapsim_len - 2;
|
|
|
9700c1 |
+
|
|
|
9700c1 |
+ newvp->vp_octets = p = talloc_array(newvp, uint8_t, newvp->vp_length);
|
|
|
9700c1 |
+ memcpy(p, &attr[2], newvp->vp_length);
|
|
|
9700c1 |
+ fr_pair_add(&(r->vps), newvp);
|
|
|
9700c1 |
+ }
|
|
|
9700c1 |
|
|
|
9700c1 |
/* advance pointers, decrement length */
|
|
|
9700c1 |
attr += eapsim_len;
|