Enhancement #493 - fixes for Codacy identified issues

README.md

@@ -2,7 +2,6 @@ Tcpreplay
 =========
 [![Build Status](https://travis-ci.org/appneta/tcpreplay.svg?branch=master)](https://travis-ci.org/appneta/tcpreplay)
 [![Coverity Scan Build Status](https://scan.coverity.com/projects/12017/badge.svg)](https://scan.coverity.com/projects/12017)
-[![Code Climate](https://codeclimate.com/github/appneta/tcpreplay.png)](https://codeclimate.com/github/appneta/tcpreplay)
 [![Codacy Badge](https://api.codacy.com/project/badge/Grade/0e49d208c69e440182ba21109ecaf31d)](https://www.codacy.com/app/fklassen/tcpreplay?utm_source=github.com&utm_medium=referral&utm_content=appneta/tcpreplay&utm_campaign=badger)
 [![Website](https://img.shields.io/website-up-down-green-red/http/shields.io.svg)](http://tcpreplay.appneta.com)
 

docs/CHANGELOG

@@ -1,4 +1,5 @@
 10/18/2018 Version 4.3.0 beta2
+    - fix issues identifed by Codacy (#493)
     - CVE-2018-17974 heap-buffer-overflow dlt_en10mb_encode (#486)
     - CVE-2018-17582 heap-buffer-overflow in get_next_packet (#484)
     - CVE-2018-13112 heap-buffer-overflow in get_l2len (#477 dup #408)

src/common/cidr.c

@@ -241,7 +241,7 @@ cidr2cidr(char *cidr)
             if (octets[count] > 255)
                 goto error;
 
-            snprintf(tempoctet, sizeof(octets[count]), "%d", octets[count]);
+            snprintf(tempoctet, sizeof(octets[count]), "%u", octets[count]);
             strcat(networkip, tempoctet);
             /* we don't want a '.' at the end of the last octet */
             if (count < 3)

src/common/get.c

@@ -78,13 +78,8 @@ get_pcap_version(void)
 uint16_t
 get_l2protocol(const u_char *pktdata, const int datalen, const int datalink)
 {
-    eth_hdr_t *eth_hdr;
-    vlan_hdr_t *vlan_hdr;
-    hdlc_hdr_t *hdlc_hdr;
-    sll_hdr_t *sll_hdr;
     uint16_t ether_type;
     uint16_t eth_hdr_offset = 0;
-    struct tcpr_pppserial_hdr *ppp;
 
     if (!pktdata || !datalen) {
         errx(-1, "invalid l2 parameters: pktdata=0x%p len=%d",
@@ -111,10 +106,11 @@ get_l2protocol(const u_char *pktdata, const int datalen, const int datalink)
         } else {
             eth_hdr_offset = 4; /* no header extensions */
         }
-        /* fall through */
+        /* no break */
     case DLT_EN10MB:
         if (datalen >= (sizeof(eth_hdr_t) + eth_hdr_offset)) {
-            eth_hdr = (eth_hdr_t *)(pktdata + eth_hdr_offset);
+            vlan_hdr_t *vlan_hdr;
+            eth_hdr_t *eth_hdr = (eth_hdr_t *)(pktdata + eth_hdr_offset);
             ether_type = ntohs(eth_hdr->ether_type);
             switch (ether_type) {
             case ETHERTYPE_VLAN: /* 802.1q */
@@ -128,7 +124,7 @@ get_l2protocol(const u_char *pktdata, const int datalen, const int datalink)
 
     case DLT_PPP_SERIAL:
         if (datalen >= sizeof(struct tcpr_pppserial_hdr)) {
-            ppp = (struct tcpr_pppserial_hdr *)pktdata;
+            struct tcpr_pppserial_hdr *ppp = (struct tcpr_pppserial_hdr *)pktdata;
             if (ntohs(ppp->protocol) == 0x0021)
                 return htons(ETHERTYPE_IP);
             else
@@ -138,14 +134,14 @@ get_l2protocol(const u_char *pktdata, const int datalen, const int datalink)
 
     case DLT_C_HDLC:
         if (datalen >= sizeof(hdlc_hdr_t)) {
-            hdlc_hdr = (hdlc_hdr_t *)pktdata;
+            hdlc_hdr_t *hdlc_hdr = (hdlc_hdr_t *)pktdata;
             return hdlc_hdr->protocol;
         }
         break;
 
     case DLT_LINUX_SLL:
         if (datalen >= sizeof(sll_hdr_t)) {
-            sll_hdr = (sll_hdr_t *)pktdata;
+            sll_hdr_t *sll_hdr = (sll_hdr_t *)pktdata;
             return sll_hdr->sll_protocol;
         }
         break;
@@ -166,8 +162,6 @@ get_l2protocol(const u_char *pktdata, const int datalen, const int datalink)
 int
 get_l2len(const u_char *pktdata, const int datalen, const int datalink)
 {
-    uint16_t ether_type = 0;
-    vlan_hdr_t *vlan_hdr;
     int l2_len = 0;
 
     assert(pktdata);
@@ -180,13 +174,13 @@ get_l2len(const u_char *pktdata, const int datalen, const int datalink)
 
     case DLT_JUNIPER_ETHER:
         l2_len = 24;
-        /* fall through */
+        /* no break */
     case DLT_EN10MB:
         if (datalen >= sizeof(eth_hdr_t) + l2_len) {
-            ether_type = ntohs(((eth_hdr_t*)(pktdata + l2_len))->ether_type);
+            uint16_t ether_type = ntohs(((eth_hdr_t*)(pktdata + l2_len))->ether_type);
 
             while (ether_type == ETHERTYPE_VLAN) {
-                vlan_hdr = (vlan_hdr_t *)(pktdata + l2_len);
+                vlan_hdr_t *vlan_hdr = (vlan_hdr_t *)(pktdata + l2_len);
                 ether_type = ntohs(vlan_hdr->vlan_len);
                 l2_len += 4;
                 if (datalen < sizeof(vlan_hdr_t) + l2_len) {

src/common/interface.c

@@ -84,8 +84,8 @@ get_interface_list(void)
     int fd = -1;
     nmreq_t nmr;
 #endif
-#if defined HAVE_LIBPCAP_NETMAP || defined HAVE_NETMAP
-    u_int32_t netmap_version = -1;
+#ifdef HAVE_NETMAP
+    u_int32_t netmap_version;
 #endif
 
 #ifndef HAVE_WIN32
@@ -135,8 +135,6 @@ get_interface_list(void)
             strncpy(nmr.nr_name, pcap_if_ptr->name, sizeof(nmr.nr_name));
             nmr.nr_version = netmap_version;
             if (ioctl(fd, NIOCGINFO, &nmr) == 0) {
-                int x;
-
 #endif /* HAVE_NETMAP */
 #if defined HAVE_LIBPCAP_NETMAP || defined HAVE_NETMAP
                 list_ptr->next = (interface_list_t *)safe_malloc(sizeof(interface_list_t));
@@ -150,8 +148,11 @@ get_interface_list(void)
                 snprintf(list_ptr->name, sizeof(list_ptr->name), "netmap:%s", pcap_if_ptr->name);
                 sprintf(list_ptr->alias, "%%%d", i++);
                 list_ptr->flags = pcap_if_ptr->flags;
-
+#endif /* HAVE_LIBPCAP_NETMAP  || HAVE_NETMAP */
+#ifdef HAVE_NETMAP
                 if (netmap_version >= 10) {
+                    int x;
+
                     list_ptr->next = (interface_list_t *)safe_malloc(sizeof(interface_list_t));
                     list_ptr = list_ptr->next;
                     snprintf(list_ptr->name, sizeof(list_ptr->name), "netmap:%s!", pcap_if_ptr->name);
@@ -169,10 +170,6 @@ get_interface_list(void)
                     snprintf(list_ptr->name, sizeof(list_ptr->name), "netmap:%s^", pcap_if_ptr->name);
                     sprintf(list_ptr->alias, "%%%d", i++);
                     list_ptr->flags = pcap_if_ptr->flags;
-                }
-#endif /* HAVE_LIBPCAP_NETMAP  || HAVE_NETMAP */
-#ifdef HAVE_NETMAP
-                if (netmap_version >= 10) {
                     for (x = 0; x < nmr.nr_rx_rings; ++x) {
                         list_ptr->next = (interface_list_t *)safe_malloc(sizeof(interface_list_t));
                         list_ptr = list_ptr->next;

src/common/list.c

@@ -1,5 +1,3 @@
-/* $Id$ */
-
 /*
  *   Copyright (c) 2001-2010 Aaron Turner <aturner at synfin dot net>
  *   Copyright (c) 2013-2018 Fred Klassen <tcpreplay at appneta dot com> - AppNeta
@@ -62,14 +60,14 @@ parse_list(tcpr_list_t ** listdata, char *ourstr)
     char *first, *second;
     int rcode;
     regex_t preg;
-    char ebuf[EBUF_SIZE];
     char regex[] = "^[0-9]+(-[0-9]+)?$";
     char *token = NULL;
     u_int i;
 
 
     /* compile the regex first */
     if ((rcode = regcomp(&preg, regex, REG_EXTENDED | REG_NOSUB)) != 0) {
+        char ebuf[EBUF_SIZE];
         regerror(rcode, &preg, ebuf, sizeof(ebuf));
         errx(-1, "Unable to compile regex (%s): %s", regex, ebuf);
     }

src/fragroute/fragroute.c

@@ -42,7 +42,6 @@ fragroute_close(fragroute_t *ctx)
     assert(ctx);
     free(ctx->pktq);
     free(ctx);
-    ctx = NULL;
     pkt_close();
 }
 

src/fragroute/mod_ip6_opt.c

@@ -56,7 +56,6 @@ void *
 ip6_opt_open(int argc, char *argv[])
 {
 	struct ip6_opt_data *opt;
-	int i, j;
 
 	if (argc < 4)
 		return (NULL);
@@ -65,8 +64,9 @@ ip6_opt_open(int argc, char *argv[])
 		return (NULL);
 
 	if (strcasecmp(argv[1], "route") == 0) {
-		opt->type = OPT6_TYPE_ROUTE;
+	    int i, j;
 
+	    opt->type = OPT6_TYPE_ROUTE;
 		if ((opt->u.route.segments = atoi(argv[2])) < 1 ||
 				opt->u.route.segments > MAX_ADDRS) {
 			warnx("<segments> must be >= 1");

src/fragroute/mod_ip6_qos.c

@@ -41,11 +41,11 @@ ip6_qos_open(int argc, char *argv[])
 	if ((data = calloc(1, sizeof(*data))) == NULL)
 		return (NULL);
 
-	if (sscanf(argv[1], "%x", &data->ip6_tc) != 1 ||
+	if (sscanf(argv[1], "%x", (unsigned int*)&data->ip6_tc) != 1 ||
 			data->ip6_tc < 0 || data->ip6_tc > 255)
 		return (ip6_qos_close(data));
 
-	if (sscanf(argv[2], "%x", &data->ip6_fl) != 1 ||
+	if (sscanf(argv[2], "%x", (unsigned int*)&data->ip6_fl) != 1 ||
 			data->ip6_fl < 0 || data->ip6_fl > 0x100000)
 		return (ip6_qos_close(data));
 

src/tcpedit/edit_packet.c

@@ -522,10 +522,9 @@ extract_data(tcpedit_t *tcpedit, const u_char *pktdata, int caplen,
         char *l7data[])
 {
     int datalen = 0; /* amount of data beyond ip header */
-    ipv4_hdr_t *ip_hdr = NULL;
-    tcp_hdr_t *tcp_hdr = NULL;
+    ipv4_hdr_t *ip_hdr;
     u_char ipbuff[MAXPACKET];
-    u_char *dataptr = NULL;
+    u_char *dataptr;
     int ip_len;
 
     assert(tcpedit);
@@ -557,7 +556,7 @@ extract_data(tcpedit_t *tcpedit, const u_char *pktdata, int caplen,
 
     /* TCP ? */
     if (ip_hdr->ip_p == IPPROTO_TCP) {
-        tcp_hdr = (tcp_hdr_t *) get_layer4_v4(ip_hdr, datalen);
+        tcp_hdr_t *tcp_hdr = (tcp_hdr_t *) get_layer4_v4(ip_hdr, datalen);
         datalen -= tcp_hdr->th_off << 2;
         if (datalen <= 0)
             goto nodata;
@@ -964,7 +963,6 @@ randomize_iparp(tcpedit_t *tcpedit, struct pcap_pkthdr *pkthdr,
     arp_hdr_t *arp_hdr = NULL;
     int l2len = 0;
     uint32_t *ip;
-    u_char *add_hdr;
 #ifdef FORCE_ALIGN
     uint32_t iptemp;
 #endif
@@ -984,8 +982,8 @@ randomize_iparp(tcpedit_t *tcpedit, struct pcap_pkthdr *pkthdr,
          (ntohs(arp_hdr->ar_op) == ARPOP_REPLY))) {
 
         /* jump to the addresses */
-        add_hdr = (u_char *)arp_hdr;
-        add_hdr += sizeof(arp_hdr_t) + arp_hdr->ar_hln;
+        u_char *add_hdr = ((u_char *)arp_hdr) + sizeof(arp_hdr_t) +
+                arp_hdr->ar_hln;
 #ifdef FORCE_ALIGN
         /* copy IP to a temporary buffer for processing */
         memcpy(&iptemp, add_hdr, sizeof(uint32_t));

src/tcpedit/plugins/dlt_ieee80211/ieee80211_hdr.c

@@ -143,19 +143,17 @@ ieee80211_is_encrypted(tcpeditdlt_t *ctx, const void *packet, const int pktlen)
 u_char *
 ieee80211_get_src(const void *header)
 {
-    ieee80211_hdr_t *addr3;
-    ieee80211_addr4_hdr_t *addr4;
     uint16_t *frame_control, fc;
 
     assert(header);
     frame_control = (uint16_t *)header;
     fc = ntohs(*frame_control);
 
     if (ieee80211_USE_4(fc)) {
-        addr4 = (ieee80211_addr4_hdr_t *)header;
+        ieee80211_addr4_hdr_t *addr4 = (ieee80211_addr4_hdr_t *)header;
         return addr4->addr4;
     } else {
-        addr3 = (ieee80211_hdr_t *)header;
+        ieee80211_hdr_t *addr3 = (ieee80211_hdr_t *)header;
         switch (fc & (ieee80211_FC_TO_DS_MASK + ieee80211_FC_FROM_DS_MASK)) {
             case ieee80211_FC_TO_DS_MASK:
                 return addr3->addr2;

src/tcpedit/portmap.c

@@ -63,7 +63,7 @@ ports2PORT(char *ports)
 {
     tcpedit_portmap_t *portmap = NULL, *portmap_head = NULL, *portmap_last = NULL;
     char *from_s, *to_s, *from_begin, *from_end, *badchar;
-    long from_l, to_l, from_b, from_e, i;
+    long from_l, to_l, i;
     char *token = NULL, *token2 = NULL;
 
     assert(ports);
@@ -107,13 +107,13 @@ ports2PORT(char *ports)
     if (strchr(from_s, '-')) {
         from_begin = strtok_r(from_s, "-", &token2);
         from_end = strtok_r(NULL, "-", &token2);
-        from_b = strtol(from_begin, &badchar, 10);
+        long from_b = strtol(from_begin, &badchar, 10);
         if (strlen(badchar) != 0) {
             free(portmap);
             return NULL;
         }
-        from_e = strtol(from_end, &badchar, 10);
 
+        long from_e = strtol(from_end, &badchar, 10);
         if (from_b > 65535 || from_b < 0 || from_e > 65535 || from_e < 0) {
             free(portmap);
             return NULL;
@@ -184,7 +184,7 @@ int
 parse_portmap(tcpedit_portmap_t ** portmap, const char *ourstr)
 {
     tcpedit_portmap_t *portmap_ptr;
-    char *substr = NULL, *ourstrcpy = NULL, *token = NULL;
+    char *substr, *ourstrcpy, *token = NULL;
 
     assert(ourstr);
     ourstrcpy = safe_strdup(ourstr);

src/tcpliveplay.c

@@ -259,7 +259,7 @@ main(int argc, char **argv)
     /* Start replay by sending the first packet, the SYN, from the schedule */
     else if(sched[0].local){  /* Send first packet*/
         sendpacket(sp, sched[sched_index].packet_ptr, sched[sched_index].pkthdr.len, &sched[sched_index].pkthdr);
-        printf("Sending Local Packet...............	[%d]\n",sched_index+1);
+        printf("Sending Local Packet...............	[%u]\n",sched_index+1);
         sched_index++; /* Proceed in the schedule */
     }
 
@@ -296,8 +296,8 @@ main(int argc, char **argv)
         } */
         /* Do the following if we receive a packet that ACKs for the same ACKing of next packet */
         else if((tcphdr_rprev->th_seq==htonl(sched[sched_index].exp_rseq)) && (tcphdr_rprev->th_ack==htonl(sched[sched_index].exp_rack)) && (size_payload_prev>0)){
-            printf("Received Remote Packet...............	[%d]\n",sched_index+1);
-            printf("Skipping Packet......................	[%d] to Packet [%d]\n",sched_index+1, sched_index+2);
+            printf("Received Remote Packet...............	[%u]\n",sched_index+1);
+            printf("Skipping Packet......................	[%u] to Packet [%u]\n",sched_index+1, sched_index+2);
             printf("Next Remote Packet Expectation met.\nProceeding in replay...\n");
             sched_index++;   
         } 
@@ -714,9 +714,9 @@ set_offline_filter(char* file)
 void 
 got_packet(u_char *args, const struct pcap_pkthdr *header, const u_char *packet){
 
-    ether_hdr *etherhdr = NULL; 
-    tcp_hdr *tcphdr = NULL;
-    ipv4_hdr *iphdr = NULL;
+    ether_hdr *etherhdr;
+    tcp_hdr *tcphdr;
+    ipv4_hdr *iphdr;
 
     unsigned int size_ip, size_tcp, size_payload;
     unsigned int flags = 0;
@@ -956,7 +956,6 @@ rewrite(input_addr* new_remoteip, struct mac_addr* new_remotemac, input_addr* my
     struct pcap_pkthdr *header;
     pcap_dumper_t *dumpfile;
     input_addr sip;  /* Source IP */ 
-    unsigned int flags;
     int local_packets = 0;
     bool initstep1 = false;  /* keep track of successful handshake step */
     bool warned = false;
@@ -986,6 +985,7 @@ rewrite(input_addr* new_remoteip, struct mac_addr* new_remotemac, input_addr* my
 
     /*Modify each packet's IP & MAC based on the passed args then do a checksum of each packet*/
     for (pkt_counter = 0; safe_pcap_next_ex(pcap, &header, &packet) > 0; pkt_counter++){
+        unsigned int flags;
 
         if (!warned && header->len > header->caplen) {
             fprintf(stderr, "warning: packet capture truncated to %d byte packets\n",

src/tcpprep_api.c

@@ -221,12 +221,12 @@ int
 tcpprep_set_regex(tcpprep_t *ctx, char *value)
 {
     int regex_error;
-    char ebuf[EBUF_SIZE];
 
     assert(ctx);
 
     if ((regex_error = regcomp(&ctx->options->preg, value,
                     REG_EXTENDED|REG_NOSUB))) {
+        char ebuf[EBUF_SIZE];
         regerror(regex_error, &ctx->options->preg, ebuf, EBUF_SIZE);
         tcpprep_seterr(ctx, "Unable to compile regex (%s): %s", value, regex_error);
         return -1;

src/tcpreplay.c

@@ -60,7 +60,6 @@ main(int argc, char *argv[])
 {
     int i, optct = 0;
     int rcode;
-    char buf[1024];
 
     fflush(NULL);
 
@@ -146,6 +145,8 @@ main(int argc, char *argv[])
     }
 
     if (ctx->stats.bytes_sent > 0) {
+        char buf[1024];
+
         packet_stats(&ctx->stats);
         if (ctx->options->flow_stats)
             flow_stats(ctx);