From 0cc2a7575dec997e6db3b071fa43db6d740694ab Mon Sep 17 00:00:00 2001
From: martinhsv <55407942+martinhsv@users.noreply.github.com>
Date: Tue, 15 Dec 2020 09:48:03 -0800
Subject: [PATCH] Fix memory leak of ValidateDTD's dtd object

---
 CHANGES                       |  2 ++
 src/operators/validate_dtd.cc | 10 ++++------
 src/operators/validate_dtd.h  | 23 +++++++++++++++++------
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/CHANGES b/CHANGES
index 395cb7af1d..e9a8751eea 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
  
+  - Fix memory leak of ValidateDTD's m_dtd
+    [#2469 - @martinhsv, @zimmerle]
   - Replaces put with setenv in SetEnv action
     [#2469 - @martinhsv, @WGH-, @zimmerle]
   - Using a custom VariableMatch* implementation
diff --git a/src/operators/validate_dtd.cc b/src/operators/validate_dtd.cc
index 226d595ad8..e1f0cad04d 100644
--- a/src/operators/validate_dtd.cc
+++ b/src/operators/validate_dtd.cc
@@ -47,10 +47,8 @@ bool ValidateDTD::evaluate(Transaction *transaction,
         const RuleWithActions *rule,
         const bpstd::string_view &input,
         RuleMessage *ruleMessage) {
-    xmlValidCtxtPtr cvp;
-
-    m_dtd = xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str());
-    if (m_dtd == NULL) {
+    XmlDtdPtrManager m_dtd(xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str()));
+    if (m_dtd.get() == NULL) {
         std::string err = std::string("XML: Failed to load DTD: ") \
             + m_resource;
         ms_dbg_a(transaction, 4, err);
@@ -79,7 +77,7 @@ bool ValidateDTD::evaluate(Transaction *transaction,
     }
 #endif
 
-    cvp = xmlNewValidCtxt();
+    xmlValidCtxtPtr cvp = xmlNewValidCtxt();
     if (cvp == NULL) {
         ms_dbg_a(transaction, 4,
             "XML: Failed to create a validation context.");
@@ -91,7 +89,7 @@ bool ValidateDTD::evaluate(Transaction *transaction,
     cvp->warning = (xmlSchemaValidityErrorFunc)warn_runtime;
     cvp->userData = transaction;
 
-    if (!xmlValidateDtd(cvp, transaction->m_xml->m_data.doc, m_dtd)) {
+    if (!xmlValidateDtd(cvp, transaction->m_xml->m_data.doc, m_dtd.get())) {
         ms_dbg_a(transaction, 4, "XML: DTD validation failed.");
         xmlFreeValidCtxt(cvp);
         return true;
diff --git a/src/operators/validate_dtd.h b/src/operators/validate_dtd.h
index 944fa66a7c..7d3612c13b 100644
--- a/src/operators/validate_dtd.h
+++ b/src/operators/validate_dtd.h
@@ -33,18 +33,30 @@
 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 RuleWithActions *rule,
@@ -93,7 +105,6 @@ class ValidateDTD : public Operator {
 
  private:
     std::string m_resource;
-    xmlDtdPtr m_dtd = NULL;
 #endif
 };