Fix memory leaks in ValidateSchema

Author: martinhsv

CHANGES

@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+  - Fix memory leaks in ValidateSchema
+    [#2469 - @martinhsv, @zimmerle]
   - Fix memory leak of ValidateDTD's m_dtd
     [#2469 - @martinhsv, @zimmerle]
   - Replaces put with setenv in SetEnv action

src/operators/validate_schema.cc

@@ -43,10 +43,20 @@ bool ValidateSchema::evaluate(Transaction *transaction,
     const RuleWithActions *rule,
     const bpstd::string_view &str,
     RuleMessage *ruleMessage) {
-    int rc;
 
-    m_parserCtx = xmlSchemaNewParserCtxt(m_resource.c_str());
-    if (m_parserCtx == NULL) {
+    if (transaction->m_xml->m_data.doc == NULL) {
+        ms_dbg_a(transaction, 4, "XML document tree could not be found for " \
+            "schema validation.");
+        return true;
+    }
+    if (transaction->m_xml->m_data.well_formed != 1) {
+        ms_dbg_a(transaction, 4, "XML: Schema validation failed because " \
+            "content is not well formed.");
+        return true;
+    }
+
+    xmlSchemaParserCtxtPtr parserCtx = xmlSchemaNewParserCtxt(m_resource.c_str());
+    if (parserCtx == NULL) {
         std::stringstream err;
         err << "XML: Failed to load Schema from file: ";
         err << m_resource;
@@ -58,18 +68,18 @@ bool ValidateSchema::evaluate(Transaction *transaction,
         return true;
     }
 
-    xmlSchemaSetParserErrors(m_parserCtx,
+    xmlSchemaSetParserErrors(parserCtx,
         (xmlSchemaValidityErrorFunc)error_load,
         (xmlSchemaValidityWarningFunc)warn_load, &m_err);
 
-    xmlThrDefSetGenericErrorFunc(m_parserCtx,
+    xmlThrDefSetGenericErrorFunc(parserCtx,
         null_error);
 
-    xmlSetGenericErrorFunc(m_parserCtx,
+    xmlSetGenericErrorFunc(parserCtx,
         null_error);
 
-    m_schema = xmlSchemaParse(m_parserCtx);
-    if (m_schema == NULL) {
+    xmlSchemaPtr schema = xmlSchemaParse(parserCtx);
+    if (schema == NULL) {
         std::stringstream err;
         err << "XML: Failed to load Schema: ";
         err << m_resource;
@@ -78,60 +88,40 @@ bool ValidateSchema::evaluate(Transaction *transaction,
             err << " " << m_err;
         }
         ms_dbg_a(transaction, 4, err.str());
-        xmlSchemaFreeParserCtxt(m_parserCtx);
+        xmlSchemaFreeParserCtxt(parserCtx);
         return true;
     }
 
-    m_validCtx = xmlSchemaNewValidCtxt(m_schema);
-    if (m_validCtx == NULL) {
+    xmlSchemaValidCtxtPtr validCtx = xmlSchemaNewValidCtxt(schema);
+    if (validCtx == NULL) {
         std::stringstream err("XML: Failed to create validation context.");
         if (m_err.empty() == false) {
             err << " " << m_err;
         }
         ms_dbg_a(transaction, 4, err.str());
+        xmlSchemaFree(schema);
+        xmlSchemaFreeParserCtxt(parserCtx);
         return true;
     }
 
     /* Send validator errors/warnings to msr_log */
-    xmlSchemaSetValidErrors(m_validCtx,
+    xmlSchemaSetValidErrors(validCtx,
         (xmlSchemaValidityErrorFunc)error_runtime,
         (xmlSchemaValidityWarningFunc)warn_runtime, transaction);
 
-    if (transaction->m_xml->m_data.doc == NULL) {
-        ms_dbg_a(transaction, 4, "XML document tree could not be found for " \
-            "schema validation.");
-        return true;
-    }
-
-    if (transaction->m_xml->m_data.well_formed != 1) {
-        ms_dbg_a(transaction, 4, "XML: Schema validation failed because " \
-            "content is not well formed.");
-        return true;
-    }
-
-    /* Make sure there were no other generic processing errors */
-    /*
-    if (msr->msc_reqbody_error) {
-        ms_dbg_a(t, 4, "XML: Schema validation could not proceed due to previous"
-                " processing errors.");
-        return true;
-    }
-    */
+    int rc = xmlSchemaValidateDoc(validCtx, transaction->m_xml->m_data.doc);
 
-    rc = xmlSchemaValidateDoc(m_validCtx, transaction->m_xml->m_data.doc);
+    xmlSchemaFreeValidCtxt(validCtx);
+    xmlSchemaFree(schema);
+    xmlSchemaFreeParserCtxt(parserCtx);
     if (rc != 0) {
         ms_dbg_a(transaction, 4, "XML: Schema validation failed.");
-        xmlSchemaFree(m_schema);
-        xmlSchemaFreeParserCtxt(m_parserCtx);
         return true; /* No match. */
+    } else {
+        ms_dbg_a(transaction, 4, "XML: Successfully validated payload against " \
+            "Schema: " + m_resource);
+        return false;
     }
-
-    ms_dbg_a(transaction, 4, "XML: Successfully validated payload against " \
-        "Schema: " + m_resource);
-    xmlSchemaFree(m_schema);
-    xmlSchemaFreeParserCtxt(m_parserCtx);
-
-    return false;
 }
 
 #endif

src/operators/validate_schema.h

@@ -36,27 +36,10 @@ namespace operators {
 class ValidateSchema : public Operator {
  public:
     /** @ingroup ModSecurity_Operator */
-#ifndef WITH_LIBXML2
     explicit ValidateSchema(std::unique_ptr<RunTimeString> param)
         : Operator("ValidateSchema", std::move(param)) { }
-#else
-    explicit ValidateSchema(std::unique_ptr<RunTimeString> param)
-        : Operator("ValidateSchema", std::move(param)),
-        m_parserCtx(NULL),
-        m_validCtx(NULL),
-        m_schema(NULL) { }
-    ~ValidateSchema() {
-        /*
-        if (m_schema != NULL) {
-            xmlSchemaFree(m_schema);
-            m_schema = NULL;
-        }
-        */
-        if (m_validCtx != NULL) {
-            xmlSchemaFreeValidCtxt(m_validCtx);
-            m_validCtx = NULL;
-        }
-    }
+    ~ValidateSchema() { }
+#ifdef WITH_LIBXML2
 
     bool evaluate(Transaction *transaction,
         const RuleWithActions *rule,
@@ -133,9 +116,6 @@ class ValidateSchema : public Operator {
     }
 
  private:
-    xmlSchemaParserCtxtPtr m_parserCtx;
-    xmlSchemaValidCtxtPtr m_validCtx;
-    xmlSchemaPtr m_schema;
     std::string m_resource;
     std::string m_err;
 #endif