From 3cff1a566a5aa1d4cf4eaf262142590ca1ffd512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Heged=C3=BCs?= Date: Thu, 9 Jul 2026 17:06:55 +0200 Subject: [PATCH 1/2] clean cppcheck warnings in the iis/ ext/ and mlogc/ directories --- Makefile.am | 5 +- ext/mod_op_strstr.c | 10 +- ext/mod_var_remote_addr_port.c | 6 +- iis/main.cpp | 3 +- iis/moduleconfig.cpp | 16 +++- iis/moduleconfig.h | 3 +- iis/mymodule.cpp | 169 +++++++++++++++++---------------- mlogc/mlogc.c | 50 +++++----- standalone/api.c | 4 +- standalone/api.h | 4 +- 10 files changed, 143 insertions(+), 127 deletions(-) diff --git a/Makefile.am b/Makefile.am index 24d9db0a50..a95cfb8ee6 100755 --- a/Makefile.am +++ b/Makefile.am @@ -55,7 +55,10 @@ cppcheck: --inconclusive \ --template="warning: {file},{line},{severity},{id},{message}" \ --error-exitcode=1 \ - standalone/ + standalone/ \ + mlogc/ \ + iis/ \ + ext/ check-static: cppcheck diff --git a/ext/mod_op_strstr.c b/ext/mod_op_strstr.c index 53c5138a55..f1ddc30110 100644 --- a/ext/mod_op_strstr.c +++ b/ext/mod_op_strstr.c @@ -62,7 +62,7 @@ static int op_strstr_init(msre_rule *rule, char **error_msg) { /** * Operator execution entry point. */ -static int op_strstr_exec(modsec_rec *msr, msre_rule *rule, msre_var *var, char **error_msg) { +static int op_strstr_exec(modsec_rec const *msr, msre_rule const *rule, msre_var const *var, char **error_msg) { /* Here we need to inspect the contents of the supplied variable. */ /* In a general case it is possible for the value @@ -169,14 +169,12 @@ static void initBoyerMooreHorspool(const char *pattern, int patlength, } static int BoyerMooreHorspool(const char *pattern, int patlength, - const char *text, int textlen, int *bm_badcharacter_array) + const char *text, int textlen, int const *bm_badcharacter_array) { - int j; - char c; + int j = 0; - j = 0; while (j <= textlen - patlength) { - c = text[j + patlength - 1]; + char c = text[j + patlength - 1]; if (pattern[patlength - 1] == c && memcmp(pattern, text + j, patlength - 1) == 0) { return j; } diff --git a/ext/mod_var_remote_addr_port.c b/ext/mod_var_remote_addr_port.c index 4bd81ee9db..b21b684a62 100644 --- a/ext/mod_var_remote_addr_port.c +++ b/ext/mod_var_remote_addr_port.c @@ -27,7 +27,7 @@ /** * Generates a variable from a string and a length. */ -static int var_simple_generate_ex(msre_var *var, apr_table_t *vartab, apr_pool_t *mptmp, +static int var_simple_generate_ex(msre_var const *var, apr_table_t *vartab, apr_pool_t *mptmp, const char *value, int value_len) { msre_var *rvar = NULL; @@ -45,7 +45,7 @@ static int var_simple_generate_ex(msre_var *var, apr_table_t *vartab, apr_pool_t /** * Generates a variable from a NULL-terminated string. */ -static int var_simple_generate(msre_var *var, apr_table_t *vartab, apr_pool_t *mptmp, +static int var_simple_generate(msre_var const *var, apr_table_t *vartab, apr_pool_t *mptmp, const char *value) { if (value == NULL) return 0; @@ -58,7 +58,7 @@ static int var_simple_generate(msre_var *var, apr_table_t *vartab, apr_pool_t *m /** * Create a silly variable with value = a.b.c.d:port */ -static int var_remote_addr_port_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, +static int var_remote_addr_port_generate(modsec_rec *msr, msre_var const *var, msre_rule *rule, apr_table_t *vartab, apr_pool_t *mptmp) { const char *value = apr_psprintf(mptmp, "%s:%d", msr->remote_addr, msr->remote_port); diff --git a/iis/main.cpp b/iis/main.cpp index f761609a40..6afeb88d27 100644 --- a/iis/main.cpp +++ b/iis/main.cpp @@ -85,7 +85,8 @@ RegisterModule( } hr = pModuleInfo->SetPriorityForRequestNotification(RQ_BEGIN_REQUEST, PRIORITY_ALIAS_FIRST); - hr = pModuleInfo->SetPriorityForRequestNotification(RQ_SEND_RESPONSE, PRIORITY_ALIAS_LAST); // reverted! + hr = pModuleInfo->SetPriorityForRequestNotification(RQ_SEND_RESPONSE, PRIORITY_ALIAS_LAST); // cppcheck-suppress redundantAssignment + // reverted! //hr = pModuleInfo2->SetPriorityForPostRequestNotification(RQ_END_REQUEST, PRIORITY_ALIAS_LAST); pFactory = NULL; diff --git a/iis/moduleconfig.cpp b/iis/moduleconfig.cpp index d3bcefc9b6..066f73e161 100644 --- a/iis/moduleconfig.cpp +++ b/iis/moduleconfig.cpp @@ -79,8 +79,14 @@ MODSECURITY_STORED_CONTEXT::Initialize( // If there is a config failure, we cannot continue execution: if ( pPropertyException != NULL ) { - - ppException = ( IAppHostConfigException** ) &pPropertyException; + + // If there is a config failure, we cannot continue execution + // To avoid using reinterpret_cast, we will use static_cast to + // convert the IAppHostPropertyException to IAppHostConfigException + // An additional variable was created to hold the static_cast result + // and then assigned to ppException therefore avoiding double referencing. + auto pConfigExc = static_cast(pPropertyException); + ppException = &pConfigExc; goto Failure; } @@ -107,7 +113,9 @@ MODSECURITY_STORED_CONTEXT::Initialize( if ( pPropertyException != NULL ) { - ppException = ( IAppHostConfigException** ) &pPropertyException; + // See prevoius comment regarding static_cast and ppException assignment + auto pConfigExc = static_cast(pPropertyException); + ppException = &pConfigExc; goto Failure; } @@ -466,7 +474,7 @@ MODSECURITY_STORED_CONTEXT::~MODSECURITY_STORED_CONTEXT() MODSECURITY_STORED_CONTEXT::MODSECURITY_STORED_CONTEXT(): m_bIsEnabled ( FALSE ), m_pszPath( NULL ), - m_Config( NULL ) + m_Config( NULL ) { } diff --git a/iis/moduleconfig.h b/iis/moduleconfig.h index 75ee71dca2..d683d9269c 100644 --- a/iis/moduleconfig.h +++ b/iis/moduleconfig.h @@ -25,6 +25,7 @@ extern PVOID g_pModuleContext; class MODSECURITY_STORED_CONTEXT : public IHttpStoredContext { public: + void* m_Config; // cppcheck-suppress initializerList MODSECURITY_STORED_CONTEXT(); ~MODSECURITY_STORED_CONTEXT(); @@ -67,8 +68,6 @@ class MODSECURITY_STORED_CONTEXT : public IHttpStoredContext CHAR** ppszDestination, USHORT* pdwLengthDestination ); - void* m_Config; - private: HRESULT GetBooleanPropertyValue( diff --git a/iis/mymodule.cpp b/iis/mymodule.cpp index dfaee4b2cb..79ae7f1a83 100644 --- a/iis/mymodule.cpp +++ b/iis/mymodule.cpp @@ -38,13 +38,13 @@ class REQUEST_STORED_CONTEXT : public IHttpStoredContext public: REQUEST_STORED_CONTEXT() { - m_pConnRec = NULL; - m_pRequestRec = NULL; - m_pHttpContext = NULL; - m_pProvider = NULL; - m_pResponseBuffer = NULL; - m_pResponseLength = 0; - m_pResponsePosition = 0; + m_pConnRec = nullptr; + m_pRequestRec = nullptr; + m_pHttpContext = nullptr; + m_pProvider = nullptr; + m_pResponseBuffer = nullptr; + m_pResponseLength = 0; // cppcheck-suppress useInitializationList + m_pResponsePosition = 0; // cppcheck-suppress useInitializationList } ~REQUEST_STORED_CONTEXT() @@ -64,23 +64,23 @@ class REQUEST_STORED_CONTEXT : public IHttpStoredContext void FinishRequest() { - if(m_pRequestRec != NULL) + if(m_pRequestRec != nullptr) { modsecFinishRequest(m_pRequestRec); - m_pRequestRec = NULL; + m_pRequestRec = nullptr; } - if(m_pConnRec != NULL) + if(m_pConnRec != nullptr) { modsecFinishConnection(m_pConnRec); - m_pConnRec = NULL; + m_pConnRec = nullptr; } } - conn_rec *m_pConnRec; - request_rec *m_pRequestRec; - IHttpContext *m_pHttpContext; - IHttpEventProvider *m_pProvider; - char *m_pResponseBuffer; + conn_rec *m_pConnRec = nullptr; + request_rec *m_pRequestRec = nullptr; + IHttpContext *m_pHttpContext = nullptr; + IHttpEventProvider *m_pProvider = nullptr; + char *m_pResponseBuffer = nullptr; ULONGLONG m_pResponseLength; ULONGLONG m_pResponsePosition; }; @@ -129,7 +129,7 @@ apr_sockaddr_t *CopySockAddr(apr_pool_t *pool, PSOCKADDR pAddr) addr->family = pAddr->sa_family; if (pAddr->sa_family == AF_INET) { - auto sin = (SOCKADDR_IN *)pAddr; + auto sin = (const SOCKADDR_IN *)pAddr; addr->addr_str_len = INET_ADDRSTRLEN; addr->ipaddr_len = sizeof(struct in_addr); addr->ipaddr_ptr = &addr->sa.sin.sin_addr; @@ -140,7 +140,7 @@ apr_sockaddr_t *CopySockAddr(apr_pool_t *pool, PSOCKADDR pAddr) addr->salen = sizeof(addr->sa); addr->port = ntohs(sin->sin_port); } else if (pAddr->sa_family == AF_INET6) { - auto sin6 = (SOCKADDR_IN6 *)pAddr; + auto sin6 = (const SOCKADDR_IN6 *)pAddr; addr->addr_str_len = INET6_ADDRSTRLEN; addr->ipaddr_len = sizeof(struct in6_addr); addr->ipaddr_ptr = &addr->sa.sin6.sin6_addr; @@ -175,9 +175,9 @@ char *ZeroTerminate(const char *str, size_t len, apr_pool_t *pool) char *ConvertUTF16ToUTF8( __in const WCHAR * pszTextUTF16, size_t cchUTF16, apr_pool_t *pool ) { // - // Special case of NULL or empty input string + // Special case of nullptr or empty input string // - if ( (pszTextUTF16 == NULL) || (*pszTextUTF16 == L'\0') || cchUTF16 == 0 ) + if ( (pszTextUTF16 == nullptr) || (*pszTextUTF16 == L'\0') || cchUTF16 == 0 ) { // Return empty string return ""; @@ -191,9 +191,9 @@ char *ConvertUTF16ToUTF8( __in const WCHAR * pszTextUTF16, size_t cchUTF16, apr_ 0, // specify conversion behavior pszTextUTF16, // source UTF-16 string static_cast( cchUTF16 ), // total source string length, in WCHAR's, - NULL, // unused - no conversion required in this step + nullptr, // unused - no conversion required in this step 0, // request buffer size - NULL, NULL // unused + nullptr, nullptr // unused ); if ( cbUTF8 == 0 ) @@ -220,7 +220,7 @@ char *ConvertUTF16ToUTF8( __in const WCHAR * pszTextUTF16, size_t cchUTF16, apr_ // including end-of-string \0 pszUTF8, // destination buffer cbUTF8, // destination buffer size, in bytes - NULL, NULL // unused + nullptr, nullptr // unused ); if ( result == 0 ) @@ -243,7 +243,7 @@ void Log(void *obj, int level, char *str) if(level <= APLOG_ERR) logcat = EVENTLOG_ERROR_TYPE; - if(level == APLOG_WARNING || strstr(str, "Warning.") != NULL) + if(level == APLOG_WARNING || strstr(str, "Warning.") != nullptr) logcat = EVENTLOG_WARNING_TYPE; mod->WriteEventViewerLog(str, logcat); @@ -253,25 +253,25 @@ void Log(void *obj, int level, char *str) void StoreIISContext(request_rec *r, REQUEST_STORED_CONTEXT *rsc) { - apr_table_setn(r->notes, NOTE_IIS, (const char *)rsc); + apr_table_setn(r->notes, NOTE_IIS, (const char*)rsc); // cppcheck-suppress dangerousTypeCast } REQUEST_STORED_CONTEXT *RetrieveIISContext(request_rec *r) { - REQUEST_STORED_CONTEXT *msr = NULL; - request_rec *rx = NULL; + REQUEST_STORED_CONTEXT *msr = nullptr; + request_rec *rx = nullptr; /* Look in the current request first. */ msr = (REQUEST_STORED_CONTEXT *)apr_table_get(r->notes, NOTE_IIS); - if (msr != NULL) { + if (msr != nullptr) { //msr->r = r; return msr; } /* If this is a subrequest then look in the main request. */ - if (r->main != NULL) { + if (r->main != nullptr) { msr = (REQUEST_STORED_CONTEXT *)apr_table_get(r->main->notes, NOTE_IIS); - if (msr != NULL) { + if (msr != nullptr) { //msr->r = r; return msr; } @@ -279,16 +279,16 @@ REQUEST_STORED_CONTEXT *RetrieveIISContext(request_rec *r) /* If the request was redirected then look in the previous requests. */ rx = r->prev; - while(rx != NULL) { + while(rx != nullptr) { msr = (REQUEST_STORED_CONTEXT *)apr_table_get(rx->notes, NOTE_IIS); - if (msr != NULL) { + if (msr != nullptr) { //msr->r = r; return msr; } rx = rx->prev; } - return NULL; + return nullptr; } @@ -297,25 +297,25 @@ HRESULT CMyHttpModule::ReadFileChunk(HTTP_DATA_CHUNK *chunk, char *buf) OVERLAPPED ovl; DWORD dwDataStartOffset; ULONGLONG bytesTotal = 0; - BYTE * pIoBuffer = NULL; + BYTE * pIoBuffer = nullptr; HANDLE hIoEvent = INVALID_HANDLE_VALUE; HRESULT hr = S_OK; - pIoBuffer = (BYTE *)VirtualAlloc(NULL, + pIoBuffer = (BYTE *)VirtualAlloc(nullptr, 1, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); - if (pIoBuffer == NULL) + if (pIoBuffer == nullptr) { hr = HRESULT_FROM_WIN32(GetLastError()); goto Done; } - hIoEvent = CreateEvent(NULL, // security attr + hIoEvent = CreateEvent(nullptr, // security attr FALSE, // manual reset FALSE, // initial state - NULL); // name - if (hIoEvent == NULL) + nullptr); // name + if (hIoEvent == nullptr) { hr = HRESULT_FROM_WIN32(GetLastError()); goto Done; @@ -404,7 +404,7 @@ HRESULT CMyHttpModule::ReadFileChunk(HTTP_DATA_CHUNK *chunk, char *buf) } Done: - if(NULL != pIoBuffer) + if(nullptr != pIoBuffer) { VirtualFree(pIoBuffer, 0, MEM_RELEASE); } @@ -423,7 +423,7 @@ CMyHttpModule::OnSendResponse( IN ISendResponseProvider * pResponseProvider ) { - REQUEST_STORED_CONTEXT *rsc = NULL; + REQUEST_STORED_CONTEXT *rsc = nullptr; rsc = (REQUEST_STORED_CONTEXT *)pHttpContext->GetModuleContextContainer()->GetModuleContext(g_pModuleContext); @@ -431,16 +431,16 @@ CMyHttpModule::OnSendResponse( // here we must check if response body processing is enabled // - if(rsc == NULL || rsc->m_pRequestRec == NULL || rsc->m_pResponseBuffer != NULL || !modsecIsResponseBodyAccessEnabled(rsc->m_pRequestRec)) + if(rsc == nullptr || rsc->m_pRequestRec == nullptr || rsc->m_pResponseBuffer != nullptr || !modsecIsResponseBodyAccessEnabled(rsc->m_pRequestRec)) { goto Exit; } HRESULT hr = S_OK; - IHttpResponse *pHttpResponse = NULL; - HTTP_RESPONSE *pRawHttpResponse = NULL; - HTTP_BYTE_RANGE *pFileByteRange = NULL; - HTTP_DATA_CHUNK *pSourceDataChunk = NULL; + IHttpResponse *pHttpResponse = nullptr; + HTTP_RESPONSE *pRawHttpResponse = nullptr; + HTTP_BYTE_RANGE *pFileByteRange = nullptr; + HTTP_DATA_CHUNK *pSourceDataChunk = nullptr; LARGE_INTEGER lFileSize; REQUEST_NOTIFICATION_STATUS ret = RQ_NOTIFICATION_CONTINUE; ULONGLONG ulTotalLength = 0; @@ -460,7 +460,7 @@ CMyHttpModule::OnSendResponse( // here we must transfer response headers // USHORT ctcch = 0; - char *ct = (char *)pHttpResponse->GetHeader(HttpHeaderContentType, &ctcch); + auto ct = (const char *)pHttpResponse->GetHeader(HttpHeaderContentType, &ctcch); char *ctz = ZeroTerminate(ct, ctcch, r->pool); // assume HTML if content type not set @@ -471,7 +471,7 @@ CMyHttpModule::OnSendResponse( r->content_type = ctz; -#define _TRANSHEADER(id,str) if(pRawHttpResponse->Headers.KnownHeaders[id].pRawValue != NULL) \ +#define _TRANSHEADER(id,str) if(pRawHttpResponse->Headers.KnownHeaders[id].pRawValue != nullptr) \ {\ apr_table_setn(r->headers_out, str, \ ZeroTerminate(pRawHttpResponse->Headers.KnownHeaders[id].pRawValue, pRawHttpResponse->Headers.KnownHeaders[id].RawValueLength, r->pool)); \ @@ -522,7 +522,7 @@ CMyHttpModule::OnSendResponse( const char *lng = apr_table_get(r->headers_out, "Content-Languages"); - if(lng != NULL) + if(lng != nullptr) { r->content_languages = apr_array_make(r->pool, 1, sizeof(const char *)); @@ -633,7 +633,7 @@ CMyHttpModule::OnSendResponse( if (pResponseProvider->GetHeadersBeingSent() && (dwFlags & HTTP_SEND_RESPONSE_FLAG_MORE_DATA) == 0 && - pHttpContext->GetResponse()->GetHeader(HttpHeaderContentLength) == NULL) + pHttpContext->GetResponse()->GetHeader(HttpHeaderContentLength) == nullptr) { CHAR szLength[21]; //Max length for a 64 bit int is 20 @@ -682,7 +682,7 @@ CMyHttpModule::OnSendResponse( Exit: // temporary hack, in reality OnSendRequest theoretically could possibly come before OnEndRequest // - if(rsc != NULL) + if(rsc != nullptr) rsc->FinishRequest(); LeaveCriticalSection(&m_csLock); @@ -696,13 +696,13 @@ CMyHttpModule::OnPostEndRequest( IN IHttpEventProvider * pProvider ) { - REQUEST_STORED_CONTEXT *rsc = NULL; + REQUEST_STORED_CONTEXT *rsc = nullptr; rsc = (REQUEST_STORED_CONTEXT *)pHttpContext->GetModuleContextContainer()->GetModuleContext(g_pModuleContext); // only finish request if OnSendResponse have been called already // - if(rsc != NULL && rsc->m_pResponseBuffer != NULL) + if(rsc != nullptr && rsc->m_pResponseBuffer != nullptr) { EnterCriticalSection(&m_csLock); @@ -721,14 +721,14 @@ CMyHttpModule::OnBeginRequest( ) { HRESULT hr = S_OK; - IHttpRequest* pRequest = NULL; - MODSECURITY_STORED_CONTEXT* pConfig = NULL; + IHttpRequest* pRequest = nullptr; + MODSECURITY_STORED_CONTEXT* pConfig = nullptr; UNREFERENCED_PARAMETER ( pProvider ); EnterCriticalSection(&m_csLock); - if ( pHttpContext == NULL ) + if ( pHttpContext == nullptr ) { hr = E_UNEXPECTED; goto Finished; @@ -736,7 +736,7 @@ CMyHttpModule::OnBeginRequest( pRequest = pHttpContext->GetRequest(); - if ( pRequest == NULL ) + if ( pRequest == nullptr ) { hr = E_UNEXPECTED; goto Finished; @@ -758,7 +758,7 @@ CMyHttpModule::OnBeginRequest( goto Finished; } - if(pConfig->m_Config == NULL) + if(pConfig->m_Config == nullptr) { char *path; USHORT pathlen; @@ -790,7 +790,7 @@ CMyHttpModule::OnBeginRequest( { const char * err = modsecProcessConfig((directory_config *)pConfig->m_Config, path, apppath); - if(err != NULL) + if(err != nullptr) { WriteEventViewerLog(err, EVENTLOG_ERROR_TYPE); delete apppath; @@ -840,19 +840,18 @@ CMyHttpModule::OnBeginRequest( r->hostname = ConvertUTF16ToUTF8(req->CookedUrl.pHost, req->CookedUrl.HostLength / sizeof(WCHAR), r->pool); r->path_info = ConvertUTF16ToUTF8(req->CookedUrl.pAbsPath, req->CookedUrl.AbsPathLength / sizeof(WCHAR), r->pool); - if(r->hostname == NULL) + if(r->hostname == nullptr) { - if(req->Headers.KnownHeaders[HttpHeaderHost].pRawValue != NULL) + if(req->Headers.KnownHeaders[HttpHeaderHost].pRawValue != nullptr) r->hostname = ZeroTerminate(req->Headers.KnownHeaders[HttpHeaderHost].pRawValue, req->Headers.KnownHeaders[HttpHeaderHost].RawValueLength, r->pool); } int port = 0; - char *port_str = NULL; + char *port_str = nullptr; - if(r->hostname != NULL) + if(r->hostname != nullptr) { - int k = 0; char *ptr = (char *)r->hostname; while(*ptr != 0 && *ptr != ':') @@ -866,10 +865,10 @@ CMyHttpModule::OnBeginRequest( } } - if(req->CookedUrl.pQueryString != NULL && req->CookedUrl.QueryStringLength > 0) + if(req->CookedUrl.pQueryString != nullptr && req->CookedUrl.QueryStringLength > 0) r->args = ConvertUTF16ToUTF8(req->CookedUrl.pQueryString + 1, (req->CookedUrl.QueryStringLength / sizeof(WCHAR)) - 1, r->pool); -#define _TRANSHEADER(id,str) if(req->Headers.KnownHeaders[id].pRawValue != NULL) \ +#define _TRANSHEADER(id,str) if(req->Headers.KnownHeaders[id].pRawValue != nullptr) \ {\ apr_table_setn(r->headers_in, str, \ ZeroTerminate(req->Headers.KnownHeaders[id].pRawValue, req->Headers.KnownHeaders[id].RawValueLength, r->pool)); \ @@ -931,7 +930,7 @@ CMyHttpModule::OnBeginRequest( const char *lng = apr_table_get(r->headers_in, "Content-Languages"); - if(lng != NULL) + if(lng != nullptr) { r->content_languages = apr_array_make(r->pool, 1, sizeof(const char *)); @@ -1026,9 +1025,9 @@ CMyHttpModule::OnBeginRequest( r->parsed_uri.query = r->args; r->parsed_uri.dns_looked_up = 0; r->parsed_uri.dns_resolved = 0; - r->parsed_uri.password = NULL; - r->parsed_uri.user = NULL; - r->parsed_uri.fragment = NULL; + r->parsed_uri.password = nullptr; + r->parsed_uri.user = nullptr; + r->parsed_uri.fragment = nullptr; r->unparsed_uri = ZeroTerminate(req->pRawUrl, req->RawUrlLength, r->pool); r->uri = r->unparsed_uri; @@ -1059,7 +1058,7 @@ CMyHttpModule::OnBeginRequest( c->client_addr = CopySockAddr(r->pool, pAddr); c->client_ip = GetIpAddr(r->pool, pAddr); #endif - c->remote_host = NULL; + c->remote_host = nullptr; LeaveCriticalSection(&m_csLock); int status = modsecProcessRequest(r); @@ -1091,7 +1090,7 @@ apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, un *readcnt = 0; - if (rsc == NULL) + if (rsc == nullptr) { *is_eos = 1; return APR_SUCCESS; @@ -1106,7 +1105,11 @@ apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, un return APR_SUCCESS; } - HRESULT hr = pRequest->ReadEntityBody(buf, length, false, (DWORD *)readcnt, NULL); + DWORD dwReadcnt = 0; + HRESULT hr = pRequest->ReadEntityBody(buf, length, false, &dwReadcnt, nullptr); + if (readcnt != nullptr) { + *readcnt = static_cast(dwReadcnt); + } if (FAILED(hr)) { @@ -1123,11 +1126,11 @@ apr_status_t ReadBodyCallback(request_rec *r, char *buf, unsigned int length, un return APR_SUCCESS; } -apr_status_t WriteBodyCallback(request_rec *r, char *buf, unsigned int length) +apr_status_t WriteBodyCallback(request_rec *r, const char *buf, unsigned int length) { REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r); - if(rsc == NULL || rsc->m_pRequestRec == NULL) + if(rsc == nullptr || rsc->m_pRequestRec == nullptr) return APR_SUCCESS; IHttpContext *pHttpContext = rsc->m_pHttpContext; @@ -1175,7 +1178,7 @@ apr_status_t ReadResponseCallback(request_rec *r, char *buf, unsigned int length *readcnt = 0; - if(rsc == NULL || rsc->m_pResponseBuffer == NULL) + if(rsc == nullptr || rsc->m_pResponseBuffer == nullptr) { *is_eos = 1; return APR_SUCCESS; @@ -1197,11 +1200,11 @@ apr_status_t ReadResponseCallback(request_rec *r, char *buf, unsigned int length return APR_SUCCESS; } -apr_status_t WriteResponseCallback(request_rec *r, char *buf, unsigned int length) +apr_status_t WriteResponseCallback(request_rec *r, const char *buf, unsigned int length) { REQUEST_STORED_CONTEXT *rsc = RetrieveIISContext(r); - if(rsc == NULL || rsc->m_pRequestRec == NULL || rsc->m_pResponseBuffer == NULL) + if(rsc == nullptr || rsc->m_pRequestRec == nullptr || rsc->m_pResponseBuffer == nullptr) return APR_SUCCESS; IHttpContext *pHttpContext = rsc->m_pHttpContext; @@ -1255,7 +1258,7 @@ apr_status_t WriteResponseCallback(request_rec *r, char *buf, unsigned int lengt CMyHttpModule::CMyHttpModule() { // Open a handle to the Event Viewer. - m_hEventLog = RegisterEventSource( NULL, "ModSecurity" ); + m_hEventLog = RegisterEventSource( nullptr, "ModSecurity" ); // cppcheck-suppress useInitializationList SYSTEM_INFO sysInfo; @@ -1298,11 +1301,11 @@ CMyHttpModule::~CMyHttpModule() //WriteEventViewerLog("Module deleted."); // Test whether the handle for the Event Viewer is open. - if (NULL != m_hEventLog) + if (nullptr != m_hEventLog) { // Close the handle to the Event Viewer. DeregisterEventSource( m_hEventLog ); - m_hEventLog = NULL; + m_hEventLog = nullptr; DeleteCriticalSection(&m_csLock); } @@ -1315,13 +1318,13 @@ void CMyHttpModule::Dispose() BOOL CMyHttpModule::WriteEventViewerLog(LPCSTR szNotification, WORD category) { // Test whether the handle for the Event Viewer is open. - if (NULL != m_hEventLog) + if (nullptr != m_hEventLog) { // Write any strings to the Event Viewer and return. return ReportEvent( m_hEventLog, category, 0, 0x1, - NULL, 1, 0, &szNotification, NULL ); + nullptr, 1, 0, &szNotification, nullptr ); } return FALSE; } diff --git a/mlogc/mlogc.c b/mlogc/mlogc.c index 7a9edaa12b..89664eac2e 100644 --- a/mlogc/mlogc.c +++ b/mlogc/mlogc.c @@ -259,7 +259,7 @@ static char *_log_escape(apr_pool_t *mp, const char *input, * into a proper byte. Handles uppercase and lowercase letters * but does not check for overflows. */ -static unsigned char x2c(unsigned char *what) { +static unsigned char x2c(unsigned char const *what) { register unsigned char digit; digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0')); @@ -397,9 +397,11 @@ static void add_entry(const char *data, int start_worker) entry_t *entry = NULL; entry = (entry_t *)malloc(sizeof(entry_t)); - entry->id = 0; - entry->line = strdup(data); - entry->line_size = strlen(entry->line); + if(entry != NULL){ + entry->id = 0; + entry->line = strdup(data); + entry->line_size = strlen(entry->line); + } error_log(LOG_DEBUG, NULL, "Queue locking thread mutex."); if (APR_STATUS_IS_EBUSY(apr_thread_mutex_trylock(mutex))) { @@ -408,7 +410,9 @@ static void add_entry(const char *data, int start_worker) } /* Assign unique ID to this log entry. */ - entry->id = entry_counter++; + if(entry != NULL) { + entry->id = entry_counter++; + } /* Add the new audit log entry to the queue. */ *(entry_t **)apr_array_push(queue) = entry; @@ -435,12 +439,11 @@ static int read_queue_entries(apr_file_t *fd, apr_time_t *queue_time) char linebuf[4100]; int line_count = -1; int line_size = 0; - apr_status_t rc = 0; char *p = NULL; for(;;) { memset(linebuf, 0, 4100); - rc = apr_file_gets(linebuf, 4096, fd); + apr_status_t rc = apr_file_gets(linebuf, 4096, fd); if (rc == APR_EOF) break; if (rc != APR_SUCCESS) { @@ -644,7 +647,7 @@ static void transaction_checkpoint(void) /* Dump the entries sitting in the queue first. */ for (i = 0; i < queue->nelts; i++) { - entry_t *entry = ((entry_t **)queue->elts)[i]; + const entry_t *entry = ((entry_t **)queue->elts)[i]; apr_file_write_full(queue_fd, entry->line, entry->line_size, NULL); apr_file_write_full(queue_fd, &"\n", 1, NULL); } @@ -657,7 +660,7 @@ static void transaction_checkpoint(void) hi != NULL; hi = apr_hash_next(hi))\ { void *e; - entry_t *entry = NULL; + const entry_t *entry = NULL; i++; apr_hash_this(hi, NULL, NULL, &e); @@ -693,7 +696,8 @@ static void transaction_checkpoint(void) */ static void parse_configuration_line(const char *line, int line_count) { - char *start = NULL, *command = NULL; + const char *start = NULL; + const char *command = NULL; char *p = NULL; /* Remove the trailing newline character. */ @@ -1099,7 +1103,7 @@ static size_t curl_readfunction(void *ptr, size_t size, static size_t curl_writefunction(void *ptr, size_t size, size_t nmemb, void *stream) { - unsigned char *data = (unsigned char *)ptr; + const unsigned char *data = (unsigned char *)ptr; unsigned char *status = (unsigned char *)stream; /* Grab the status line text from the first line of output */ @@ -1162,7 +1166,7 @@ static int curl_debugfunction(CURL *curl, curl_infotype infotype, case CURLINFO_TEXT: /* More verbose data starts with an indent */ if (apr_isspace(data[0])) { - char *dataptr = data + 1; + const char *dataptr = data + 1; /* Skip initial whitespace (indent) */ while ( ((size_t)(dataptr - data) > datalen) @@ -1214,8 +1218,10 @@ static void logc_init(void) { char errstr[1024]; apr_status_t rc = 0; - const char *errptr = NULL; - int i, erroffset; + /* These variables are used by pcre_compile() and pcre2_compile() */ + const char *errptr = NULL; // cppcheck-suppress unreadVariable + int i; + int erroffset; // cppcheck-suppress unusedVariable /* cURL major, minor and patch version */ short cmaj, cmin, cpat = 0; #ifndef WITH_PCRE @@ -1320,8 +1326,9 @@ static void logc_init(void) *(CURL **)apr_array_push(curl_handles) = curl; } - - if (cmaj <= 7 && cmin < 34) { + /* These variables are initialized in the beginning of logc_init */ + if (cmaj <= 7 && cmin < 34) // cppcheck-suppress uninitvar + { error_log(LOG_DEBUG2, NULL, "TLSv1.2 is unsupported in cURL %d.%d.%d", cmaj, cmin, cpat); } @@ -1469,7 +1476,7 @@ static void * APR_THREAD_FUNC thread_worker(apr_thread_t *thread, void *data) * with rapid requests. With an invalid entry we never hit the * server, so we should not delay processing the next event. */ - int nodelay = 0; + int nodelay = 0; // cppcheck-suppress unreadVariable error_log(LOG_DEBUG, thread, "Worker thread starting."); @@ -1568,7 +1575,7 @@ static void * APR_THREAD_FUNC thread_worker(apr_thread_t *thread, void *data) rc = pcre2_match(logline_regex, entry->line, entry->line_size, 0, 0, pcre2_match_data, NULL); if (rc > 0) { - PCRE2_SIZE *pcre2_ovector = pcre2_get_ovector_pointer(pcre2_match_data); + const PCRE2_SIZE *pcre2_ovector = pcre2_get_ovector_pointer(pcre2_match_data); for (int i = 0; i < rc; i++) { capturevector[2*i] = pcre2_ovector[2*i]; capturevector[2*i+1] = pcre2_ovector[2*i+1]; @@ -1973,12 +1980,11 @@ static void create_new_worker(int lock) static void * APR_THREAD_FUNC thread_manager(apr_thread_t *thread, void *data) { apr_time_t last = 0; - apr_time_t now = 0; error_log(LOG_DEBUG, thread, "Management thread: Starting."); for(;;) { - now = apr_time_now(); + apr_time_t now = apr_time_now(); /* Should we stop running? */ if (running == 0) { @@ -2110,8 +2116,6 @@ static void receive_loop(void) { /* Loop forever receiving entries from stdin. */ while(!done || (curr < next)) { - apr_status_t rc; - if (error_log_level >= LOG_DEBUG2) { error_log(LOG_DEBUG2, NULL, "Internal state: " @@ -2126,7 +2130,7 @@ static void receive_loop(void) { if (!done && (nbytes > 0)) { buffered_events = 0; nbytes = PIPE_BUF_SIZE - next; - rc = apr_file_read(fd_stdin, (buf + next), &nbytes); + apr_status_t rc = apr_file_read(fd_stdin, (buf + next), &nbytes); if (rc != APR_SUCCESS) { if (have_read_data) { error_log(LOG_NOTICE, NULL, diff --git a/standalone/api.c b/standalone/api.c index aaf6db4113..257378cede 100644 --- a/standalone/api.c +++ b/standalone/api.c @@ -684,11 +684,11 @@ void modsecSetReadResponse(apr_status_t (*func)(request_rec *r, char *buf, unsig modsecReadResponse = func; } -void modsecSetWriteBody(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length)) { +void modsecSetWriteBody(apr_status_t (*func)(request_rec *r, const char *buf, unsigned int length)) { modsecWriteBody = func; } -void modsecSetWriteResponse(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length)) { +void modsecSetWriteResponse(apr_status_t (*func)(request_rec *r, const char *buf, unsigned int length)) { modsecWriteResponse = func; } diff --git a/standalone/api.h b/standalone/api.h index fbed33a16d..9416474077 100644 --- a/standalone/api.h +++ b/standalone/api.h @@ -124,8 +124,8 @@ static inline apr_bucket_brigade * modsecGetResponseBrigade(const request_rec *r void modsecSetReadBody(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos)); void modsecSetReadResponse(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length, unsigned int *readcnt, int *is_eos)); -void modsecSetWriteBody(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length)); -void modsecSetWriteResponse(apr_status_t (*func)(request_rec *r, char *buf, unsigned int length)); +void modsecSetWriteBody(apr_status_t (*func)(request_rec *r, const char *buf, unsigned int length)); +void modsecSetWriteResponse(apr_status_t (*func)(request_rec *r, const char *buf, unsigned int length)); void modsecSetDropAction(int (*func)(request_rec *r)); int modsecIsResponseBodyAccessEnabled(request_rec *r); From d26a12f4ce458a3ef4cf4c3867cbba968156a4b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Heged=C3=BCs=20D=C3=A1vid=20Marcell?= <119692618+umprayz@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:05:09 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- iis/moduleconfig.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iis/moduleconfig.cpp b/iis/moduleconfig.cpp index 066f73e161..5f33ebcdcb 100644 --- a/iis/moduleconfig.cpp +++ b/iis/moduleconfig.cpp @@ -113,7 +113,7 @@ MODSECURITY_STORED_CONTEXT::Initialize( if ( pPropertyException != NULL ) { - // See prevoius comment regarding static_cast and ppException assignment + // See previous comment regarding static_cast and ppException assignment auto pConfigExc = static_cast(pPropertyException); ppException = &pConfigExc; goto Failure;