]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
Merge pull request #662 in SNORT/snort3 from fixit_a to master
authorRuss Combs (rucombs) <rucombs@cisco.com>
Thu, 6 Oct 2016 22:29:13 +0000 (18:29 -0400)
committerRuss Combs (rucombs) <rucombs@cisco.com>
Thu, 6 Oct 2016 22:29:13 +0000 (18:29 -0400)
Squashed commit of the following:

commit 80dd0e30955432821d0f81f673951ee238b5c303
Author: Russ Combs (rucombs) <rucombs@cisco.com>
Date:   Thu Oct 6 18:04:27 2016 -0400

    add FIXIT-A for unresolved static analysis issues

src/loggers/alert_luajit.cc
src/network_inspectors/appid/service_plugins/service_base.cc
src/network_inspectors/perf_monitor/base_tracker.cc
src/profiler/rule_profiler.cc
src/service_inspectors/dce_rpc/dce_smb.h
src/service_inspectors/dce_rpc/dce_smb_utils.cc
src/service_inspectors/sip/sip_dialog.cc
src/stream/ip/ip_defrag.cc

index 60ba20d400fef3f1d0f6331be8512e66b060def1..eaab01605a5988ea6b6deaa716b2ad434f22435f 100644 (file)
@@ -18,7 +18,7 @@
 // alert_luajit.cc author Russ Combs <rucombs@cisco.com>
 
 #include <assert.h>
-#include <vector>
+#include <vector>  // FIXIT-A Returning null reference (somewhere below)
 #include <lua.hpp>
 
 #include "main/snort_types.h"
index 9d2ec66718e9e1d7b3464da34f07b897dc32b000..10e386a4fee48664e124c1c78429355ff04601f3 100644 (file)
@@ -986,12 +986,10 @@ static inline RNAServiceElement* AppIdGetServiceByPattern(const Packet* pkt, IpP
             ServiceMatch** tmp;
             smOrderedListSize *= 2;
             assert(smOrderedListSize > 0);
-            // FIXIT-A - Even with the assert() on the previous line, the clang 
-            //          static analyser version 3.4.2 throws a false positive
-            //          realloc() zero size error,  More recent clang versions
-            //          do NOT find an error.
+
             tmp = (ServiceMatch**)realloc(smOrderedList,
                     smOrderedListSize * sizeof(*smOrderedList));
+
             if (!tmp)
             {
                 ErrorMessage("Realloc failure %u\n",smOrderedListSize);
index 8aefd9e0c2c6dfade362b57081838c98321291f5..38aba7536263b025a9ed09b9e604b8ad09a5e3e8 100644 (file)
@@ -18,7 +18,7 @@
 
 // base_tracker.cc author Carter Waxman <cwaxman@cisco.com>
 
-#include "base_tracker.h"
+#include "base_tracker.h"  // FIXIT-A Returning null reference (from <vector>)
 #include "perf_module.h"
 
 #include "framework/module.h"
index 1d6eaf5149bc25a2ae1caf10d6d2da9ff6314e1d..603684ab1db6931454979d74453c79e02c0997aa 100644 (file)
 #include <sstream>
 #include <vector>
 
-#include "detection/detection_options.h"
+// this include eventually leads to possible issues with std::chrono:
+// 1.  Undefined or garbage value returned to caller (rep count())
+// 2.  The left expression of the compound assignment is an uninitialized value.
+//     The computed value will also be garbage (duration& operator+=(const duration& __d))
+#include "detection/detection_options.h"  // ... FIXIT-A
+
 #include "detection/treenodes.h"
 #include "hash/sfghash.h"
 #include "main/snort_config.h"
index 53802955f4c69c925e425b41876d961abe327a4d..ea46076138f78cec158bb0ce68e0423dd327198d 100644 (file)
@@ -818,7 +818,8 @@ struct SmbAndXCommon
 inline uint32_t NbssLen(const NbssHdr* nb)
 {
     /* Treat first bit of flags as the upper byte to length */
-    return ((nb->flags & 0x01) << 16) | ntohs(nb->length);
+    // The left operand of '&' is a garbage value
+    return ((nb->flags & 0x01) << 16) | ntohs(nb->length);  // ... FIXIT-A
 }
 
 inline uint8_t NbssType(const NbssHdr* nb)
@@ -853,7 +854,9 @@ inline uint16_t SmbEmptyComBcc(const SmbEmptyCom* ec)
 
 inline int SmbType(const SmbNtHdr* hdr)
 {
-    if (hdr->smb_flg & SMB_FLG__TYPE)
+    // Access to field 'smb_flg' results in a dereference of a null pointer
+    // (loaded from variable 'hdr')
+    if (hdr->smb_flg & SMB_FLG__TYPE)  // ... FIXIT-A
         return SMB_TYPE__RESPONSE;
 
     return SMB_TYPE__REQUEST;
index 61542329aa912dea5334aef33c518bab0e31ca61..ed62c3949cbe1cba18c3fd7e77970b0c6edb52c4 100644 (file)
@@ -1971,11 +1971,6 @@ static void DCE2_SmbSetNewFileAPIFileTracker(DCE2_SmbSsnData* ssd)
         else
             ftracker = (DCE2_SmbFileTracker*)DCE2_ListNext(ssd->ftrackers);
     }
-
-    // FIXIT-A - Even with the assert(ssd) a few lines prior, the clang 
-    //          static analyser version 3.4.2 throws a false positive
-    //          null pointer dereference error,  More recent clang versions
-    //          do NOT find an error.
     ssd->fapi_ftracker = ftracker;
 }
 
index 2ebbe9fa7a966d33a1597b598bcd617e3d26c6b5..c098761b50bbc1b3451667685bed0b70e6d087a2 100644 (file)
@@ -79,7 +79,8 @@ static int SIP_processRequest(SIPMsg* sipMsg, SIP_DialogData* dialog, SIP_Dialog
     if ((NULL == dialog)&&(SIP_METHOD_CANCEL != sipMsg->methodFlag))
     {
         // Clang analyzer is false positive, dlist->head is updated after free
-        dialog = SIP_addDialog(sipMsg, dList->head, dList); // FIXIT-A
+        // (Use of memory after it is freed)
+        dialog = SIP_addDialog(sipMsg, dList->head, dList); // ... FIXIT-A
     }
 
     methodFlag = sipMsg->methodFlag;
@@ -649,7 +650,7 @@ static int SIP_deleteDialog(SIP_DialogData* currDialog, SIP_DialogList* dList)
     return true;
 }
 
-// TO-DO: Appid related. Publish event for appid
+// FIXIT-H Publish event for appid
 #if 0
 /*********************************************************************
  * Update appId sip detector with parsed SIP message and dialog
@@ -764,6 +765,7 @@ int SIP_updateDialog(SIPMsg* sipMsg, SIP_DialogList* dList, Packet* p, SIP_PROTO
     else
         ret = false;
 
+// FIXIT-H Publish event for appid
 #if 0
     for (dialog = dList->head;
         dialog;
@@ -772,8 +774,8 @@ int SIP_updateDialog(SIPMsg* sipMsg, SIP_DialogList* dList, Packet* p, SIP_PROTO
         if (sipMsg->dlgID.callIdHash == dialog->dlgID.callIdHash)
             break;
     }
+    sip_update_appid(p, sipMsg, dialog);
 #endif
-    //sip_update_appid(p, sipMsg, dialog);
 
     return ret;
 }
index c96501cd6733a3f3cb3be2f8adc9e3df1db58ebd..2715e5d54ca9026bb2b50193168dc6981d816e5e 100644 (file)
@@ -933,7 +933,7 @@ static inline void add_node(FragTracker* ft, Fragment* prev,
     {
         node->next = ft->fraglist;
         if (node->next)
-            node->next->prev = node;
+            node->next->prev = node;  // FIXIT-A Use of memory after it is freed
         else
             ft->fraglist_tail = node;
         ft->fraglist = node;