diff --git a/CHANGES b/CHANGES index e9a8751eea..8346f9038f 100644 --- a/CHANGES +++ b/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 diff --git a/src/operators/validate_schema.cc b/src/operators/validate_schema.cc index 65b471703d..6845c67074 100644 --- a/src/operators/validate_schema.cc +++ b/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 diff --git a/src/operators/validate_schema.h b/src/operators/validate_schema.h index 1182773528..ea9c1c8c50 100644 --- a/src/operators/validate_schema.h +++ b/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 param) : Operator("ValidateSchema", std::move(param)) { } -#else - explicit ValidateSchema(std::unique_ptr 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