From c947f5e40dd3428024a3a8b9c9174ee9c85fd150 Mon Sep 17 00:00:00 2001
From: Eduardo Arias <eduar_hte@hotmail.com>
Date: Mon, 5 Aug 2024 11:18:35 -0700
Subject: [PATCH] Do not assume ModSecurityIntervention argument to
 transaction::intervention has been initialized/cleaned - Keep
 m_it->disruptive value and use it as return value to guarantee   that the
 value is correct.   - If m_it->disruptive is false and the 'it' argument has
 not been     initialized/cleaned, the function may incorrectly return a
 non-zero     value. - When a disruptive intervention is being reported by the
 function,   defensively initialize log & url to NULL if there's no such data
 to   provide to the caller.   - If the caller has not initialized/cleaned
 those fields in the 'it'     argument, after returning from
 transaction::intervention, the user     can safely read the log & url fields
 and in all scenarios they'll     have valid values.

---
 src/transaction.cc | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/transaction.cc b/src/transaction.cc
index 5826c264..a19ce88b 100644
--- a/src/transaction.cc
+++ b/src/transaction.cc
@@ -1469,16 +1469,24 @@ int Transaction::processLogging() {
  * @brief   Check if ModSecurity has anything to ask to the server.
  *
  * Intervention can generate a log event and/or perform a disruptive action.
+ * 
+ * If the return value is true, all fields in the ModSecurityIntervention
+ * parameter have been initialized and are safe to access.
+ * If the return value is false, no changes to the ModSecurityIntervention
+ * parameter have been made.
  *
- * @param Pointer ModSecurityIntervention structure
+ * @param it Pointer to ModSecurityIntervention structure
  * @retval true  A intervention should be made.
  * @retval false Nothing to be done.
  *
  */
 bool Transaction::intervention(ModSecurityIntervention *it) {
+    const auto disruptive = m_it.disruptive;
     if (m_it.disruptive) {
         if (m_it.url) {
             it->url = strdup(m_it.url);
+        } else {
+            it->url = NULL;
         }
         it->disruptive = m_it.disruptive;
         it->status = m_it.status;
@@ -1489,11 +1497,13 @@ bool Transaction::intervention(ModSecurityIntervention *it) {
             utils::string::replaceAll(&log, std::string("%d"),
                 std::to_string(it->status));
             it->log = strdup(log.c_str());
+        } else {
+            it->log = NULL;
         }
         intervention::reset(&m_it);
     }
 
-    return it->disruptive;
+    return disruptive;
 }
 
 
@@ -2260,11 +2270,16 @@ extern "C" void msc_transaction_cleanup(Transaction *transaction) {
  *
  * Intervention can generate a log event and/or perform a disruptive action.
  *
- * @param transaction ModSecurity transaction.
+ * If the return value is not zero, all fields in the ModSecurityIntervention
+ * parameter have been initialized and are safe to access.
+ * If the return value is zero, no changes to the ModSecurityIntervention
+ * parameter have been made.
  *
- * @return Pointer to ModSecurityIntervention structure
- * @retval >0   A intervention should be made.
- * @retval NULL Nothing to be done.
+ * @param transaction ModSecurity transaction.
+ * @param it Pointer to ModSecurityIntervention structure
+ * @returns If an intervention should be made
+ * @retval >0 A intervention should be made.
+ * @retval 0  Nothing to be done.
  *
  */
 extern "C" int msc_intervention(Transaction *transaction,