diff --git a/.github/workflows/scan-build.yml b/.github/workflows/scan-build.yml index 49691d2d1934..990f0fa97a9c 100644 --- a/.github/workflows/scan-build.yml +++ b/.github/workflows/scan-build.yml @@ -165,3 +165,63 @@ jobs: name: scan-build-results path: scan-build-report/ retention-days: 5 + gcc-analyzer: + name: GCC analyzer + runs-on: ubuntu-latest + container: ubuntu:26.04 + steps: + - name: Cache scan-build + uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb + with: + path: ~/.cargo + key: scan-build + + - name: Install system packages + run: | + apt update + apt -y install \ + libpcre2-dev \ + build-essential \ + autoconf \ + automake \ + cargo \ + cbindgen \ + dpdk-dev \ + gcc-16 \ + git \ + libtool \ + libpcap-dev \ + libnet1-dev \ + libyaml-0-2 \ + libyaml-dev \ + libcap-ng-dev \ + libcap-ng0 \ + libmagic-dev \ + libnetfilter-log-dev \ + libnetfilter-queue-dev \ + libnetfilter-queue1 \ + libnfnetlink-dev \ + libnfnetlink0 \ + libnuma-dev \ + libhiredis-dev \ + libhyperscan-dev \ + libjansson-dev \ + libevent-dev \ + libevent-pthreads-2.1-7 \ + liblz4-dev \ + make \ + python3-yaml \ + rustc \ + software-properties-common \ + zlib1g \ + zlib1g-dev + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + - run: git config --global --add safe.directory /__w/suricata/suricata + - run: ./scripts/bundle.sh + - run: ./autogen.sh + - run: ./configure --enable-warnings --enable-dpdk --enable-nfqueue --enable-nflog --enable-debug-validation + env: + CC: gcc-16 + CFLAGS: "-fanalyzer -Werror" + SURICATA_LUA_SYS_CFLAGS: "" + - run: make diff --git a/scripts/check-doc-rules.py b/scripts/check-doc-rules.py index e84338387de0..2785039c539f 100644 --- a/scripts/check-doc-rules.py +++ b/scripts/check-doc-rules.py @@ -111,9 +111,11 @@ def check_rule_with_suricata( rule: str, suricata_bin: Path, suricata_yaml: Path, + data_dir: Path, ) -> Tuple[bool, str]: with tempfile.TemporaryDirectory(prefix="doc-rule-check-") as tmpdir: rule_file = Path(tmpdir) / "rule.rules" + shutil.copytree(data_dir, tmpdir, dirs_exist_ok=True) rule_file.write_text(rule + "\n", encoding="utf-8") cmd = [ @@ -185,6 +187,7 @@ def main() -> int: rule, suricata_bin, suricata_yaml, + repo_root / "scripts" / "docrules" / "data", ) if not is_valid: print( diff --git a/scripts/docrules/data/blocked-domains.lst b/scripts/docrules/data/blocked-domains.lst new file mode 100644 index 000000000000..26a0127725a8 --- /dev/null +++ b/scripts/docrules/data/blocked-domains.lst @@ -0,0 +1 @@ +LmV2aWwuY29tCg== diff --git a/src/app-layer-frames.c b/src/app-layer-frames.c index 2d79fb463ab6..3962110414fa 100644 --- a/src/app-layer-frames.c +++ b/src/app-layer-frames.c @@ -233,6 +233,7 @@ static void FrameClean(Frame *frame) static void FrameCopy(Frame *dst, Frame *src) { + DEBUG_VALIDATE_BUG_ON(dst == src); memcpy(dst, src, sizeof(*dst)); } @@ -353,8 +354,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c #endif } else { Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -378,8 +379,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -780,8 +781,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) le = MIN(le, fle); SCLogDebug("le %" PRIu64 ", frame fle %" PRIu64, le, fle); Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; @@ -808,8 +809,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; diff --git a/src/conf.c b/src/conf.c index 9c0c3e0ad4eb..49659266d80c 100644 --- a/src/conf.c +++ b/src/conf.c @@ -161,6 +161,8 @@ void SCConfNodeFree(SCConfNode *node) while ((tmp = TAILQ_FIRST(&node->head))) { TAILQ_REMOVE(&node->head, tmp, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&node->head) == tmp); SCConfNodeFree(tmp); } diff --git a/src/decode-tcp.c b/src/decode-tcp.c index a7d5ee4b1686..f2cad13d06bf 100644 --- a/src/decode-tcp.c +++ b/src/decode-tcp.c @@ -79,8 +79,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) } tcp_opts[tcp_opt_cnt].type = type; - tcp_opts[tcp_opt_cnt].len = olen; - tcp_opts[tcp_opt_cnt].data = (olen > 2) ? (pkt+2) : NULL; + tcp_opts[tcp_opt_cnt].len = olen; /* we are parsing the most commonly used opts to prevent * us from having to walk the opts list for these all the @@ -90,6 +89,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_WS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.wscale_set != 0) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -107,6 +107,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_MSS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.mss_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -119,6 +120,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_SACKOK_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (TCP_GET_SACKOK(p)) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -130,6 +132,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_TS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.ts_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -149,6 +152,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !((olen - 2) % 8 == 0)) { ENGINE_SET_EVENT(p, TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.sack_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -166,6 +170,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !(((olen - 2) & 0x1) == 0))) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.tfo_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -178,6 +183,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) case TCP_OPT_EXP2: SCLogDebug("TCP EXP option, len %u", olen); if (olen == 4 || olen == 12) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); uint16_t magic = DecodeTCPGetU16(tcp_opts[tcp_opt_cnt].data); if (magic == 0xf989) { if (p->l4.vars.tcp.tfo_set) { @@ -196,6 +202,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != 18) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.md5_option_present = true; } @@ -206,10 +213,18 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen < 4) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.ao_option_present = true; } break; + default: + if (olen > 2) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); + } else { + + tcp_opts[tcp_opt_cnt].data = NULL; + } } pkt += olen; diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index ca9cda0ce0e7..038146a946b6 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -500,7 +500,10 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc } SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + ret = -1; + goto end; + } SCSigSignatureOrderingModuleCleanup(de_ctx); if (SCThresholdConfInitContext(de_ctx) < 0) { diff --git a/src/detect-engine-sigorder.c b/src/detect-engine-sigorder.c index 9447b527393c..98012cf9d3f2 100644 --- a/src/detect-engine-sigorder.c +++ b/src/detect-engine-sigorder.c @@ -799,13 +799,14 @@ static inline SCSigSignatureWrapper *SCSigAllocSignatureWrapper(Signature *sig) * \param de_ctx Pointer to the Detection Engine Context that holds the * signatures to be ordered */ -void SCSigOrderSignatures(DetectEngineCtx *de_ctx) +int SCSigOrderSignatures(DetectEngineCtx *de_ctx) { if (de_ctx->sig_list == NULL) { SCLogDebug("no signatures to order"); - return; + return 0; } + int retval = 0; SCLogDebug("ordering signatures in memory"); SCSigSignatureWrapper *sigw = NULL; SCSigSignatureWrapper *td_sigw_list = NULL; /* unified td list */ @@ -816,6 +817,12 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) Signature *sig = de_ctx->sig_list; while (sig != NULL) { sigw = SCSigAllocSignatureWrapper(sig); + if (sigw == NULL) { + SCLogError("failed to alloc signature wrapper for rule ordering"); + retval = -1; + goto cleanup; + } + /* Push signature wrapper onto a list, order doesn't matter here. */ if (sig->init_data->firewall_rule) { if (sig->type == SIG_TYPE_PKT) { @@ -851,6 +858,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) /* Recreate the sig list in order */ de_ctx->sig_list = NULL; +cleanup: /* firewall list for hook packet_filter */ for (sigw = fw_pf_sigw_list; sigw != NULL;) { SCLogDebug("post-sort packet_filter: sid %u", sigw->sig->id); @@ -901,6 +909,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) sigw = sigw->next; SCFree(sigw_to_free); } + return retval; } /** @@ -1059,7 +1068,7 @@ static int SCSigOrderingTest02(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1198,7 +1207,7 @@ static int SCSigOrderingTest03(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1313,7 +1322,7 @@ static int SCSigOrderingTest04(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1411,7 +1420,7 @@ static int SCSigOrderingTest05(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1500,7 +1509,7 @@ static int SCSigOrderingTest06(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1587,7 +1596,7 @@ static int SCSigOrderingTest07(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1687,7 +1696,7 @@ static int SCSigOrderingTest08(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1793,7 +1802,7 @@ static int SCSigOrderingTest09(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1897,7 +1906,7 @@ static int SCSigOrderingTest10(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1965,7 +1974,7 @@ static int SCSigOrderingTest11(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -2056,7 +2065,7 @@ static int SCSigOrderingTest13(void) FAIL_IF_NULL(sig); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowbitsCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); #ifdef DEBUG sig = de_ctx->sig_list; diff --git a/src/detect-engine-sigorder.h b/src/detect-engine-sigorder.h index d859846c629e..358aa275c1a2 100644 --- a/src/detect-engine-sigorder.h +++ b/src/detect-engine-sigorder.h @@ -24,7 +24,7 @@ #ifndef SURICATA_DETECT_ENGINE_SIGORDER_H #define SURICATA_DETECT_ENGINE_SIGORDER_H -void SCSigOrderSignatures(DetectEngineCtx *); +int WARN_UNUSED SCSigOrderSignatures(DetectEngineCtx *); void SCSigRegisterSignatureOrderingFuncs(DetectEngineCtx *); void SCSigRegisterSignatureOrderingTests(void); void SCSigSignatureOrderingModuleCleanup(DetectEngineCtx *); diff --git a/src/detect-flowint.c b/src/detect-flowint.c index d83a9529f570..efc105468349 100644 --- a/src/detect-flowint.c +++ b/src/detect-flowint.c @@ -1131,7 +1131,7 @@ static int DetectFlowintTestPacket01Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1207,7 +1207,7 @@ static int DetectFlowintTestPacket02Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1280,7 +1280,7 @@ static int DetectFlowintTestPacket03Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 3) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); diff --git a/src/detect-flowvar.c b/src/detect-flowvar.c index d5396742456f..e1e2397afa02 100644 --- a/src/detect-flowvar.c +++ b/src/detect-flowvar.c @@ -287,17 +287,16 @@ static int DetectFlowvarPostMatch( DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s, const SigMatchCtx *ctx) { - DetectVarList *fs, *prev; - const DetectFlowvarData *fd; - if (det_ctx->varlist == NULL) return 1; - fd = (const DetectFlowvarData *)ctx; + const DetectFlowvarData *fd = (const DetectFlowvarData *)ctx; + DetectVarList *prev = NULL; - prev = NULL; - fs = det_ctx->varlist; + DetectVarList *fs = det_ctx->varlist; while (fs != NULL) { + DetectVarList *next_fs = fs->next; + if (fd->idx == 0 || fd->idx == fs->idx) { SCLogDebug("adding to the flow %u:", fs->idx); //PrintRawDataFp(stdout, fs->buffer, fs->len); @@ -323,17 +322,16 @@ static int DetectFlowvarPostMatch( } if (fs == det_ctx->varlist) { - det_ctx->varlist = fs->next; SCFree(fs); - fs = det_ctx->varlist; + det_ctx->varlist = fs = next_fs; } else { - prev->next = fs->next; + DEBUG_VALIDATE_BUG_ON(prev == NULL); SCFree(fs); - fs = prev->next; + fs = prev->next = next_fs; } } else { prev = fs; - fs = fs->next; + fs = next_fs; } } return 1; diff --git a/src/detect-ipproto.c b/src/detect-ipproto.c index 7fc05b15e826..22b5624f346b 100644 --- a/src/detect-ipproto.c +++ b/src/detect-ipproto.c @@ -83,13 +83,6 @@ void DetectIPProtoRegister(void) */ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) { - DetectIPProtoData *data = NULL; - char *args[2] = { NULL, NULL }; - int res = 0; - size_t pcre2_len; - int i; - const char *str_ptr; - /* Execute the regex and populate args with captures. */ pcre2_match_data *match = NULL; int ret = DetectParsePcreExec(&parse_regex, &match, optstr, 0, 0); @@ -97,11 +90,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) SCLogError("pcre_exec parse error, ret" "%" PRId32 ", string %s", ret, optstr); - goto error; + if (match) { + pcre2_match_data_free(match); + } + return NULL; } - for (i = 0; i < (ret - 1); i++) { - res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); + char *args[2] = { NULL, NULL }; + DetectIPProtoData *data = NULL; + + for (int i = 0; i < 2; i++) { + const char *str_ptr = NULL; + size_t pcre2_len = 0; + int res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); if (res < 0) { SCLogError("pcre2_substring_get_bynumber failed"); goto error; @@ -110,7 +111,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) } /* Initialize the data */ - data = SCMalloc(sizeof(DetectIPProtoData)); + data = SCCalloc(1, sizeof(DetectIPProtoData)); if (unlikely(data == NULL)) goto error; data->op = DETECT_IPPROTO_OP_EQ; @@ -125,19 +126,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (!isdigit((unsigned char)*(args[1]))) { uint8_t proto; if (!SCGetProtoByName(args[1], &proto)) { - SCLogError("Unknown protocol name: \"%s\"", str_ptr); + SCLogError("Unknown protocol name: \"%s\"", args[1]); goto error; } data->proto = proto; } else { if (StringParseUint8(&data->proto, 10, 0, args[1]) <= 0) { - SCLogError("Malformed protocol number: %s", str_ptr); + SCLogError("Malformed protocol number: %s", args[1]); goto error; } } - for (i = 0; i < (ret - 1); i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } @@ -149,7 +150,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (match) { pcre2_match_data_free(match); } - for (i = 0; i < (ret - 1) && i < 2; i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } diff --git a/src/log-pcap.c b/src/log-pcap.c index 077fd9f6a481..7d6f3ccd018d 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -129,11 +129,11 @@ typedef struct PcapLogCompressionData_ { #ifdef HAVE_LIBLZ4 LZ4F_compressionContext_t lz4f_context; LZ4F_preferences_t lz4f_prefs; + FILE *pcap_buf_wrapper; #endif /* HAVE_LIBLZ4 */ FILE *file; uint8_t *pcap_buf; uint64_t pcap_buf_size; - FILE *pcap_buf_wrapper; uint64_t bytes_in_block; } PcapLogCompressionData; @@ -277,15 +277,7 @@ static int PcapLogCloseFile(ThreadVars *t, PcapLogData *pl) #ifdef HAVE_LIBLZ4 PcapLogCompressionData *comp = &pl->compression; if (comp->format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { - /* pcap_dump_close() has closed its output ``file'', - * so we need to call fmemopen again. */ - - comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, - comp->pcap_buf_size, "w"); - if (comp->pcap_buf_wrapper == NULL) { - SCLogError("SCFmemopen failed: %s", strerror(errno)); - return TM_ECODE_FAILED; - } + comp->pcap_buf_wrapper = NULL; } #endif /* HAVE_LIBLZ4 */ } @@ -367,6 +359,7 @@ static int PcapLogRotateFile(ThreadVars *t, PcapLogData *pl) } TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); pl->file_cnt--; } @@ -442,6 +435,12 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p) pl->fopen_err = 0; } + comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, comp->pcap_buf_size, "w"); + if (comp->pcap_buf_wrapper == NULL) { + fclose(comp->file); + comp->file = NULL; + return TM_ECODE_FAILED; + } if ((pl->pcap_dumper = pcap_dump_fopen(pl->pcap_dead_handle, comp->pcap_buf_wrapper)) == NULL) { if (!pl->pcap_open_err) { @@ -1015,6 +1014,7 @@ static TmEcode PcapLogInitRingBuffer(PcapLogData *pl) PcapFileName *pf = TAILQ_FIRST(&pl->pcap_file_list); while (pf != NULL && pl->file_cnt > pl->max_files) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); SCLogDebug("Removing PCAP file %s", pf->filename); if (remove(pf->filename) != 0) { @@ -1166,6 +1166,7 @@ static void PcapLogDataFree(PcapLogData *pl) PcapFileName *pf; while ((pf = TAILQ_FIRST(&pl->pcap_file_list)) != NULL) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); } if (pl == g_pcap_data) { @@ -1191,7 +1192,8 @@ static void PcapLogDataFree(PcapLogData *pl) #ifdef HAVE_LIBLZ4 if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { SCFree(pl->compression.buffer); - fclose(pl->compression.pcap_buf_wrapper); + if (pl->compression.pcap_buf_wrapper) + fclose(pl->compression.pcap_buf_wrapper); SCFree(pl->compression.pcap_buf); LZ4F_errorCode_t errcode = LZ4F_freeCompressionContext(pl->compression.lz4f_context); @@ -1217,7 +1219,7 @@ static TmEcode PcapLogDataDeinit(ThreadVars *t, void *thread_data) PcapLogData *pl = td->pcap_log; if (pl->pcap_dumper != NULL) { - if (PcapLogCloseFile(t,pl) < 0) { + if (PcapLogCloseFile(t, pl) != TM_ECODE_OK) { SCLogDebug("PcapLogCloseFile failed"); } } @@ -1482,7 +1484,9 @@ static OutputInitResult PcapLogInitCtx(SCConfNode *conf) comp->file = NULL; comp->pcap_buf = NULL; comp->pcap_buf_size = 0; +#ifdef HAVE_LIBLZ4 comp->pcap_buf_wrapper = NULL; +#endif } else if (strcmp(compression_str, "lz4") == 0) { #ifdef HAVE_LIBLZ4 pl->compression.format = PCAP_LOG_COMPRESSION_FORMAT_LZ4; @@ -1849,7 +1853,7 @@ static void FormatNumber(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fb", (float)num/1000000000UL); } -static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) +static void ProfileReportPair(FILE *fp, const char *name, const PcapLogProfileData *p) { char ticks_str[32] = "n/a"; char cnt_str[32] = "n/a"; @@ -1863,7 +1867,7 @@ static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) fprintf(fp, "%-28s %-10s %-10s %-10s\n", name, cnt_str, avg_str, ticks_str); } -static void ProfileReport(FILE *fp, PcapLogData *pl) +static void ProfileReport(FILE *fp, const PcapLogData *pl) { ProfileReportPair(fp, "open", &pl->profile_open); ProfileReportPair(fp, "close", &pl->profile_close); @@ -1886,23 +1890,8 @@ static void FormatBytes(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fGiB", (float)num/1000000000UL); } -static void PcapLogProfilingDump(PcapLogData *pl) +static void DoDump(const PcapLogData *pl, FILE *fp) { - FILE *fp = NULL; - - if (profiling_pcaplog_enabled == 0) - return; - - if (profiling_pcaplog_output_to_file == 1) { - fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); - if (fp == NULL) { - SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); - return; - } - } else { - fp = stdout; - } - /* counters */ fprintf(fp, "\n\nOperation Cnt Avg ticks Total ticks\n"); fprintf(fp, "---------------------------- ---------- ---------- -----------\n"); @@ -1942,9 +1931,24 @@ static void PcapLogProfilingDump(PcapLogData *pl) if (ticks_per_gib > 0) FormatNumber(ticks_per_gib, ticks_per_gib_str, sizeof(ticks_per_gib_str)); fprintf(fp, " Ticks per GiB: %s\n", ticks_per_gib_str); +} - if (fp != stdout) +static void PcapLogProfilingDump(PcapLogData *pl) +{ + if (profiling_pcaplog_enabled == 0) + return; + + if (profiling_pcaplog_output_to_file == 1) { + FILE *fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); + if (fp == NULL) { + SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); + return; + } + DoDump(pl, fp); fclose(fp); + } else { + DoDump(pl, stdout); + } } void PcapLogProfileSetup(void) diff --git a/src/output-filestore.c b/src/output-filestore.c index 84a68aa4e8fc..2453aeb2ab15 100644 --- a/src/output-filestore.c +++ b/src/output-filestore.c @@ -192,6 +192,9 @@ static void OutputFilestoreFinalizeFiles(ThreadVars *tv, const OutputFilestoreLo } } +/** + * \note `filestore_open_file_cnt` should only be used when FileGetMaxOpenFiles() is non-zero + */ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet *p, File *ff, void *tx, const uint64_t tx_id, const uint8_t *data, uint32_t data_len, uint8_t flags, uint8_t dir) @@ -201,6 +204,7 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet OutputFilestoreCtx *ctx = aft->ctx; char filename[PATH_MAX] = ""; int file_fd = -1; + bool close_file = false; SCLogDebug("ff %p, data %p, data_len %u", ff, data, data_len); @@ -218,18 +222,20 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet return -1; } - if (SC_ATOMIC_GET(filestore_open_file_cnt) < FileGetMaxOpenFiles()) { - SC_ATOMIC_ADD(filestore_open_file_cnt, 1); - ff->fd = file_fd; - } else { - if (FileGetMaxOpenFiles() > 0) { - StatsCounterIncr(&tv->stats, aft->counter_max_hits); - } + /* SC_ATOMIC_ADD returns value before the addition. */ + if (FileGetMaxOpenFiles() > 0 && + SC_ATOMIC_ADD(filestore_open_file_cnt, 1) >= FileGetMaxOpenFiles()) { + (void)SC_ATOMIC_SUB(filestore_open_file_cnt, 1); + StatsCounterIncr(&tv->stats, aft->counter_max_hits); ff->fd = -1; + close_file = true; /* not storing it, so need to close */ + } else { + ff->fd = file_fd; } /* we can get called with NULL data when we need to close */ } else if (data != NULL) { - if (ff->fd == -1) { + file_fd = ff->fd; + if (file_fd == -1) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -242,14 +248,15 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet strerror(errno)); return -1; } - } else { - file_fd = ff->fd; + close_file = true; /* close temporary open */ } + } else if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { + file_fd = ff->fd; } - if (file_fd != -1) { + if (file_fd != -1 && data != NULL && data_len > 0) { ssize_t r = write(file_fd, (const void *)data, (size_t)data_len); - if (r == -1) { + if (r == -1 || (ssize_t)data_len != r) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -257,22 +264,23 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet StatsCounterIncr(&tv->stats, aft->fs_error_counter); WARN_ONCE(WOT_WRITE, "Filestore (v2) failed to write to %s: %s", filename, strerror(errno)); - if (ff->fd != -1) { + close_file = true; /* close in the error case */ + } + } + + /* close the open file if needed */ + if (file_fd != -1 && ((flags & OUTPUT_FILEDATA_FLAG_CLOSE) != 0 || close_file)) { + /* if it was stored in the ff, disconnect */ + if (ff->fd != -1) { + if (FileGetMaxOpenFiles()) { SC_ATOMIC_SUB(filestore_open_file_cnt, 1); } ff->fd = -1; } - if (ff->fd == -1) { - close(file_fd); - } + close(file_fd); } if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { - if (ff->fd != -1) { - close(ff->fd); - ff->fd = -1; - SC_ATOMIC_SUB(filestore_open_file_cnt, 1); - } OutputFilestoreFinalizeFiles(tv, aft, ctx, p, ff, tx, tx_id, dir); } diff --git a/src/output.c b/src/output.c index e9f76f4e79d9..cdecfe519fdb 100644 --- a/src/output.c +++ b/src/output.c @@ -656,6 +656,7 @@ void OutputDeregisterAll(void) while ((module = TAILQ_FIRST(&output_modules))) { TAILQ_REMOVE(&output_modules, module, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&output_modules) == module); SCFree(module); } SCFree(simple_json_applayer_loggers); @@ -898,6 +899,7 @@ void OutputClearActiveLoggers(void) RootLogger *logger; while ((logger = TAILQ_FIRST(&active_loggers)) != NULL) { TAILQ_REMOVE(&active_loggers, logger, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&active_loggers) == logger); SCFree(logger); } } diff --git a/src/source-nfq.c b/src/source-nfq.c index 7f75cd171c1b..e664503b35f2 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -567,6 +567,7 @@ static int NFQCallBack(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg, if (ret == -1) { #ifdef COUNTERS NFQQueueVars *q = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(q == NULL); q->errs++; q->pkts++; q->bytes += GET_PKT_LEN(p); @@ -976,6 +977,7 @@ static void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv) { int ret; int flag = NFQVerdictCacheLen(t) ? MSG_DONTWAIT : 0; + DEBUG_VALIDATE_BUG_ON(t == NULL); int rv = recv(t->fd, tv->data, tv->datalen, flag); if (rv < 0) { @@ -1049,6 +1051,7 @@ void ReceiveNFQThreadExitStats(ThreadVars *tv, void *data) { NFQThreadVars *ntv = (NFQThreadVars *)data; NFQQueueVars *nq = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(nq == NULL); #ifdef COUNTERS SCLogNotice("(%s) Treated: Pkts %" PRIu32 ", Bytes %" PRIu64 ", Errors %" PRIu32 "", tv->name, nq->pkts, nq->bytes, nq->errs); diff --git a/src/tm-queues.c b/src/tm-queues.c index 81bbc3042b83..c36f97de4d4c 100644 --- a/src/tm-queues.c +++ b/src/tm-queues.c @@ -27,6 +27,7 @@ #include "threads.h" #include "tm-queues.h" #include "util-debug.h" +#include "util-validate.h" static TAILQ_HEAD(TmqList_, Tmq_) tmq_list = TAILQ_HEAD_INITIALIZER(tmq_list); @@ -83,6 +84,9 @@ void TmqResetQueues(void) while ((tmq = TAILQ_FIRST(&tmq_list))) { TAILQ_REMOVE(&tmq_list, tmq, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&tmq_list) == tmq); + if (tmq->name) { SCFree(tmq->name); } diff --git a/src/util-affinity.c b/src/util-affinity.c index cba5aec1d60a..9edd601c4f27 100644 --- a/src/util-affinity.c +++ b/src/util-affinity.c @@ -229,6 +229,7 @@ int BuildCpusetWithCallback( { SCConfNode *lnode; TAILQ_FOREACH(lnode, &node->head, next) { + char *sep = NULL; uint32_t i; uint32_t a, b; uint32_t stop = 0; @@ -240,8 +241,7 @@ int BuildCpusetWithCallback( a = 0; b = max; stop = 1; - } else if (strchr(lnode->val, '-') != NULL) { - char *sep = strchr(lnode->val, '-'); + } else if ((sep = strchr(lnode->val, '-')) != NULL) { if (StringParseUint32(&a, 10, sep - lnode->val, lnode->val) <= 0) { SCLogError("%s: invalid cpu range (start invalid): \"%s\"", name, lnode->val); return -1; diff --git a/src/util-mpm-hs.c b/src/util-mpm-hs.c index 3b77a4de44c5..a8b1e2d0ac22 100644 --- a/src/util-mpm-hs.c +++ b/src/util-mpm-hs.c @@ -775,7 +775,8 @@ int SCHSPreparePatterns(MpmConfig *mpm_conf, MpmCtx *mpm_ctx) } const char *cache_path = pd->no_cache || !mpm_conf ? NULL : mpm_conf->cache_dir_path; - if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0 && pd != NULL) { + if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0) { + DEBUG_VALIDATE_BUG_ON(pd == NULL); cd = NULL; ctx->pattern_db = pd; if (PatternDatabaseGetSize(pd, &ctx->hs_db_size) != 0) { diff --git a/src/util-spm-bm.c b/src/util-spm-bm.c index bfdb28600aa3..850662e42fa0 100644 --- a/src/util-spm-bm.c +++ b/src/util-spm-bm.c @@ -184,7 +184,7 @@ static void BoyerMooreSuffixes(const uint8_t *x, uint16_t m, uint16_t *suff) static int PreBmGs(const uint8_t *x, uint16_t m, uint16_t *bmGs) { int32_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixes(x, m, suff); @@ -260,7 +260,7 @@ static void BoyerMooreSuffixesNocase(const uint8_t *x, uint16_t m, static void PreBmGsNocase(const uint8_t *x, uint16_t m, uint16_t *bmGs) { uint16_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixesNocase(x, m, suff); diff --git a/src/util-unittest-helper.c b/src/util-unittest-helper.c index 8aa92b226f65..0d26588f19a9 100644 --- a/src/util-unittest-helper.c +++ b/src/util-unittest-helper.c @@ -738,7 +738,9 @@ int UTHMatchPackets(DetectEngineCtx *de_ctx, Packet **p, int num_packets) memset(&th_v, 0, sizeof(th_v)); StatsThreadInit(&th_v.stats); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + result = 0; + } SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); diff --git a/src/util-var-name.c b/src/util-var-name.c index a24c9f4874de..f62675950f09 100644 --- a/src/util-var-name.c +++ b/src/util-var-name.c @@ -127,6 +127,7 @@ void VarNameStoreDestroy(void) while ((s = TAILQ_FIRST(&free_list))) { TAILQ_REMOVE(&free_list, s, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&free_list) == s); HashListTableFree(s->names); HashListTableFree(s->ids); SCFree(s);