]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
fixing some static analyzer bugs
authorJosh <jrosenba@cisco.com>
Thu, 4 Sep 2014 16:15:57 +0000 (12:15 -0400)
committerJosh <jrosenba@cisco.com>
Thu, 4 Sep 2014 16:17:26 +0000 (12:17 -0400)
1  2 
src/codecs/ip/checksum.h
src/codecs/misc/cd_icmp4_ip.cc
src/file_api/libs/file_config.cc
src/log/log_text.cc
src/loggers/log_codecs.cc
src/parser/parse_stream.cc
src/service_inspectors/ftp_telnet/hi_util_kmap.cc
src/service_inspectors/http_inspect/hi_util_kmap.cc
src/sfip/sf_vartable.cc
src/utils/util.cc
src/utils/util_jsnorm.cc

index c8f3864369bbcafda703ea324fb6f569fb27d792,c8f3864369bbcafda703ea324fb6f569fb27d792..03580683e640b837c91d96d4f1b0242819321db5
@@@ -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;
  }
  
  
--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];
  }
  
  
--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);
  }
index 1f4b93e16396746933805242ed75409974270d98,1f4b93e16396746933805242ed75409974270d98..e769b4bd9f82c3e9c44b98a20855f618db5a3a99
@@@ -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)
index d9c9e5eda44e4c3ab581cb39d7b19dab4768ca80,d9c9e5eda44e4c3ab581cb39d7b19dab4768ca80..670daa865451d7ff19b806035661f450c2a7c488
@@@ -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*)"";
          }
  
          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*)"";
          }
  
          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;
index c500374ae5961bc88c858030e9c86d0832da6a04,c500374ae5961bc88c858030e9c86d0832da6a04..3e04c1589eee96373e3fa7dc556568a264ac6e2b
@@@ -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, ' ');
index 3ba609c3b7f922950afae492a87c9916e4bb594f,3ba609c3b7f922950afae492a87c9916e4bb594f..650f97f1073cfc2ea80a3baf806a05a95d6b5b81
@@@ -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
  };
index 84c106c5ef08d7d1d93dc809a518affa148ffc65,84c106c5ef08d7d1d93dc809a518affa148ffc65..6fb748d7ad6032b0b31becad70a8567ecdc3c480
@@@ -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;
index 6fd7e91ea475d12c18f1fa6e9372dc2be3818fbc,6fd7e91ea475d12c18f1fa6e9372dc2be3818fbc..d994557dfc5d85b4c19fe5a732143043f37c6549
@@@ -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 */
  */
  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);
  
index 192e3090e8b2ab3d43207b390a8036968abc1fc5,192e3090e8b2ab3d43207b390a8036968abc1fc5..4c8923e91d5cfec4d045e64aeec4c82abd88b5a0
@@@ -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 */
  */
  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);
  
index 7b7f03c9fe485d4e1b46e74228ad2930d40c2e38,7b7f03c9fe485d4e1b46e74228ad2930d40c2e38..027f26e47c243da5863c3f7097ea820bf9078fce
@@@ -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;
      }
index 28a900cb77987e7742c55de4b3bdd2859279247a,28a900cb77987e7742c55de4b3bdd2859279247a..4b192c149d0984dd68c3516b8ed0bbb31583b84e
@@@ -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;
      }
  
      if(absdir == NULL)
      {
          ParseError("NULL Chroot found\n");
++        free(logdir);
          return;
      }
  
      {
          ParseError("Can not chroot to \"%s\": absolute: %s: %s\n",
                     directory, absdir, get_error(errno));
++        free(logdir);
          return;
      }
  
      {
          ParseError("Can not chdir to \"/\" after chroot: %s\n",
                     get_error(errno));
++        free(logdir);
          return;
      }
  
      if(strncmp(absdir, logdir, strlen(absdir)))
      {
          ParseError("Absdir is not a subset of the logdir");
++        free(logdir);
          return;
      }
  
                              logdir, *logstore));
  
      LogMessage("Chroot directory = %s\n", directory);
++    free(logdir);
  }
  
  
index ac0660d437069599578720b4498626468091e5f6,ac0660d437069599578720b4498626468091e5f6..3cb53043fd3679124954be9f26753864378f5311
@@@ -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;