From: Russ Combs (rucombs) Date: Thu, 6 Oct 2016 22:29:13 +0000 (-0400) Subject: Merge pull request #662 in SNORT/snort3 from fixit_a to master X-Git-Tag: 3.0.0-233~234 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6ff6b91eb1461d9e5143bcb66c9fab08bcd2d583;p=thirdparty%2Fsnort3.git Merge pull request #662 in SNORT/snort3 from fixit_a to master Squashed commit of the following: commit 80dd0e30955432821d0f81f673951ee238b5c303 Author: Russ Combs (rucombs) Date: Thu Oct 6 18:04:27 2016 -0400 add FIXIT-A for unresolved static analysis issues --- diff --git a/src/loggers/alert_luajit.cc b/src/loggers/alert_luajit.cc index 60ba20d40..eaab01605 100644 --- a/src/loggers/alert_luajit.cc +++ b/src/loggers/alert_luajit.cc @@ -18,7 +18,7 @@ // alert_luajit.cc author Russ Combs #include -#include +#include // FIXIT-A Returning null reference (somewhere below) #include #include "main/snort_types.h" diff --git a/src/network_inspectors/appid/service_plugins/service_base.cc b/src/network_inspectors/appid/service_plugins/service_base.cc index 9d2ec6671..10e386a4f 100644 --- a/src/network_inspectors/appid/service_plugins/service_base.cc +++ b/src/network_inspectors/appid/service_plugins/service_base.cc @@ -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); diff --git a/src/network_inspectors/perf_monitor/base_tracker.cc b/src/network_inspectors/perf_monitor/base_tracker.cc index 8aefd9e0c..38aba7536 100644 --- a/src/network_inspectors/perf_monitor/base_tracker.cc +++ b/src/network_inspectors/perf_monitor/base_tracker.cc @@ -18,7 +18,7 @@ // base_tracker.cc author Carter Waxman -#include "base_tracker.h" +#include "base_tracker.h" // FIXIT-A Returning null reference (from ) #include "perf_module.h" #include "framework/module.h" diff --git a/src/profiler/rule_profiler.cc b/src/profiler/rule_profiler.cc index 1d6eaf514..603684ab1 100644 --- a/src/profiler/rule_profiler.cc +++ b/src/profiler/rule_profiler.cc @@ -30,7 +30,12 @@ #include #include -#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" diff --git a/src/service_inspectors/dce_rpc/dce_smb.h b/src/service_inspectors/dce_rpc/dce_smb.h index 53802955f..ea4607613 100644 --- a/src/service_inspectors/dce_rpc/dce_smb.h +++ b/src/service_inspectors/dce_rpc/dce_smb.h @@ -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; diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.cc b/src/service_inspectors/dce_rpc/dce_smb_utils.cc index 61542329a..ed62c3949 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_utils.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_utils.cc @@ -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; } diff --git a/src/service_inspectors/sip/sip_dialog.cc b/src/service_inspectors/sip/sip_dialog.cc index 2ebbe9fa7..c098761b5 100644 --- a/src/service_inspectors/sip/sip_dialog.cc +++ b/src/service_inspectors/sip/sip_dialog.cc @@ -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; } diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc index c96501cd6..2715e5d54 100644 --- a/src/stream/ip/ip_defrag.cc +++ b/src/stream/ip/ip_defrag.cc @@ -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;