Better handle a "known" attribute with invalid length

If we receive an "integer" attribute with length "10", don't
leave the name as "Foo-Bar".  Instead, make it clear that the
attribute is unknown, and print it as "Attr-%d"

diff --git a/src/include/libradius.h b/src/include/libradius.h
index db17422..34ab1ef 100644 (file)
--- a/src/include/libradius.h
+++ b/src/include/libradius.h
@@ -323,6 +323,7 @@ int         rad_vp2attr(const RADIUS_PACKET *packet,
 
 /* valuepair.c */
 VALUE_PAIR     *pairalloc(DICT_ATTR *da);
+VALUE_PAIR     *paircreate_raw(int attr, int type, VALUE_PAIR *vp);
 VALUE_PAIR     *paircreate(int attr, int type);
 void           pairfree(VALUE_PAIR **);
 void            pairbasicfree(VALUE_PAIR *pair);

diff --git a/src/lib/radius.c b/src/lib/radius.c
index 7bd517a..0d45a8b 100644 (file)
--- a/src/lib/radius.c
+++ b/src/lib/radius.c
@@ -2200,7 +2200,7 @@ int rad_verify(RADIUS_PACKET *packet, RADIUS_PACKET *original,
 static VALUE_PAIR *data2vp(const RADIUS_PACKET *packet,
                           const RADIUS_PACKET *original,
                           const char *secret,
-                          UNUSED unsigned int attribute, size_t length,
+                          unsigned int attribute, size_t length,
                           const uint8_t *data, VALUE_PAIR *vp)
 {
        int offset = 0;
@@ -2432,16 +2432,33 @@ static VALUE_PAIR *data2vp(const RADIUS_PACKET *packet,
 
        default:
        raw:
-               vp->type = PW_TYPE_OCTETS;
-               vp->length = length;
-               memcpy(vp->vp_octets, data, length);
-
-
                /*
-                *      Ensure there's no encryption or tag stuff,
-                *      we just pass the attribute as-is.
+                *      Change the name to show the user that the
+                *      attribute is not of the correct format.
                 */
-               memset(&vp->flags, 0, sizeof(vp->flags));
+               {
+                       VALUE_PAIR *vp2;
+
+                       vp2 = pairalloc(NULL);
+                       if (!vp2) {
+                               pairfree(&vp);
+                               return NULL;
+                       }
+                       pairfree(&vp);
+                       vp = vp2;
+
+                       /*
+                        *      This sets "vp->flags" appropriately,
+                        *      and vp->type.
+                        */
+                       if (!paircreate_raw(attribute, PW_TYPE_OCTETS, vp)) {
+                               return NULL;
+                       }
+
+                       vp->length = length;
+                       memcpy(vp->vp_octets, data, length);
+               }
+               break;
        }
 
        return vp;
@@ -2659,8 +2676,9 @@ static VALUE_PAIR *rad_continuation2vp(const RADIUS_PACKET *packet,
                        goto not_well_formed;
                }
 
-               if (!data2vp(packet, original, secret,
-                            ptr[0], ptr[1] - 2, ptr + 2, vp)) {
+               vp = data2vp(packet, original, secret,
+                            ptr[0], ptr[1] - 2, ptr + 2, vp);
+               if (!vp) {      /* called frees vp */
                        pairfree(&head);
                        goto not_well_formed;
                }

diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c
index d4e65c9..c692256 100644 (file)
--- a/src/lib/valuepair.c
+++ b/src/lib/valuepair.c
@@ -135,6 +135,33 @@ VALUE_PAIR *pairalloc(DICT_ATTR *da)
 }
 
 
+VALUE_PAIR *paircreate_raw(int attr, int type, VALUE_PAIR *vp)
+{
+       char *p = (char *) (vp + 1);
+
+       if (!vp->flags.unknown_attr) {
+               pairfree(&vp);
+               return NULL;
+       }
+
+       vp->vendor = VENDOR(attr);
+       vp->attribute = attr;
+       vp->operator = T_OP_EQ;
+       vp->name = p;
+       vp->type = type;
+       vp->length = 0;
+       memset(&vp->flags, 0, sizeof(vp->flags));
+       vp->flags.unknown_attr = 1;
+       
+       if (!vp_print_name(p, FR_VP_NAME_LEN, vp->attribute)) {
+               free(vp);
+               return NULL;
+       }
+
+       return vp;
+}
+
+
 /*
  *     Create a new valuepair.
  */
@@ -153,19 +180,7 @@ VALUE_PAIR *paircreate(int attr, int type)
        /*
         *      It isn't in the dictionary: update the name.
         */
-       if (!da) {
-               char *p = (char *) (vp + 1);
-               
-               vp->vendor = VENDOR(attr);
-               vp->attribute = attr;
-               vp->name = p;
-               vp->type = type; /* be forgiving */
-
-               if (!vp_print_name(p, FR_VP_NAME_LEN, vp->attribute)) {
-                       free(vp);
-                       return NULL;
-               }
-       }
+       if (!da) return paircreate_raw(attr, type, vp);
 
        return vp;
 }
@@ -320,6 +335,12 @@ VALUE_PAIR *paircopyvp(const VALUE_PAIR *vp)
                return NULL;
        }
        memcpy(n, vp, sizeof(*n) + name_len);
+
+       /*
+        *      Reset the name field to point to the NEW attribute,
+        *      rather than to the OLD one.
+        */
+       if (vp->flags.unknown_attr) n->name = (char *) (n + 1);
        n->next = NULL;
 
        if ((n->type == PW_TYPE_TLV) &&