diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000000..c503a6b4f0a1 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,32 @@ +--- +Checks: " + bugprone-*, + cert-*, + clang-analyzer-*, + concurrency-*, + misc-*, + modernize-*, + performance-*, + portability-*, + readability-*, + -readability-identifier-length, + -readability-braces-around-statements, + -readability-avoid-const-params-in-decls, + -bugprone-easily-swappable-parameters, + -misc-include-cleaner, +" +WarningsAsErrors: " + bugprone-unsafe-functions, + clang-analyzer-unix.Malloc, +" +HeaderFileExtensions: + - '' + - 'h' +ImplementationFileExtensions: + - 'c' +FormatStyle: 'file' +CheckOptions: + bugprone-unsafe-functions.ReportDefaultFunctions: 'true' + bugprone-unsafe-functions.ReportMoreUnsafeFunctions: 'true' + bugprone-unsafe-functions.CustomFunctions: '^strtok$,strtok_r;^sprintf$,snprintf;^strcat$,strlcat;^strcpy$,strlcpy;^strncpy$,strlcat;^strncat$,strlcpy;^strndup$,,is OS specific;^strchrnul$;^rand$;^rand_r$;^index$;^rindex$;^bzero$,^memset$' + readability-function-cognitive-complexity.Threshold: 50 diff --git a/.gitignore b/.gitignore index 66416e27d14e..f7e183ff1989 100644 --- a/.gitignore +++ b/.gitignore @@ -69,3 +69,4 @@ test.sh /libsuricata-config !/libsuricata-config.in !/.readthedocs.yaml +compile_commands.json diff --git a/configure.ac b/configure.ac index ca964d9039a0..0623de52446c 100644 --- a/configure.ac +++ b/configure.ac @@ -224,6 +224,11 @@ [], [ #include ]) + AC_CHECK_DECL([quick_exit], + AC_DEFINE([HAVE_QUICK_EXIT], [1], [Use quick_exit]), + [], [ + #include + ]) AC_CHECK_HEADERS([malloc.h]) AC_CHECK_DECL([malloc_trim], diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index a6a1f632fcb4..4f8486086453 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -111,7 +111,7 @@ uint32_t ftp_max_line_len = 4096; SC_ATOMIC_DECLARE(uint64_t, ftp_memuse); SC_ATOMIC_DECLARE(uint64_t, ftp_memcap); -static FTPTransaction *FTPGetOldestTx(const FtpState *, FTPTransaction *); +static FTPTransaction *FTPGetOldestTx(const FtpState * /*ftp_state*/, FTPTransaction * /*starttx*/); static void FTPParseMemcap(void) { @@ -375,7 +375,8 @@ static AppLayerResult FTPGetLineForDirection( SCReturnStruct(APP_LAYER_OK); } SCReturnStruct(APP_LAYER_INCOMPLETE(input->consumed, input->len + 1)); - } else if (*current_line_truncated) { + } + if (*current_line_truncated) { // Whatever came in with first LF should also get discarded *current_line_truncated = false; line->len = 0; @@ -507,7 +508,8 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt if (input == NULL && AppLayerParserStateIssetFlag(pstate, APP_LAYER_PARSER_EOF_TS)) { SCReturnStruct(APP_LAYER_OK); - } else if (input == NULL || input_len == 0) { + } + if (input == NULL || input_len == 0) { SCReturnStruct(APP_LAYER_ERROR); } @@ -520,7 +522,8 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt res = FTPGetLineForDirection(&line, &ftpi, &state->current_line_truncated_ts); if (res.status == 1) { return res; - } else if (res.status == -1) { + } + if (res.status == -1) { break; } const FtpCommand *cmd_descriptor; @@ -617,11 +620,9 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state, AppLayerParserSt FtpTransferCmdFree(data); SCLogDebug("No expectation created."); SCReturnStruct(APP_LAYER_ERROR); - } else { - SCLogDebug("Expectation created [direction: %s, dynamic port %"PRIu16"].", - state->active ? "to server" : "to client", - state->dyn_port); } + SCLogDebug("Expectation created [direction: %s, dynamic port %" PRIu16 "].", + state->active ? "to server" : "to client", state->dyn_port); /* reset the dyn port to avoid duplicate */ state->dyn_port = 0; @@ -713,7 +714,8 @@ static AppLayerResult FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserS res = FTPGetLineForDirection(&line, &ftpi, &state->current_line_truncated_tc); if (res.status == 1) { return res; - } else if (res.status == -1) { + } + if (res.status == -1) { break; } FTPTransaction *tx = FTPGetOldestTx(state, lasttx); @@ -929,7 +931,7 @@ static void FTPStateTransactionFree(void *state, uint64_t tx_id) TAILQ_FOREACH(tx, &ftp_state->tx_list, next) { if (tx_id < tx->tx_id) break; - else if (tx_id > tx->tx_id) + if (tx_id > tx->tx_id) continue; if (tx == ftp_state->curr_tx) @@ -1046,7 +1048,7 @@ static StreamingBufferConfig sbcfg = STREAMING_BUFFER_CONFIG_INITIALIZER; * \retval 1 when the command is parsed, 0 otherwise */ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, - AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data, uint8_t direction) + AppLayerParserState *pstate, StreamSlice stream_slice, uint8_t direction) { const uint8_t *input = StreamSliceGetData(&stream_slice); uint32_t input_len = StreamSliceGetDataLen(&stream_slice); @@ -1116,10 +1118,8 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, /* open with fixed track_id 0 as we can have just one * file per ftp-data flow. */ - if (FileOpenFileWithId(ftpdata_state->files, &sbcfg, - 0ULL, (uint8_t *) ftpdata_state->file_name, - ftpdata_state->file_len, - input, input_len, flags) != 0) { + if (FileOpenFileWithId(ftpdata_state->files, &sbcfg, 0ULL, ftpdata_state->file_name, + ftpdata_state->file_len, input, input_len, flags) != 0) { SCLogDebug("Can't open file"); ret = -1; } @@ -1171,13 +1171,13 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, static AppLayerResult FTPDataParseRequest(Flow *f, void *ftp_state, AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data) { - return FTPDataParse(f, ftp_state, pstate, stream_slice, local_data, STREAM_TOSERVER); + return FTPDataParse(f, ftp_state, pstate, stream_slice, STREAM_TOSERVER); } static AppLayerResult FTPDataParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstate, StreamSlice stream_slice, void *local_data) { - return FTPDataParse(f, ftp_state, pstate, stream_slice, local_data, STREAM_TOCLIENT); + return FTPDataParse(f, ftp_state, pstate, stream_slice, STREAM_TOCLIENT); } #ifdef DEBUG @@ -1260,8 +1260,7 @@ static int FTPDataGetAlstateProgress(void *tx, uint8_t direction) FtpDataState *ftpdata_state = (FtpDataState *)tx; if (direction == ftpdata_state->direction) return ftpdata_state->state; - else - return FTPDATA_STATE_FINISHED; + return FTPDATA_STATE_FINISHED; } static AppLayerGetFileState FTPDataStateGetTxFiles(void *tx, uint8_t direction) diff --git a/src/detect-http2.c b/src/detect-http2.c index 4d954a5ac96a..6986044d684f 100644 --- a/src/detect-http2.c +++ b/src/detect-http2.c @@ -26,19 +26,11 @@ #include "detect.h" #include "detect-parse.h" -#include "detect-content.h" - #include "detect-engine.h" #include "detect-engine-uint.h" -#include "detect-engine-mpm.h" -#include "detect-engine-prefilter.h" -#include "detect-engine-content-inspection.h" #include "detect-engine-helper.h" - #include "detect-http2.h" #include "util-byte.h" -#include "rust.h" -#include "util-profiling.h" #ifdef UNITTESTS void DetectHTTP2frameTypeRegisterTests (void); @@ -53,38 +45,44 @@ void DetectHTTP2sizeUpdateRegisterTests (void); static int DetectHTTP2frametypeMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2frametypeSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2frametypeFree (DetectEngineCtx *, void *); +static int DetectHTTP2frametypeSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2frametypeFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2errorcodeMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2errorcodeSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2errorcodeFree (DetectEngineCtx *, void *); +static int DetectHTTP2errorcodeSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2errorcodeFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2priorityMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2prioritySetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2priorityFree (DetectEngineCtx *, void *); +static int DetectHTTP2prioritySetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2priorityFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2windowMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2windowSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2windowFree (DetectEngineCtx *, void *); +static int DetectHTTP2windowSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2windowFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2sizeUpdateMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2sizeUpdateSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2sizeUpdateFree (DetectEngineCtx *, void *); +static int DetectHTTP2sizeUpdateSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2sizeUpdateFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2settingsMatch(DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, const Signature *s, const SigMatchCtx *ctx); -static int DetectHTTP2settingsSetup (DetectEngineCtx *, Signature *, const char *); -void DetectHTTP2settingsFree (DetectEngineCtx *, void *); +static int DetectHTTP2settingsSetup( + DetectEngineCtx * /*de_ctx*/, Signature * /*s*/, const char * /*str*/); +void DetectHTTP2settingsFree(DetectEngineCtx * /*de_ctx*/, void * /*ptr*/); static int DetectHTTP2headerNameSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg); @@ -597,5 +595,5 @@ static int DetectHTTP2headerNameSetup(DetectEngineCtx *de_ctx, Signature *s, con } #ifdef UNITTESTS -#include "tests/detect-http2.c" +#include "tests/detect-http2.c" // NOLINT(bugprone-suspicious-include) #endif diff --git a/src/reputation.c b/src/reputation.c index 417b60a52480..9a25fef01381 100644 --- a/src/reputation.c +++ b/src/reputation.c @@ -297,7 +297,8 @@ static int SRepSplitLine(SRepCIDRTree *cidr_ctx, char *line, Address *ip, uint8_ if (strcmp(ptrs[0], "ip") == 0) return 1; - uint8_t c, v; + uint8_t c; + uint8_t v; if (StringParseU8RangeCheck(&c, 10, 0, (const char *)ptrs[1], 0, SREP_MAX_CATS - 1) < 0) return -1; @@ -307,19 +308,18 @@ static int SRepSplitLine(SRepCIDRTree *cidr_ctx, char *line, Address *ip, uint8_ if (strchr(ptrs[0], '/') != NULL) { SRepCIDRAddNetblock(cidr_ctx, ptrs[0], c, v); return 1; + } + if (inet_pton(AF_INET, ptrs[0], &ip->address) == 1) { + ip->family = AF_INET; + } else if (inet_pton(AF_INET6, ptrs[0], &ip->address) == 1) { + ip->family = AF_INET6; } else { - if (inet_pton(AF_INET, ptrs[0], &ip->address) == 1) { - ip->family = AF_INET; - } else if (inet_pton(AF_INET6, ptrs[0], &ip->address) == 1) { - ip->family = AF_INET6; - } else { - return -1; - } - - *cat = c; - *value = v; + return -1; } + *cat = c; + *value = v; + return 0; } @@ -448,7 +448,8 @@ int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp) memset(&a, 0x00, sizeof(a)); a.family = AF_INET; - uint8_t cat = 0, value = 0; + uint8_t cat = 0; + uint8_t value = 0; int r = SRepSplitLine(cidr_ctx, line, &a, &cat, &value); if (r < 0) { SCLogError("bad line \"%s\"", line); @@ -467,45 +468,42 @@ int SRepLoadFileFromFD(SRepCIDRTree *cidr_ctx, FILE *fp) if (h == NULL) { SCLogError("failed to get a host, increase host.memcap"); break; - } else { - //SCLogInfo("host %p", h); + } // SCLogInfo("host %p", h); - if (h->iprep == NULL) { - h->iprep = SCCalloc(1, sizeof(SReputation)); - if (h->iprep != NULL) { - HostIncrUsecnt(h); - } - } + if (h->iprep == NULL) { + h->iprep = SCCalloc(1, sizeof(SReputation)); if (h->iprep != NULL) { - SReputation *rep = h->iprep; + HostIncrUsecnt(h); + } + } + if (h->iprep != NULL) { + SReputation *rep = h->iprep; - /* if version is outdated, it's an older entry that we'll - * now replace. */ - if (rep->version != SRepGetVersion()) { - memset(rep, 0x00, sizeof(SReputation)); - } + /* if version is outdated, it's an older entry that we'll + * now replace. */ + if (rep->version != SRepGetVersion()) { + memset(rep, 0x00, sizeof(SReputation)); + } - rep->version = SRepGetVersion(); - rep->rep[cat] = value; + rep->version = SRepGetVersion(); + rep->rep[cat] = value; - SCLogDebug("host %p iprep %p setting cat %u to value %u", - h, h->iprep, cat, value); + SCLogDebug("host %p iprep %p setting cat %u to value %u", h, h->iprep, cat, value); #ifdef DEBUG - if (SCLogDebugEnabled()) { - int i; - for (i = 0; i < SREP_MAX_CATS; i++) { - if (rep->rep[i] == 0) - continue; - - SCLogDebug("--> host %p iprep %p cat %d to value %u", - h, h->iprep, i, rep->rep[i]); - } + if (SCLogDebugEnabled()) { + int i; + for (i = 0; i < SREP_MAX_CATS; i++) { + if (rep->rep[i] == 0) + continue; + + SCLogDebug("--> host %p iprep %p cat %d to value %u", h, h->iprep, i, + rep->rep[i]); } -#endif } - - HostRelease(h); +#endif } + + HostRelease(h); } } diff --git a/src/source-af-packet.c b/src/source-af-packet.c index e3c83da49de4..10213c573879 100644 --- a/src/source-af-packet.c +++ b/src/source-af-packet.c @@ -1849,21 +1849,21 @@ static int SockFanoutSeteBPF(AFPThreadVars *ptv) return 0; } -static int SetEbpfFilter(AFPThreadVars *ptv) +static TmEcode SetEbpfFilter(AFPThreadVars *ptv) { int pfd = ptv->ebpf_filter_fd; if (pfd == -1) { SCLogError("Filter file descriptor is invalid"); - return -1; + return TM_ECODE_FAILED; } if (setsockopt(ptv->socket, SOL_SOCKET, SO_ATTACH_BPF, &pfd, sizeof(pfd))) { SCLogError("Error setting ebpf: %s", strerror(errno)); - return -1; + return TM_ECODE_FAILED; } SCLogInfo("Activated eBPF filter on socket"); - return 0; + return TM_ECODE_OK; } #endif diff --git a/src/util-debug.h b/src/util-debug.h index cb22e9097389..4fba832ea652 100644 --- a/src/util-debug.h +++ b/src/util-debug.h @@ -499,10 +499,14 @@ void SCLogErr(int x, const char *file, const char *func, const int line, const c #endif /* DEBUG */ +#if !HAVE_QUICK_EXIT +#define quick_exit exit +#endif + #define FatalError(...) \ do { \ SCLogError(__VA_ARGS__); \ - exit(EXIT_FAILURE); \ + quick_exit(EXIT_FAILURE); \ } while (0) /** \brief Fatal error IF we're starting up, and configured to consider @@ -515,7 +519,7 @@ void SCLogErr(int x, const char *file, const char *func, const int line, const c (void)ConfGetBool("engine.init-failure-fatal", &init_errors_fatal); \ if (init_errors_fatal && (SC_ATOMIC_GET(engine_stage) == SURICATA_INIT)) { \ SCLogError(__VA_ARGS__); \ - exit(EXIT_FAILURE); \ + quick_exit(EXIT_FAILURE); \ } \ SCLogWarning(__VA_ARGS__); \ } while (0) diff --git a/src/util-ebpf.c b/src/util-ebpf.c index ea0811b89a15..8fe378591a7a 100644 --- a/src/util-ebpf.c +++ b/src/util-ebpf.c @@ -280,6 +280,7 @@ static int EBPFLoadPinnedMaps(LiveDevice *livedev, struct ebpf_timeout_config *c } bpf_map_data->last = 0; SCLogError("Can't allocate bpf map name"); + SCFree(bpf_map_data); return -1; } diff --git a/src/util-lua-sandbox.c b/src/util-lua-sandbox.c index c3596f97c543..4bdc2f034a51 100644 --- a/src/util-lua-sandbox.c +++ b/src/util-lua-sandbox.c @@ -26,7 +26,6 @@ #include "lua.h" #include "lauxlib.h" #include "lualib.h" -#include "util-debug.h" #include "util-debug.h" #include "util-validate.h" diff --git a/src/util-memcmp.h b/src/util-memcmp.h index f9fc75357c17..578dd752d4bd 100644 --- a/src/util-memcmp.h +++ b/src/util-memcmp.h @@ -29,13 +29,14 @@ #ifndef SURICATA_UTIL_MEMCMP_H #define SURICATA_UTIL_MEMCMP_H +// NOLINTNEXTLINE(misc-header-include-cycle) #include "suricata-common.h" #include "util-optimize.h" /** \brief compare two patterns, converting the 2nd to lowercase * \warning *ONLY* the 2nd pattern is converted to lowercase */ -static inline int SCMemcmpLowercase(const void *, const void *, size_t); +static inline int SCMemcmpLowercase(const void *s1, const void *s2, size_t n); void MemcmpRegisterTests(void);