From beaa4523029ed118f7039b9cd2e8c4908abe2ee0 Mon Sep 17 00:00:00 2001
From: Martin Vierula <martin.vierula@trustwave.com>
Date: Mon, 30 Oct 2023 15:40:36 -0700
Subject: [PATCH] Fix memory leak of validateDTD's dtd object

---
 CHANGES                       |  2 ++
 src/operators/validate_dtd.cc | 31 +++++++++++++++----------------
 src/operators/validate_dtd.h  | 24 ++++++++++++++++++------
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/CHANGES b/CHANGES
index 74b68d24de..e012009446 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+  - Fix memory leak of validateDTD's dtd object
+    [Issue #3008 - @martinhsv, @zimmerle]
   - Fix memory leaks in ValidateSchema
     [Issue #3005 - @martinhsv, @zimmerle]
   - Add support for expirevar action
diff --git a/src/operators/validate_dtd.cc b/src/operators/validate_dtd.cc
index f57dbed3a5..138c707801 100644
--- a/src/operators/validate_dtd.cc
+++ b/src/operators/validate_dtd.cc
@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -43,25 +43,24 @@ bool ValidateDTD::init(const std::string &file, std::string *error) {
 }
 
 
-bool ValidateDTD::evaluate(Transaction *t, const std::string &str) {
-    xmlValidCtxtPtr cvp;
+bool ValidateDTD::evaluate(Transaction *transaction, const std::string &str) {
 
-    m_dtd = xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str());
-    if (m_dtd == NULL) {
+    XmlDtdPtrManager dtd(xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str()));
+    if (dtd.get() == NULL) {
         std::string err = std::string("XML: Failed to load DTD: ") \
             + m_resource;
-        ms_dbg_a(t, 4, err);
+        ms_dbg_a(transaction, 4, err);
         return true;
     }
 
-    if (t->m_xml->m_data.doc == NULL) {
-        ms_dbg_a(t, 4, "XML document tree could not "\
+    if (transaction->m_xml->m_data.doc == NULL) {
+        ms_dbg_a(transaction, 4, "XML document tree could not "\
             "be found for DTD validation.");
         return true;
     }
 
-    if (t->m_xml->m_data.well_formed != 1) {
-        ms_dbg_a(t, 4, "XML: DTD validation failed because " \
+    if (transaction->m_xml->m_data.well_formed != 1) {
+        ms_dbg_a(transaction, 4, "XML: DTD validation failed because " \
             "content is not well formed.");
         return true;
     }
@@ -76,24 +75,24 @@ bool ValidateDTD::evaluate(Transaction *t, const std::string &str) {
     }
 #endif
 
-    cvp = xmlNewValidCtxt();
+    xmlValidCtxtPtr cvp = xmlNewValidCtxt();
     if (cvp == NULL) {
-        ms_dbg_a(t, 4, "XML: Failed to create a validation context.");
+        ms_dbg_a(transaction, 4, "XML: Failed to create a validation context.");
         return true;
     }
 
     /* Send validator errors/warnings to msr_log */
     cvp->error = (xmlSchemaValidityErrorFunc)error_runtime;
     cvp->warning = (xmlSchemaValidityErrorFunc)warn_runtime;
-    cvp->userData = t;
+    cvp->userData = transaction;
 
-    if (!xmlValidateDtd(cvp, t->m_xml->m_data.doc, m_dtd)) {
-        ms_dbg_a(t, 4, "XML: DTD validation failed.");
+    if (!xmlValidateDtd(cvp, transaction->m_xml->m_data.doc, dtd.get())) {
+        ms_dbg_a(transaction, 4, "XML: DTD validation failed.");
         xmlFreeValidCtxt(cvp);
         return true;
     }
 
-    ms_dbg_a(t, 4, std::string("XML: Successfully validated " \
+    ms_dbg_a(transaction, 4, std::string("XML: Successfully validated " \
         "payload against DTD: ") + m_resource);
 
     xmlFreeValidCtxt(cvp);
diff --git a/src/operators/validate_dtd.h b/src/operators/validate_dtd.h
index ace6aad464..ef1c7c231d 100644
--- a/src/operators/validate_dtd.h
+++ b/src/operators/validate_dtd.h
@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -33,18 +33,31 @@
 namespace modsecurity {
 namespace operators {
 
-class ValidateDTD : public Operator {
+class XmlDtdPtrManager {
  public:
     /** @ingroup ModSecurity_Operator */
-    explicit ValidateDTD(std::unique_ptr<RunTimeString> param)
-        : Operator("ValidateDTD", std::move(param)) { }
+    explicit XmlDtdPtrManager(xmlDtdPtr dtd)
+        : m_dtd(dtd) { }
+    ~XmlDtdPtrManager() {
 #ifdef WITH_LIBXML2
-    ~ValidateDTD() {
         if (m_dtd != NULL) {
             xmlFreeDtd(m_dtd);
             m_dtd = NULL;
         }
+#endif
     }
+    xmlDtdPtr get() const {return m_dtd;}
+ private:
+    xmlDtdPtr m_dtd; // The resource being managed
+};
+
+class ValidateDTD : public Operator {
+ public:
+    /** @ingroup ModSecurity_Operator */
+    explicit ValidateDTD(std::unique_ptr<RunTimeString> param)
+        : Operator("ValidateDTD", std::move(param)) { }
+#ifdef WITH_LIBXML2
+    ~ValidateDTD() { }
 
     bool evaluate(Transaction *transaction, const std::string  &str) override;
     bool init(const std::string &file, std::string *error) override;
@@ -89,7 +102,6 @@ class ValidateDTD : public Operator {
 
  private:
     std::string m_resource;
-    xmlDtdPtr m_dtd = NULL;
 #endif
 };