]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
just allocate a vendor, and let the rest of the code figure it out
authorAlan T. DeKok <aland@freeradius.org>
Wed, 21 Dec 2022 15:35:29 +0000 (10:35 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 21 Dec 2022 17:49:19 +0000 (12:49 -0500)
and fix unknown child allocation, which was using the wrong parent.

src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c
src/protocols/radius/decode.c

index 17a9f309db03a76440a72a00aafdf73a4023a3d8..d06286031d1997aadb04ce09be08eba5186440dc 100644 (file)
@@ -153,6 +153,7 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur
        fr_dict_t const         *dict_radius;
        fr_dict_attr_t const    *attr_radius;
        fr_dict_attr_t const    *da;
+       TALLOC_CTX              *tmp_ctx = NULL;
 
        dict_radius = fr_dict_by_protocol_name("radius");
        fr_assert(dict_radius != NULL);
@@ -169,6 +170,7 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur
                        fr_strerror_printf("Malformed diameter attribute at offset %zu.  Needed at least 8 bytes, got %zu bytes",
                                           p - data, end - p);
                error:
+                       talloc_free(tmp_ctx);
                        fr_dcursor_free_list(cursor);
                        return -1;
                }
@@ -220,8 +222,14 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur
                                        goto error;
                                }
 
-                               MEM(da = fr_dict_unknown_afrom_fields(vp, attr_vendor_specific, vendor, attr));
-                               goto reinit;
+                               if (!tmp_ctx) {
+                                       fr_dict_attr_t *n;
+
+                                       MEM(our_parent = n = fr_dict_unknown_vendor_afrom_num(ctx, parent, vendor));
+                                       tmp_ctx = n;
+                               } else {
+                                       MEM(our_parent = fr_dict_unknown_vendor_afrom_num(tmp_ctx, parent, vendor));
+                               }
                        }
                } else {
                        our_parent = attr_radius;
@@ -241,7 +249,7 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur
                                goto error;
                        }
 
-                       MEM(da = fr_dict_unknown_attr_afrom_num(vp, parent, attr));
+                       MEM(da = fr_dict_unknown_attr_afrom_num(vp, our_parent, attr));
 
                reinit:
                        if (fr_pair_reinit_from_da(NULL, vp, da) < 0) {
@@ -326,6 +334,7 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur
        /*
         *      We got this far.  It looks OK.
         */
+       talloc_free(tmp_ctx);
        return p - data;
 }
 
index f130562e129036bc29a2a914f664efe30ff26132..7b0048bb1e1a9d143834811756a67fa03206b85d 100644 (file)
@@ -1663,26 +1663,13 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        vendor_child = fr_dict_attr_child_by_num(parent, vendor);
                        if (!vendor_child) {
                                /*
-                                *      If there's no child, it means the vendor is unknown
-                                *      which means the child attribute is unknown too.
-                                *
-                                *      fr_dict_unknown_afrom_fields will do the right thing
-                                *      and create both an unknown vendor and an unknown
-                                *      attr.
-                                *
-                                *      This can be used later by the encoder to rebuild
-                                *      the attribute header.
+                                *      If there's no child, it means the vendor is unknown.  Create a
+                                *      temporary vendor in the packet_ctx.  This will be cleaned up when the
+                                *      decoder exists, which is fine.  Because any unknown attributes which
+                                *      depend on it will copy the entire hierarchy.
                                 */
-                               vendor_child = fr_dict_unknown_afrom_fields(packet_ctx->tmp_ctx, parent, vendor, p[4]);
-                               if (!vendor_child) {
-                                       fr_strerror_printf_push("decoder failed creating unknown attribute in %s",
-                                                               parent->name);
-                                       return -1;
-                               }
-                               parent = vendor_child;
-                               p += 5;
-                               data_len -= 5;
-                               break;
+                               vendor_child = fr_dict_unknown_vendor_afrom_num(packet_ctx->tmp_ctx, parent, vendor);
+                               if (!vendor_child) return PAIR_DECODE_OOM;
                        }
 
                        child = fr_dict_attr_child_by_num(vendor_child, p[4]);
@@ -1690,7 +1677,7 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                /*
                                 *      Vendor exists but child didn't, create an unknown child.
                                 */
-                               child = fr_dict_unknown_attr_afrom_num(packet_ctx->tmp_ctx, parent, p[4]);
+                               child = fr_dict_unknown_attr_afrom_num(packet_ctx->tmp_ctx, vendor_child, p[4]);
                                if (!child) {
                                        fr_strerror_printf_push("decoder failed creating unknown attribute in %s",
                                                                parent->name);