From: Josh <jrosenba@cisco.com>
Date: Thu, 4 Sep 2014 16:15:57 +0000 (-0400)
Subject: fixing some static analyzer bugs
X-Git-Tag: 3.0.0-233~1414^2~2^2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fcd8cc17e0d90490eb3d1ff787cd8e7a104cf6be;p=thirdparty%2Fsnort3.git

fixing some static analyzer bugs
---

fcd8cc17e0d90490eb3d1ff787cd8e7a104cf6be
diff --cc src/codecs/ip/checksum.h
index c8f386436,c8f386436..03580683e
--- a/src/codecs/ip/checksum.h
+++ b/src/codecs/ip/checksum.h
@@@ -36,7 -36,7 +36,8 @@@ namespace checksu
  
  struct Pseudoheader6
  {
--    uint32_t sip[4], dip[4];
++    uint32_t sip[4];
++    uint32_t dip[4];
      uint8_t  zero;
      uint8_t  protocol;
      uint16_t len;
@@@ -45,7 -45,7 +46,9 @@@
  
  struct Pseudoheader
  {
--    uint32_t sip, dip;
++
++    uint32_t sip;
++    uint32_t dip;
      uint8_t  zero;
      uint8_t  protocol;
      uint16_t len;
@@@ -78,6 -78,6 +81,25 @@@ inline uint16_t ip_cksum(const uint16_
  namespace detail
  {
  
++
++struct PsuedoheaderUnion
++{
++    union
++    {
++        Pseudoheader ph4;
++        uint16_t ph4_arr[12];
++    };
++};
++
++struct Psuedoheader6Union
++{
++    union
++    {
++        Pseudoheader ph6;
++        uint16_t ph6_arr[18];
++    };
++};
++
  static inline uint16_t cksum_add(const uint16_t *buf, std::size_t len, uint32_t cksum)
  {
      const uint16_t *sp = buf;
@@@ -158,9 -158,9 +180,17 @@@
  }
  
  
--static inline void add_ipv4_pseudoheader(const uint16_t* const h,
++static inline void add_ipv4_pseudoheader(const Pseudoheader* const ph4,
                                           uint32_t &cksum)
  {
++    /*
++     * This mess is necessary to make static analyzers happy.
++     * Otherwise they assume we are reading garbage values
++     */
++    const PsuedoheaderUnion* const ph4_u = reinterpret_cast
++        <const PsuedoheaderUnion* const>(ph4);
++    const uint16_t* const h = ph4_u->ph4_arr;
++
      /* ipv4 pseudo header must have 12 bytes */
      cksum += h[0];
      cksum += h[1];
@@@ -171,28 -171,28 +201,36 @@@
  }
  
  
--static inline void add_ipv6_pseudoheader(const uint16_t* const h,
++static inline void add_ipv6_pseudoheader(const Pseudoheader6* const ph6,
                                           uint32_t &cksum)
  {
--   /* PseudoHeader must have 36 bytes */
--   cksum += h[0];
--   cksum += h[1];
--   cksum += h[2];
--   cksum += h[3];
--   cksum += h[4];
--   cksum += h[5];
--   cksum += h[6];
--   cksum += h[7];
--   cksum += h[8];
--   cksum += h[9];
--   cksum += h[10];
--   cksum += h[11];
--   cksum += h[12];
--   cksum += h[13];
--   cksum += h[14];
--   cksum += h[15];
--   cksum += h[16];
--   cksum += h[17];
++    /*
++     * This mess is necessary to make static analyzers happy.
++     * Otherwise they assume we are reading garbage values
++     */
++    const Psuedoheader6Union* const ph6_u = reinterpret_cast
++        <const Psuedoheader6Union* const>(ph6);
++    const uint16_t* const h = ph6_u->ph6_arr;
++
++    /* PseudoHeader must have 36 bytes */
++    cksum += h[0];
++    cksum += h[1];
++    cksum += h[2];
++    cksum += h[3];
++    cksum += h[4];
++    cksum += h[5];
++    cksum += h[6];
++    cksum += h[7];
++    cksum += h[8];
++    cksum += h[9];
++    cksum += h[10];
++    cksum += h[11];
++    cksum += h[12];
++    cksum += h[13];
++    cksum += h[14];
++    cksum += h[15];
++    cksum += h[16];
++    cksum += h[17];
  }
  
  
@@@ -256,7 -256,7 +294,7 @@@ inline uint16_t icmp_cksum(const uint16
  {
      uint32_t cksum = 0;
  
--    detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum);
++    detail::add_ipv6_pseudoheader(ph, cksum);
      return detail::cksum_add(buf, len, cksum);
  }
  
@@@ -272,7 -272,7 +310,7 @@@ inline uint16_t tcp_cksum(const uint16_
  {
      uint32_t cksum = 0;
  
--    detail::add_ipv4_pseudoheader((const uint16_t* const)ph, cksum);
++    detail::add_ipv4_pseudoheader(ph, cksum);
      detail::add_tcp_header(h, len, cksum);
      return detail::cksum_add(h, len, cksum);
  }
@@@ -284,7 -284,7 +322,7 @@@ inline uint16_t tcp_cksum(const uint16_
  {
      uint32_t cksum = 0;
  
--    detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum);
++    detail::add_ipv6_pseudoheader(ph, cksum);
      detail::add_tcp_header(buf, len, cksum);
      return detail::cksum_add(buf, len, cksum);
  }
@@@ -296,7 -296,7 +334,7 @@@ inline uint16_t udp_cksum(const uint16_
  {
      uint32_t cksum = 0;
  
--    detail::add_ipv4_pseudoheader((const uint16_t* const)ph, cksum);
++    detail::add_ipv4_pseudoheader(ph, cksum);
      detail::add_udp_header(buf, len, cksum);
      return detail::cksum_add(buf, len, cksum);
  }
@@@ -308,7 -308,7 +346,7 @@@ inline uint16_t udp_cksum(const uint16_
  {
      uint32_t cksum = 0;
  
--    detail::add_ipv6_pseudoheader((const uint16_t* const)ph, cksum);
++    detail::add_ipv6_pseudoheader(ph, cksum);
      detail::add_udp_header(buf, len, cksum);
      return detail::cksum_add(buf, len, cksum);
  }
diff --cc src/codecs/misc/cd_icmp4_ip.cc
index 1f4b93e16,1f4b93e16..e769b4bd9
--- a/src/codecs/misc/cd_icmp4_ip.cc
+++ b/src/codecs/misc/cd_icmp4_ip.cc
@@@ -89,7 -89,7 +89,6 @@@ bool Icmp4IpCodec::decode(const uint8_
          return false;
      }
  
--    ip_len = ntohs(ip4h->get_len());/* set the IP datagram length */
      hlen = ip4h->get_hlen() << 2;    /* set the IP header length */
  
      if(raw_len < hlen)
diff --cc src/file_api/libs/file_config.cc
index d9c9e5eda,d9c9e5eda..670daa865
--- a/src/file_api/libs/file_config.cc
+++ b/src/file_api/libs/file_config.cc
@@@ -314,6 -314,6 +314,7 @@@ static uint8_t* convertTextToHex(char *
              ParseError("content hexmode argument has invalid "
                      "number of hex digits.  The argument '%s' "
                      "must contain a full even byte string.", current_ptr);
++            free(hex);
              return (uint8_t*)"";
          }
  
@@@ -324,7 -324,7 +325,8 @@@
          else
          {
              ParseError("'%c' is not a valid hex value, please input hex values (0x0 - 0xF)",
--                    (char) *current_ptr);
++                       (char) *current_ptr);
++            free(hex);
              return (uint8_t*)"";
          }
  
@@@ -337,7 -337,7 +339,8 @@@
          else
          {
              ParseError("'%c' is not a valid hex value, please input hex values (0x0 - 0xF)",
--                    (char) *current_ptr);
++                       (char) *current_ptr);
++            free(hex);
              return (uint8_t*)"";
          }
          DEBUG_WRAP(DebugMessage(DEBUG_FILE,"Hex buffer: %s\n", hex_buf););
@@@ -604,6 -604,6 +607,7 @@@ void parse_file_rule(const char *args, 
      if (file_config->FileRules[rule->id])
      {
          ParseError("file type: duplicated rule id %d defined!", rule->id);
++        free(rule);
          return;
      }
      file_config->FileRules[rule->id] = rule;
diff --cc src/log/log_text.cc
index c500374ae,c500374ae..3e04c1589
--- a/src/log/log_text.cc
+++ b/src/log/log_text.cc
@@@ -888,7 -888,7 +888,7 @@@ void LogTcpOptions(TextLog*  log, cons
                          }
  
                          if (j < opts_len)
--                            TextLog_Print(log, "%02x", opts[i].data[j]);
++                            TextLog_Print(log, "%02x", 0);
                      }
  
                      TextLog_Putc(log, ' ');
diff --cc src/loggers/log_codecs.cc
index 3ba609c3b,3ba609c3b..650f97f10
--- a/src/loggers/log_codecs.cc
+++ b/src/loggers/log_codecs.cc
@@@ -193,7 -193,7 +193,7 @@@ static const LogApi log_codecs_api 
          mod_ctor,
          mod_dtor
      },
--    (OUTPUT_TYPE_FLAG__LOG | OUTPUT_TYPE_FLAG__ALERT),
++    OUTPUT_TYPE_FLAG__LOG,
      codec_log_ctor,
      codec_log_dtor
  };
diff --cc src/parser/parse_stream.cc
index 84c106c5e,84c106c5e..6fb748d7a
--- a/src/parser/parse_stream.cc
+++ b/src/parser/parse_stream.cc
@@@ -175,9 -175,9 +175,7 @@@ static TokenType get_token
              state = 3;
              break;
          case 5:  // unquoted escape
--            if ( c == '\n' )
--                state = 0;
--            else if ( c != '\r' )
++            if ( c != '\n' && c != '\r' )
                  printf("error: invalid escape on line %d\n", lines);
              state = 0;
              break;
diff --cc src/service_inspectors/ftp_telnet/hi_util_kmap.cc
index 6fd7e91ea,6fd7e91ea..d994557df
--- a/src/service_inspectors/ftp_telnet/hi_util_kmap.cc
+++ b/src/service_inspectors/ftp_telnet/hi_util_kmap.cc
@@@ -47,6 -47,6 +47,8 @@@
  #include "config.h"
  #endif
  
++#include <string>
++#include <limits>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
@@@ -55,9 -55,9 +57,8 @@@
  #include "hi_util_xmalloc.h"
  
  //#define MEMASSERT(p) if(!p){printf("KMAP-No Memory: File: %s Line:%d!\n",__FILE__,__LINE__);exit(0);}
--
  #define MEMASSERT(p)
--#define LOWERCASE tolower
++
  
  /*
  *
@@@ -238,24 -238,24 +239,29 @@@ static KMAPNODE * KMapCreateNode(KMAP 
  */
  int KMapAdd( KMAP *km, void * key, int n, void * userdata )
  {
--    int            i,ksize;
--    int            type = 0;
--    unsigned char *P = (unsigned char *)key;
--    KMAPNODE      *root;
--    unsigned char  xkey[256];
++    int i,ksize;
++    int type = 0;
++    const unsigned char *P = (unsigned char *)key;
++    KMAPNODE *root;
++    std::string xkey;
  
      if( n <= 0 )
      {
--        n = strlen( (char*) key );
--        if( n > (int)sizeof(xkey) )
++        std::size_t tmp_len = strlen( (char*) key);
++        if (tmp_len > std::numeric_limits<int>::max())
              return -99;
++
++        n = tmp_len;
      }
  
      if( km->nocase )
      {
++        xkey.resize(n);
++
          for(i=0;i<n;i++)
--            xkey[i] = LOWERCASE( P[i] );
--        P = xkey;
++            xkey[i] = std::tolower( P[i] );
++
++        P = (const unsigned char*) xkey.c_str();
      }
  
      /* Save key size */
@@@ -382,24 -382,24 +388,27 @@@
  */
  void *  KMapFind( KMAP * ks, void * key, int n )
  {
--    unsigned char * T = (unsigned char *)key;
--    KMAPNODE      * root;
--    unsigned char   xkey[256];
--    int             i;
++    const unsigned char * T = (unsigned char *)key;
++    KMAPNODE * root;
++    std::string xkey;
++    int i;
  
      if( n <= 0 )
      {
--        n = strlen( (char*)key );
--        if( n > (int)sizeof(xkey) )
--            return 0;
++        std::size_t tmp_len = strlen( (char*) key);
++        if (tmp_len > std::numeric_limits<int>::max())
++            return nullptr;
  
++        n = tmp_len;
      }
++
      if( ks->nocase )
      {
++        xkey.resize(n);
          for(i=0;i<n;i++)
--            xkey[i] = LOWERCASE( T[i] );
++            xkey[i] = std::tolower( T[i] );
  
--        T = xkey;
++        T = (const unsigned char*)(xkey.c_str());
      }
      //printf("finding key='%.*s'\n",n,T);
  
diff --cc src/service_inspectors/http_inspect/hi_util_kmap.cc
index 192e3090e,192e3090e..4c8923e91
--- a/src/service_inspectors/http_inspect/hi_util_kmap.cc
+++ b/src/service_inspectors/http_inspect/hi_util_kmap.cc
@@@ -47,6 -47,6 +47,8 @@@
  #include "config.h"
  #endif
  
++#include <string>
++#include <limits>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
@@@ -55,9 -55,9 +57,7 @@@
  #include "hi_util_xmalloc.h"
  
  //#define MEMASSERT(p) if(!p){printf("KMAP-No Memory: File: %s Line:%d!\n",__FILE__,__LINE__);exit(0);}
--
  #define MEMASSERT(p)
--#define LOWERCASE tolower
  
  /*
  *
@@@ -241,22 -241,22 +241,27 @@@ int KMapAdd( KMAP *km, void * key, int 
  {
      int            i,ksize;
      int            type = 0;
--    unsigned char *P = (unsigned char *)key;
++    const unsigned char *P = (unsigned char *)key;
      KMAPNODE      *root;
--    unsigned char  xkey[256];
++    std::string xkey;
  
      if( n <= 0 )
      {
--        n = strlen( (char*) key );
--        if( n > (int)sizeof(xkey) )
++        const std::size_t tmp_len = strlen( (char*) key);
++        if (tmp_len > std::numeric_limits<int>::max())
              return -99;
++
++        n = (int) tmp_len;
      }
  
      if( km->nocase )
      {
++        xkey.resize(n);
++
          for(i=0;i<n;i++)
--            xkey[i] = LOWERCASE( P[i] );
--        P = xkey;
++            xkey[i] = std::tolower( P[i] );
++
++        P = (const unsigned char*)xkey.c_str();
      }
  
      /* Save key size */
@@@ -383,24 -383,24 +388,27 @@@
  */
  void *  KMapFind( KMAP * ks, void * key, int n )
  {
--    unsigned char * T = (unsigned char *)key;
--    KMAPNODE      * root;
--    unsigned char   xkey[256];
--    int             i;
++    const unsigned char * T = (unsigned char *)key;
++    KMAPNODE * root;
++    int i;
++    std::string xkey;
  
      if( n <= 0 )
      {
--        n = strlen( (char*)key );
--        if( n > (int)sizeof(xkey) )
--            return 0;
++        const std::size_t tmp_len = strlen( (char*) key);
++        if (tmp_len > std::numeric_limits<int>::max())
++            return nullptr;
  
++        n = (int) tmp_len;
      }
++
      if( ks->nocase )
      {
++        xkey.resize(n);
          for(i=0;i<n;i++)
--            xkey[i] = LOWERCASE( T[i] );
++            xkey[i] = std::tolower( T[i] );
  
--        T = xkey;
++        T = (const unsigned char*)xkey.c_str();
      }
      //printf("finding key='%.*s'\n",n,T);
  
diff --cc src/sfip/sf_vartable.cc
index 7b7f03c9f,7b7f03c9f..027f26e47
--- a/src/sfip/sf_vartable.cc
+++ b/src/sfip/sf_vartable.cc
@@@ -223,7 -223,7 +223,8 @@@ SFIP_RET sfvt_add_str(vartable_t *table
      if(!table || !str || !ipret) return SFIP_FAILURE;
  
      /* Creates the variable */
--    if( (var = sfvar_alloc(table, str, &status)) == NULL )
++    var = sfvar_alloc(table, str, &status);
++    if( var == NULL )
      {
           return status;
      }
diff --cc src/utils/util.cc
index 28a900cb7,28a900cb7..4b192c149
--- a/src/utils/util.cc
+++ b/src/utils/util.cc
@@@ -522,12 -522,12 +522,14 @@@ char *read_infile(char *fname
      if(cc < 0)
      {
          ParseError("read %s: %s\n", fname, get_error(errno));
++        free(cp);
          return nullptr;
      }
  
      if(cc != buf.st_size)
      {
          ParseError("short read %s (%d != %d)\n", fname, cc, (int) buf.st_size);
++        free(cp);
          return nullptr;
      }
  
@@@ -913,6 -913,6 +915,7 @@@ void SetChroot(char *directory, char **
      {
          ParseError("SetChroot: Can not chdir to \"%s\": %s\n", directory,
                     get_error(errno));
++        free(logdir);
          return;
      }
  
@@@ -922,6 -922,6 +925,7 @@@
      if(absdir == NULL)
      {
          ParseError("NULL Chroot found\n");
++        free(logdir);
          return;
      }
  
@@@ -934,6 -934,6 +938,7 @@@
      {
          ParseError("Can not chroot to \"%s\": absolute: %s: %s\n",
                     directory, absdir, get_error(errno));
++        free(logdir);
          return;
      }
  
@@@ -945,6 -945,6 +950,7 @@@
      {
          ParseError("Can not chdir to \"/\" after chroot: %s\n",
                     get_error(errno));
++        free(logdir);
          return;
      }
  
@@@ -955,6 -955,6 +961,7 @@@
      if(strncmp(absdir, logdir, strlen(absdir)))
      {
          ParseError("Absdir is not a subset of the logdir");
++        free(logdir);
          return;
      }
  
@@@ -971,6 -971,6 +978,7 @@@
                              logdir, *logstore));
  
      LogMessage("Chroot directory = %s\n", directory);
++    free(logdir);
  }
  
  
diff --cc src/utils/util_jsnorm.cc
index ac0660d43,ac0660d43..3cb53043f
--- a/src/utils/util_jsnorm.cc
+++ b/src/utils/util_jsnorm.cc
@@@ -581,7 -581,7 +581,7 @@@ static int PNorm_scan_fsm(PNormState* s
  
  int PNormDecode(char *src, uint16_t srclen, char *dst, uint16_t dstlen, uint16_t *bytes_copied, JSState *js)
  {
--    int iRet;
++    int iRet = RET_OK;
      const char *end;
      char *ptr;
      PNormState s;
@@@ -1218,7 -1218,7 +1218,7 @@@ static int JSNorm_scan_fsm (JSNormState
  
  int JSNormalizeDecode(char *src, uint16_t srclen, char *dst, uint16_t destlen, char **ptr, int *bytes_copied, JSState *js, uint8_t* iis_unicode_map)
  {
--    int iRet;
++    int iRet = RET_OK;
      const char *start, *end;
      JSNormState s;