From: Jennifer Richards <jennifer@painless-security.com>
Date: Wed, 2 May 2018 14:31:03 +0000 (-0400)
Subject: Fix memory leak when setting next hop for community inforecs
X-Git-Tag: 3.4.0~1^2~33^2~3
X-Git-Url: http://www.project-moonshot.org/gitweb/?p=trust_router.git;a=commitdiff_plain;h=fd58b52dff0f0cbbd52542f1f89c2e3d50f7c58a

Fix memory leak when setting next hop for community inforecs

  * Return TRP_UNSUPPORTED when setting next hop on an inforec that
    does not accept it (i.e., community inforecs)
  * Free the next hop TR_NAME if it was not stored
---

diff --git a/trp/trp_upd.c b/trp/trp_upd.c
index e1057f1..9c520e3 100644
--- a/trp/trp_upd.c
+++ b/trp/trp_upd.c
@@ -260,19 +260,31 @@ TR_NAME *trp_inforec_dup_next_hop(TRP_INFOREC *rec)
   return tr_dup_name(trp_inforec_get_next_hop(rec));
 }
 
+/**
+ * Set the next hop for the inforec
+ *
+ * Returns TRP_SUCCESS if it set the next hop value for the inforec.
+ * Returns TRP_UNSUPPORTED if the inforec does not have a next hop record but
+ * otherwise nothing went wrong.
+ * Returns TRP_ERROR or another error if there was a failure.
+ *
+ * @param rec
+ * @param next_hop
+ * @return TRP_SUCCESS if the value was set, TRP_UNSUPPORTED if the inforec does not support next hop, or an error code on failure
+ */
 TRP_RC trp_inforec_set_next_hop(TRP_INFOREC *rec, TR_NAME *next_hop)
 {
+  /* Any inforec types that support next_hop should set it here. */
   switch (rec->type) {
   case TRP_INFOREC_TYPE_ROUTE:
     if (rec->data->route==NULL)
       return TRP_ERROR;
     rec->data->route->next_hop=next_hop;
     break;
-  case TRP_INFOREC_TYPE_COMMUNITY:
-    /* next hop not used for community records */
-    break;
+
   default:
-    break;
+    /* next hop not used for other records */
+    return TRP_UNSUPPORTED;
   }
   return TRP_SUCCESS;
 }
@@ -770,11 +782,22 @@ void trp_upd_set_next_hop(TRP_UPD *upd, const char *hostname, unsigned int port)
 {
   TRP_INFOREC *rec=NULL;
   TR_NAME *cpy=NULL;
-  
+
   for (rec=trp_upd_get_inforec(upd); rec!=NULL; rec=trp_inforec_get_next(rec)) {
-    if (trp_inforec_set_next_hop(rec, cpy=tr_new_name(hostname)) != TRP_SUCCESS) {
-      tr_err("trp_upd_set_next_hop: error setting next hop.");
-      tr_free_name(cpy);
+    switch (trp_inforec_set_next_hop(rec, cpy=tr_new_name(hostname))) {
+      case TRP_SUCCESS:
+        /* Success, the TR_NAME in cpy is now stored with the inforec */
+        break;
+
+      case TRP_UNSUPPORTED:
+        /* No error, but the inforec does not accept a next_hop. Free our copy. */
+        tr_free_name(cpy);
+        break;
+
+      default:
+        tr_err("trp_upd_set_next_hop: error setting next hop.");
+        tr_free_name(cpy);
+        break;
     }
   }
 }