Fix memory leak in radius_get_vp

diff --git a/src/include/radiusd.h b/src/include/radiusd.h
index 707c35708221974c6b0f713cbc0d41fa1727bcab..9a7e414f8d2d0d9f7bc1d671e3485a2f6db00e62 100644 (file)
--- a/src/include/radiusd.h
+++ b/src/include/radiusd.h
@@ -740,8 +740,8 @@ typedef struct value_pair_tmpl {
  *  Value pair map
  */
 typedef struct value_pair_map {
-       VALUE_PAIR_TMPL         dst;
-       VALUE_PAIR_TMPL         src;
+       VALUE_PAIR_TMPL         *dst;
+       VALUE_PAIR_TMPL         *src;
        
        FR_TOKEN                op_token;
        
@@ -751,10 +751,13 @@ typedef struct value_pair_map {
 typedef VALUE_PAIR *(*radius_tmpl_getvalue_t)(REQUEST *request,
                                              const VALUE_PAIR_TMPL *src,
                                              void *ctx);
-
-int radius_attr2tmpl(const char *name, VALUE_PAIR_TMPL *vpt,
-                    request_refs_t request_def, pair_lists_t list_def);
-int radius_str2tmpl(const char *name, VALUE_PAIR_TMPL *vpt);
+void radius_tmplfree(VALUE_PAIR_TMPL **tmpl);
+int radius_parse_attr(const char *name, VALUE_PAIR_TMPL *vpt,
+                     request_refs_t request_def,
+                     pair_lists_t list_def);
+VALUE_PAIR_TMPL *radius_attr2tmpl(const char *name, request_refs_t request_def,
+                                 pair_lists_t list_def);
+VALUE_PAIR_TMPL *radius_str2tmpl(const char *name);
 VALUE_PAIR_MAP *radius_cp2map(CONF_PAIR *cp, request_refs_t request_def,
                              pair_lists_t list_def);
 int radius_map2request(REQUEST *request, const VALUE_PAIR_MAP *map,

diff --git a/src/main/valuepair.c b/src/main/valuepair.c
index a8ff59a46d1adab961cd4ae25c0a43b70da2b54f..02ab3f04e9d3028a5eb50304f76268f68c282dcd 100644 (file)
--- a/src/main/valuepair.c
+++ b/src/main/valuepair.c
@@ -1053,28 +1053,49 @@ request_refs_t radius_request_name(const char **name, request_refs_t unknown)
        return request;
 }
 
+/** Release memory allocated to value pair template.
+ *
+ * @param[in+out] tmpl to free.
+ */
+void radius_tmplfree(VALUE_PAIR_TMPL **tmpl)
+{
+       if (*tmpl == NULL) return;
+       
+       if ((*tmpl)->name) {
+               free((*tmpl)->name);
+       }
+       
+       free(*tmpl);
+       
+       *tmpl = NULL;
+}
+
 /** Parse qualifiers to convert attrname into a VALUE_PAIR_TMPL.
  *
  * VPTs are used in various places where we need to pre-parse configuration 
  * sections into attribute mappings.
  *
+ * Note: name field is just a copy of the input pointer, if you know that
+ * string might be freed before you're done with the vpt use radius_attr2tmpl
+ * instead.
+ * 
  * @param[in] name attribute name including qualifiers.
  * @param[out] vpt to modify.
  * @param[in] request_def The default request to insert unqualified 
  *     attributes into.
  * @param[in] list_def The default list to insert unqualified attributes into.
- * @return -1 if either the attribute or qualifier were invalid, else 0
+ * @return -1 on error or 0 on success.
  */
-int radius_attr2tmpl(const char *name, VALUE_PAIR_TMPL *vpt,
-                    request_refs_t request_def, pair_lists_t list_def)
+int radius_parse_attr(const char *name, VALUE_PAIR_TMPL *vpt,
+                     request_refs_t request_def,
+                     pair_lists_t list_def)
 {
        char buffer[128];
        const char *p;
        size_t len;
-       
-       vpt->name = strdup(name);
-       
-       p = vpt->name;
+
+       vpt->name = name;
+       p = name;
        
        vpt->request = radius_request_name(&p, request_def);
        len = p - name;
@@ -1083,7 +1104,8 @@ int radius_attr2tmpl(const char *name, VALUE_PAIR_TMPL *vpt,
                        len + 1 : sizeof(buffer));
                
                radlog(L_ERR, "Invalid request qualifier \"%s\"", buffer);
-               return -1;      
+               
+               return -1;
        }
        name += len;
        
@@ -1094,29 +1116,66 @@ int radius_attr2tmpl(const char *name, VALUE_PAIR_TMPL *vpt,
                        len + 1 : sizeof(buffer));
                                
                radlog(L_ERR, "Invalid list qualifier \"%s\"", buffer);
+               
                return -1;
        }
        
        vpt->da = dict_attrbyname(p);
        if (!vpt->da) {
                radlog(L_ERR, "Attribute \"%s\" unknown", p);
+               
                return -1;
        }
        
        return 0;
 }
 
+/** Parse qualifiers to convert attrname into a VALUE_PAIR_TMPL.
+ *
+ * VPTs are used in various places where we need to pre-parse configuration 
+ * sections into attribute mappings.
+ *
+ * @param[in] name attribute name including qualifiers.
+ * @param[in] request_def The default request to insert unqualified 
+ *     attributes into.
+ * @param[in] list_def The default list to insert unqualified attributes into.
+ * @return pointer to a VALUE_PAIR_TMPL struct (must be freed with 
+ *     radius_tmplfree) or NULL on error.
+ */
+VALUE_PAIR_TMPL *radius_attr2tmpl(const char *name,
+                                 request_refs_t request_def,
+                                 pair_lists_t list_def)
+{
+       VALUE_PAIR_TMPL *vpt;
+       const char *copy = strdup(name);
+       
+       vpt = rad_malloc(sizeof(VALUE_PAIR_TMPL));
+       memset(vpt, 0, sizeof(VALUE_PAIR_TMPL));
+       
+       if (radius_parse_attr(copy, vpt, request_def, list_def) < 0) {
+               radius_tmplfree(&vpt);
+               return NULL;
+       }
+       
+       return vpt;
+}
+
 /** Convert module specific attribute id to VALUE_PAIR_TMPL.
  *
  * @param[in] name string to convert.
  * @param[out] vpt to modify.
  * @return 0
  */
-int radius_str2tmpl(const char *name, VALUE_PAIR_TMPL *vpt)
+VALUE_PAIR_TMPL *radius_str2tmpl(const char *name)
 {
+       VALUE_PAIR_TMPL *vpt;
+       
+       vpt = rad_malloc(sizeof(VALUE_PAIR_TMPL));
+       memset(vpt, 0, sizeof(VALUE_PAIR_TMPL));
+
        vpt->name = strdup(name);
        
-       return 0;
+       return vpt;
 }
 
 /** Release memory used by a map linked list.
@@ -1134,8 +1193,8 @@ void radius_mapfree(VALUE_PAIR_MAP **map)
        while (vpm != NULL) {
                next = vpm->next;
                
-               if (vpm->src.name) free(vpm->src.name);
-               if (vpm->dst.name) free(vpm->dst.name);
+               radius_tmplfree(&((*map)->dst));
+               radius_tmplfree(&((*map)->src));
                
                free(vpm);
                vpm = next;
@@ -1172,7 +1231,8 @@ VALUE_PAIR_MAP *radius_cp2map(CONF_PAIR *cp, request_refs_t request_def,
      
        attr = cf_pair_attr(cp);
        
-       if (radius_attr2tmpl(attr, &map->dst, list_def, request_def) < 0){
+       map->dst = radius_attr2tmpl(attr, request_def, list_def);
+       if (!map->dst){
                goto error;
        }
 
@@ -1183,7 +1243,8 @@ VALUE_PAIR_MAP *radius_cp2map(CONF_PAIR *cp, request_refs_t request_def,
                goto error;
        }
        
-       if (radius_str2tmpl(value, &map->src) < 0) {
+       map->src = radius_str2tmpl(value);
+       if (!map->src) {
                goto error;
        }
        
@@ -1200,11 +1261,11 @@ VALUE_PAIR_MAP *radius_cp2map(CONF_PAIR *cp, request_refs_t request_def,
        {
                case T_BARE_WORD:
                case T_SINGLE_QUOTED_STRING:
-                       map->src.do_xlat = FALSE;
+                       map->src->do_xlat = FALSE;
                break;
                case T_BACK_QUOTED_STRING:
                case T_DOUBLE_QUOTED_STRING:
-                       map->src.do_xlat = TRUE;                
+                       map->src->do_xlat = TRUE;               
                break;
                default:
                        rad_assert(0);
@@ -1240,24 +1301,24 @@ int radius_map2request(REQUEST *request, const VALUE_PAIR_MAP *map,
        VALUE_PAIR **list, *vp, *head;
        char buffer[MAX_STRING_LEN];
        
-       if (!radius_request(&request, map->dst.request)) {
+       if (radius_request(&request, map->dst->request) < 0) {
                RDEBUG("WARNING: Request in mapping \"%s\" -> \"%s\" "
                       "invalid in this context, skipping!",
-                      map->src.name, map->dst.name);
+                      map->src->name, map->dst->name);
                
                return -1;
        }
        
-       list = radius_list(request, map->dst.list);
+       list = radius_list(request, map->dst->list);
        if (!list) {
                RDEBUG("WARNING: List in mapping \"%s\" -> \"%s\" "
                       "invalid in this context, skipping!",
-                      map->src.name, map->dst.name);
+                      map->src->name, map->dst->name);
                       
                return -1;
        }
        
-       head = func(request, &map->dst, ctx);
+       head = func(request, map->dst, ctx);
        if (head == NULL) {
                return -1;
        }
@@ -1268,16 +1329,16 @@ int radius_map2request(REQUEST *request, const VALUE_PAIR_MAP *map,
                if (debug_flag) {
                        vp_prints_value(buffer, sizeof(buffer), vp, 1);
                        
-                       RDEBUG("\t%s %s %s (%s)", map->dst.name,
+                       RDEBUG("\t%s %s %s (%s)", map->dst->name,
                               fr_int2str(fr_tokens, vp->operator, "¿unknown?"), 
-                              buffer, src ? src : map->src.name);
+                              buffer, src ? src : map->src->name);
                }
        }
        
        /*
         *      Use pairmove so the operator is respected
         */
-       radius_pairmove(request, list, vp);
+       radius_pairmove(request, list, head);
        pairfree(&vp); /* Free the VP if for some reason it wasn't moved */
        
        return 0;
@@ -1298,7 +1359,8 @@ int radius_get_vp(REQUEST *request, const char *name, VALUE_PAIR **vp_p)
 
        *vp_p = NULL;
        
-       if (radius_attr2tmpl(name, &vpt, REQUEST_CURRENT, PAIR_LIST_REQUEST) < 0) {
+       if (radius_parse_attr(name, &vpt, REQUEST_CURRENT,
+           PAIR_LIST_REQUEST) < 0) {
                return -1;
        }
        

diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c
index 05325f7780659ffb45a8bcc719fc0235e24c55e6..54a2464c17f8c5f8121eb46bf1c573839e96e930 100644 (file)
--- a/src/modules/rlm_ldap/rlm_ldap.c
+++ b/src/modules/rlm_ldap/rlm_ldap.c
@@ -1605,7 +1605,7 @@ static void xlat_attrsfree(const xlat_attrs_t *expanded)
                
                if (!name) return;
                
-               if (map->src.do_xlat) {
+               if (map->src->do_xlat) {
                        free(name);
                }
        }
@@ -1623,14 +1623,14 @@ static int xlat_attrs(REQUEST *request, const VALUE_PAIR_MAP *maps,
 
        for (map = maps; map != NULL; map = map->next)
        {
-               if (map->src.do_xlat) {
+               if (map->src->do_xlat) {
                        buffer = rad_malloc(MAX_ATTR_STR_LEN);
                        len = radius_xlat(buffer, MAX_ATTR_STR_LEN,
-                                         map->src.name, request, NULL, NULL);
+                                         map->src->name, request, NULL, NULL);
                                          
                        if (!len) {
                                RDEBUG("Expansion of LDAP attribute "
-                                      "\"%s\" failed", map->src.name);
+                                      "\"%s\" failed", map->src->name);
                                       
                                expanded->attrs[total] = NULL;
                                
@@ -1641,7 +1641,7 @@ static int xlat_attrs(REQUEST *request, const VALUE_PAIR_MAP *maps,
                        
                        expanded->attrs[total++] = buffer;
                } else {
-                       expanded->attrs[total++] = map->src.name;
+                       expanded->attrs[total++] = map->src->name;
                }
        }
        
@@ -1676,8 +1676,8 @@ static void do_attrmap(UNUSED ldap_instance *inst, REQUEST *request,
                
                result.values = ldap_get_values(handle, entry, name);
                if (!result.values) {
-                       DEBUG2("Attribute \"%s\" not found in LDAP object",
-                              name);
+                       RDEBUG2("Attribute \"%s\" not found in LDAP object",
+                               name);
                                
                        goto next;
                }