From d50ded26e2db5ee354e60a8fe5b7e2b7625ea32a Mon Sep 17 00:00:00 2001 From: Mike Lothian Date: Fri, 21 Nov 2025 13:00:25 +0000 Subject: [PATCH 1/2] Fix memory leak in variable translation proxies The members m_variableArgsNames, m_variableArgsGetNames, and m_variableArgsPostNames were allocated using new but stored as references. This caused a memory leak because their destructors were never called. This change wraps the allocations in std::unique_ptr to ensure proper cleanup while keeping the reference members to maintain compatibility with existing code syntax. --- headers/modsecurity/transaction.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index 3e70caa38..d9d670d9c 100644 --- a/headers/modsecurity/transaction.h +++ b/headers/modsecurity/transaction.h @@ -205,9 +205,12 @@ class TransactionAnchoredVariables { m_variableFilesTmpNames(t, "FILES_TMPNAMES"), m_variableMultipartPartHeaders(t, "MULTIPART_PART_HEADERS"), m_variableOffset(0), - m_variableArgsNames("ARGS_NAMES", &m_variableArgs), - m_variableArgsGetNames("ARGS_GET_NAMES", &m_variableArgsGet), - m_variableArgsPostNames("ARGS_POST_NAMES", &m_variableArgsPost) + m_pVariableArgsNames(std::make_unique("ARGS_NAMES", &m_variableArgs)), + m_variableArgsNames(*m_pVariableArgsNames), + m_pVariableArgsGetNames(std::make_unique("ARGS_GET_NAMES", &m_variableArgsGet)), + m_variableArgsGetNames(*m_pVariableArgsGetNames), + m_pVariableArgsPostNames(std::make_unique("ARGS_POST_NAMES", &m_variableArgsPost)), + m_variableArgsPostNames(*m_pVariableArgsPostNames) { } AnchoredSetVariable m_variableRequestHeadersNames; @@ -291,9 +294,12 @@ class TransactionAnchoredVariables { int m_variableOffset; - AnchoredSetVariableTranslationProxy m_variableArgsNames; - AnchoredSetVariableTranslationProxy m_variableArgsGetNames; - AnchoredSetVariableTranslationProxy m_variableArgsPostNames; + std::unique_ptr m_pVariableArgsNames; + AnchoredSetVariableTranslationProxy &m_variableArgsNames; + std::unique_ptr m_pVariableArgsGetNames; + AnchoredSetVariableTranslationProxy &m_variableArgsGetNames; + std::unique_ptr m_pVariableArgsPostNames; + AnchoredSetVariableTranslationProxy &m_variableArgsPostNames; }; class TransactionSecMarkerManagement { From 26422d705a51bb92e8a2c79e7d4f6f38d7e10f33 Mon Sep 17 00:00:00 2001 From: Mike Lothian Date: Sat, 22 Nov 2025 14:39:55 +0000 Subject: [PATCH 2/2] Fix memory leak in transaction rule removal lists Valgrind reported memory leaks in m_ruleRemoveTargetByTag and related containers when ctl actions were used. This change explicitly clears m_ruleRemoveById, m_ruleRemoveByIdRange, m_ruleRemoveByTag, m_ruleRemoveTargetByTag, and m_ruleRemoveTargetById in the Transaction destructor to ensure all allocated memory is freed. --- src/transaction.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/transaction.cc b/src/transaction.cc index 6c8ae9744..a3b93336d 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -170,6 +170,12 @@ Transaction::~Transaction() { m_rulesMessages.clear(); + m_ruleRemoveById.clear(); + m_ruleRemoveByIdRange.clear(); + m_ruleRemoveByTag.clear(); + m_ruleRemoveTargetById.clear(); + m_ruleRemoveTargetByTag.clear(); + intervention::free(&m_it); intervention::clean(&m_it);