Follow the flawfinder guideline
authorNeutron Soutmun <neo.neutron@gmail.com>
Mon, 8 Jun 2009 14:12:46 +0000 (21:12 +0700)
committerNeutron Soutmun <neo.neutron@gmail.com>
Mon, 8 Jun 2009 14:12:46 +0000 (21:12 +0700)
  * Reduce the vulnerability code by follow the advice of flawfinder.
  * Just finish one of the TODO lists.

TODO
src/rahunasd.c
src/rh-config.c
src/rh-ipset.c
src/rh-task-bandwidth.c
src/rh-task-dbset.c
src/rh-task-memset.c

diff --git a/TODO b/TODO
index 3d81a10..6118f5c 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,3 +1,2 @@
 - new version of ipset released, need test (work ?)
 - write the readme. (may Suriya ?)
-- using flawfinder to guideline the code vulnerability.
index fa5e616..37bb683 100644 (file)
@@ -365,7 +365,8 @@ int main(int argc, char **argv)
     exit(EXIT_FAILURE);
   }
 
-  sprintf(version, "Starting %s - Version %s", PROGRAM, RAHUNAS_VERSION);
+  snprintf(version, sizeof (version), "Starting %s - Version %s", PROGRAM, 
+           RAHUNAS_VERSION);
   logmsg(RH_LOG_NORMAL, version);
 
   rh_task_register(rh_main_server);
index 7747d4d..f64c83c 100755 (executable)
@@ -369,7 +369,8 @@ GList *append_interface (GList *inf,
   while (runner != NULL)
     {
       iface = (struct interfaces *)runner->data;
-      if (strncmp(iface->dev_internal, inf_name, strlen(inf_name)) == 0)
+      if (iface->dev_internal &&
+          strncmp(iface->dev_internal, inf_name, strlen(inf_name)) == 0)
         {
           // Already in the list
           (iface->hit)++;
@@ -387,8 +388,8 @@ done:
       return inf;
     }
 
-  strncpy(item->dev_internal, inf_name, 32);
-  sprintf(item->dev_ifb, "ifb%d", ifb_ifno);
+  strncpy(item->dev_internal, inf_name, sizeof (item->dev_internal));
+  snprintf(item->dev_ifb, sizeof (item->dev_ifb), "ifb%d", ifb_ifno);
   item->init = 0;
   item->hit  = 1;
   DP ("Interface append: %s, %s", item->dev_internal, item->dev_ifb);
@@ -410,7 +411,8 @@ GList *remove_interface (GList *inf,
   while (runner != NULL)
     {
       iface = (struct interfaces *)runner->data;
-      if (strncmp (iface->dev_internal, inf_name, strlen (inf_name)) == 0)
+      if (iface->dev_internal &&
+          strncmp (iface->dev_internal, inf_name, strlen (inf_name)) == 0)
         {
           iface->hit--;
           if (iface->hit <=0 )
index 51c8994..08fb88e 100644 (file)
@@ -121,7 +121,7 @@ struct set *set_adt_get(const char *name)
 
   req_adt_get.op = IP_SET_OP_ADT_GET;
   req_adt_get.version = IP_SET_PROTOCOL_VERSION;
-  strcpy(req_adt_get.set.name, name);
+  strncpy(req_adt_get.set.name, name, IP_SET_MAXNAMELEN);
   size = sizeof(struct ip_set_req_adt_get);
 
   kernel_getfrom((void *) &req_adt_get, &size);
@@ -149,6 +149,9 @@ void parse_ip(const char *str, ip_set_ip_t *ip)
 void parse_mac(const char *mac, unsigned char *ethernet)
 {
   unsigned int i = 0;
+  if (!mac)
+    return;
+
   if (strlen(mac) != ETH_ALEN * 3 - 1)
     return;
 
@@ -180,7 +183,7 @@ char *mac_tostring(unsigned char macaddress[ETH_ALEN])
 {
   static char mac_string[18] = "";
  
-  sprintf(mac_string, "%02X:%02X:%02X:%02X:%02X:%02X", 
+  snprintf(mac_string, sizeof (mac_string), "%02X:%02X:%02X:%02X:%02X:%02X", 
           macaddress[0], macaddress[1], macaddress[2],
           macaddress[3], macaddress[4], macaddress[5]);
   return mac_string; 
@@ -260,7 +263,7 @@ void set_flush(const char *name)
 
   req.op = IP_SET_OP_FLUSH;
   req.version = IP_SET_PROTOCOL_VERSION;
-  strcpy(req.name, name);
+  strncpy(req.name, name, IP_SET_MAXNAMELEN);
 
   kernel_sendto(&req, sizeof(struct ip_set_req_std));
 }
@@ -287,7 +290,7 @@ tryagain:
   /* Get max_sets */
   req_max_sets.op = IP_SET_OP_MAX_SETS;
   req_max_sets.version = IP_SET_PROTOCOL_VERSION;
-  strcpy(req_max_sets.set.name, name);
+  strncpy(req_max_sets.set.name, name, IP_SET_MAXNAMELEN);
   size = sizeof(req_max_sets);
   kernel_getfrom(&req_max_sets, &size);
 
index c400254..5bc19e7 100644 (file)
@@ -281,15 +281,15 @@ static int startsess (struct vserver *vs, struct task_req *req)
     return 0;
   
   // Formating the bandwidth request
-  sprintf(bw_req.ip, "%s", idtoip(vs->v_map, req->id));
-  sprintf(bw_req.bandwidth_max_down, "%lu", 
-    member->bandwidth_max_down);
-  sprintf(bw_req.bandwidth_max_up, "%lu", 
-    member->bandwidth_max_up);
+  snprintf(bw_req.ip, sizeof (bw_req.ip), "%s", idtoip(vs->v_map, req->id));
+  snprintf(bw_req.bandwidth_max_down, sizeof (bw_req.bandwidth_max_down), 
+           "%lu", member->bandwidth_max_down);
+  snprintf(bw_req.bandwidth_max_up, sizeof (bw_req.bandwidth_max_up), "%lu", 
+           member->bandwidth_max_up);
   
   while (max_try > 0) { 
     slot_id = _get_slot_id();
-    sprintf(bw_req.slot_id, "%d", slot_id);
+    snprintf(bw_req.slot_id, sizeof (bw_req.slot_id), "%d", slot_id);
     if (bandwidth_add(vs, &bw_req) == 0)
       break;
     else
@@ -327,7 +327,7 @@ static int stopsess  (struct vserver *vs, struct task_req *req)
   if (slot_id < 1)
     return 0;
 
-  sprintf(bw_req.slot_id, "%d", slot_id);
+  snprintf(bw_req.slot_id, sizeof (bw_req.slot_id), "%d", slot_id);
 
   if (bandwidth_del(vs, &bw_req) == 0) {
     member->bandwidth_slot_id = 0;
index 629ae23..fc7f66f 100644 (file)
@@ -293,8 +293,9 @@ static void init (struct vserver *vs)
                  PROGRAM, NULL, NULL,
                  GDA_CONNECTION_OPTIONS_READ_ONLY, NULL);
 
-  sprintf(select_cmd, "SELECT * FROM dbset WHERE vserver_id='%d'",
-          vs->vserver_config->vserver_id);
+  snprintf(select_cmd, sizeof (select_cmd), 
+           "SELECT * FROM dbset WHERE vserver_id='%d'",
+           vs->vserver_config->vserver_id);
 
   DP("SQL: %s", select_cmd);
 
@@ -342,7 +343,7 @@ static int startsess (struct vserver *vs, struct task_req *req)
 
   member = (struct rahunas_member *) member_node->data;
 
-  sprintf(startsess_cmd, "INSERT INTO dbset"
+  snprintf(startsess_cmd, sizeof (startsess_cmd), "INSERT INTO dbset"
          "(session_id,vserver_id,username,ip,mac,session_start,"
          "session_timeout,bandwidth_slot_id,bandwidth_max_down,"
          "bandwidth_max_up) "
@@ -393,7 +394,7 @@ static int stopsess (struct vserver *vs, struct task_req *req)
   DP("Username  : %s", member->username);
   DP("SessionID : %s", member->session_id);
 
-  sprintf(stopsess_cmd, "DELETE FROM dbset WHERE "
+  snprintf(stopsess_cmd, sizeof (stopsess_cmd), "DELETE FROM dbset WHERE "
          "session_id='%s' AND username='%s' AND vserver_id='%d'",
          member->session_id, 
          member->username,
index a1028c6..92bf76a 100644 (file)
@@ -199,19 +199,19 @@ static int stopsess  (struct vserver *vs, struct task_req *req)
 
   switch (req->req_opt) {
     case RH_RADIUS_TERM_IDLE_TIMEOUT :
-      strcpy(cause, "idle timeout");
+      strncpy(cause, "idle timeout", sizeof (cause));
       break;
     case RH_RADIUS_TERM_SESSION_TIMEOUT :
-      strcpy(cause, "session timeout");
+      strncpy(cause, "session timeout", sizeof (cause));
       break;
     case RH_RADIUS_TERM_USER_REQUEST :
-      strcpy(cause, "user request");
+      strncpy(cause, "user request", sizeof (cause));
       break;
     case RH_RADIUS_TERM_NAS_REBOOT :
-      strcpy(cause, "nas reboot");
+      strncpy(cause, "nas reboot", sizeof (cause));
       break;
     case RH_RADIUS_TERM_ADMIN_RESET :
-      strcpy(cause, "admin reset");
+      strncpy(cause, "admin reset", sizeof (cause));
       break;
   }
   if (!member->username)