Adds support to SecXMLExternalEntity

headers/modsecurity/rules_properties.h

@@ -56,6 +56,7 @@ class RulesProperties {
         requestBodyInMemoryLimit(0),
         secRequestBodyAccess(false),
         secResponseBodyAccess(false),
+        secXMLExternalEntity(false),
         requestBodyLimitAction(ProcessPartialBodyLimitAction),
         responseBodyLimit(0),
         responseBodyLimitAction(ProcessPartialBodyLimitAction),
@@ -71,6 +72,7 @@ class RulesProperties {
         requestBodyInMemoryLimit(0),
         secRequestBodyAccess(false),
         secResponseBodyAccess(false),
+        secXMLExternalEntity(false),
         requestBodyLimitAction(ProcessPartialBodyLimitAction),
         responseBodyLimit(0),
         responseBodyLimitAction(ProcessPartialBodyLimitAction),
@@ -202,6 +204,7 @@ class RulesProperties {
 
     bool secRequestBodyAccess;
     bool secResponseBodyAccess;
+    bool secXMLExternalEntity;
     std::string audit_log_path;
     std::string audit_log_parts;
     std::list<std::string> components;

src/actions/ctl_request_body_processor_xml.cc

@@ -24,8 +24,9 @@ namespace modsecurity {
 namespace actions {
 
 
-bool CtlRequestBodyProcessorXML::evaluate(Rule *rule, Transaction *transaction) {
+bool CtlRequestBodyProcessorXML::evaluate(Rule *rule,
-    transaction->m_requestBodyProcessor = modsecurity::Transaction::XMLRequestBody;
+    Transaction *transaction) {
+    transaction->m_requestBodyProcessor = Transaction::XMLRequestBody;
     return true;
 }
 

src/operators/validate_dtd.cc

@@ -39,14 +39,6 @@ bool ValidateDTD::init(const std::string &file, const char **error) {
     xmlSetGenericErrorFunc(NULL,
         null_error);
 
-    m_dtd = xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str());
-    if (m_dtd == NULL) {
-        std::string err = std::string("XML: Failed to load DTD: ") \
-            + m_resource;
-        *error = strdup(err.c_str());
-        return false;
-    }
-
     return true;
 }
 
@@ -54,6 +46,14 @@ bool ValidateDTD::init(const std::string &file, const char **error) {
 bool ValidateDTD::evaluate(Transaction *t, const std::string &str) {
     xmlValidCtxtPtr cvp;
 
+    m_dtd = xmlParseDTD(NULL, (const xmlChar *)m_resource.c_str());
+    if (m_dtd == NULL) {
+        std::string err = std::string("XML: Failed to load DTD: ") \
+            + m_resource;
+        t->debug(4, err);
+        return true;
+    }
+
     if (t->m_xml->m_data.doc == NULL) {
         t->debug(4, "XML document tree could not "\
             "be found for DTD validation.");

src/operators/validate_schema.cc

@@ -33,6 +33,14 @@ bool ValidateSchema::init(const std::string &file, const char **error) {
         return false;
     }
 
+    return true;
+}
+
+
+bool ValidateSchema::evaluate(Transaction *t,
+    const std::string &str) {
+    int rc;
+
     m_parserCtx = xmlSchemaNewParserCtxt(m_resource.c_str());
     if (m_parserCtx == NULL) {
         std::stringstream err;
@@ -42,8 +50,8 @@ bool ValidateSchema::init(const std::string &file, const char **error) {
         if (m_err.empty() == false) {
             err << m_err;
         }
-        *error = strdup(err.str().c_str());
+        t->debug(4, err.str());
-        return false;
+        return true;
     }
 
     xmlSchemaSetParserErrors(m_parserCtx,
@@ -65,9 +73,9 @@ bool ValidateSchema::init(const std::string &file, const char **error) {
         if (m_err.empty() == false) {
             err << " " << m_err;
         }
-        *error = strdup(err.str().c_str());
+        t->debug(4, err.str());
         xmlSchemaFreeParserCtxt(m_parserCtx);
-        return false;
+        return true;
     }
 
     m_validCtx = xmlSchemaNewValidCtxt(m_schema);
@@ -76,18 +84,10 @@ bool ValidateSchema::init(const std::string &file, const char **error) {
         if (m_err.empty() == false) {
             err << " " << m_err;
         }
-        *error = strdup(err.str().c_str());
+        t->debug(4, err.str());
-        return false;
+        return true;
     }
 
-    return true;
-}
-
-
-bool ValidateSchema::evaluate(Transaction *t,
-    const std::string &str) {
-    int rc;
-
     /* Send validator errors/warnings to msr_log */
     xmlSchemaSetValidErrors(m_validCtx,
         (xmlSchemaValidityErrorFunc)error_runtime,

src/parser/seclang-parser.yy

@@ -213,6 +213,8 @@ using modsecurity::Variables::XML;
 %token <std::string> CONFIG_DIR_DEBUG_LOG
 %token <std::string> CONFIG_DIR_DEBUG_LVL
 
+%token <std::string> CONFIG_XML_EXTERNAL_ENTITY
+
 %token <std::string> CONFIG_DIR_SEC_ACTION
 %token <std::string> CONFIG_DIR_SEC_DEFAULT_ACTION
 %token <std::string> CONFIG_DIR_SEC_MARKER
@@ -652,6 +654,14 @@ expression:
             driver.m_responseBodyTypeToBeInspected.insert(*it);
         }
       }
+    | CONFIG_XML_EXTERNAL_ENTITY CONFIG_VALUE_OFF
+      {
+        driver.secXMLExternalEntity = false;
+      }
+    | CONFIG_XML_EXTERNAL_ENTITY CONFIG_VALUE_ON
+      {
+        driver.secXMLExternalEntity = true;
+      }
     | CONGIG_DIR_SEC_TMP_DIR
     | CONGIG_DIR_SEC_DATA_DIR
     | CONGIG_DIR_SEC_ARG_SEP

src/parser/seclang-scanner.ll

@@ -83,6 +83,7 @@ CONFIG_DIR_RULE_ENG (?i:SecRuleEngine)
 CONFIG_DIR_REQ_BODY (?i:SecRequestBodyAccess)
 CONFIG_DIR_RES_BODY (?i:SecResponseBodyAccess)
 
+CONFIG_XML_EXTERNAL_ENTITY (?i:SecXmlExternalEntity)
 
 CONFIG_DIR_AUDIT_DIR_MOD (?i:SecAuditLogDirMode)
 CONFIG_DIR_AUDIT_DIR     (?i:SecAuditLogStorageDir)
@@ -285,6 +286,7 @@ CONFIG_DIR_UNICODE_MAP_FILE (?i:SecUnicodeMapFile)
 {CONFIG_COMPONENT_SIG}[ ]["]{FREE_TEXT}["] { return yy::seclang_parser::make_CONFIG_COMPONENT_SIG(strchr(yytext, ' ') + 2, *driver.loc.back()); }
 
 %{ /* Other configurations */ %}
+{CONFIG_XML_EXTERNAL_ENTITY} { return yy::seclang_parser::make_CONFIG_XML_EXTERNAL_ENTITY(yytext, *driver.loc.back()); }
 {CONFIG_DIR_PCRE_MATCH_LIMIT_RECURSION}[ ]{CONFIG_VALUE_NUMBER}  { return yy::seclang_parser::make_CONFIG_DIR_PCRE_MATCH_LIMIT_RECURSION(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_PCRE_MATCH_LIMIT}[ ]{CONFIG_VALUE_NUMBER}    { return yy::seclang_parser::make_CONFIG_DIR_PCRE_MATCH_LIMIT(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONGIG_DIR_RESPONSE_BODY_MP}[ ]{FREE_TEXT_NEW_LINE}   { return yy::seclang_parser::make_CONGIG_DIR_RESPONSE_BODY_MP(strchr(yytext, ' ') + 1, *driver.loc.back()); }

src/request_body_processor/xml.cc

@@ -41,13 +41,25 @@ XML::~XML() {
 
 
 bool XML::init() {
-    // xmlParserInputBufferCreateFilenameFunc entity;
+    xmlParserInputBufferCreateFilenameFunc entity;
-    // entity = xmlParserInputBufferCreateFilenameDefault(
+    if (m_transaction->m_rules->secXMLExternalEntity == true) {
-    //     this->unloadExternalEntity);
+        entity = xmlParserInputBufferCreateFilenameDefault(
+            __xmlParserInputBufferCreateFilename);
+    } else {
+        entity = xmlParserInputBufferCreateFilenameDefault(
+            this->unloadExternalEntity);
+    }
+
     return true;
 }
 
 
+xmlParserInputBufferPtr XML::unloadExternalEntity(const char *URI,
+    xmlCharEncoding enc) {
+    return NULL;
+}
+
+
 bool XML::processChunk(const char *buf, unsigned int size) {
     /* We want to initialise our parsing context here, to
      * enable us to pass it the first chunk of data so that

src/request_body_processor/xml.h

@@ -21,6 +21,7 @@
 #include <iostream>
 
 #include "modsecurity/transaction.h"
+#include "modsecurity/rules.h"
 
 #ifndef SRC_REQUEST_BODY_PROCESSOR_XML_H_
 #define SRC_REQUEST_BODY_PROCESSOR_XML_H_
@@ -48,7 +49,7 @@ class XML {
     bool processChunk(const char *buf, unsigned int size);
     bool complete();
     static xmlParserInputBufferPtr unloadExternalEntity(const char *URI,
-        xmlCharEncoding enc) { return NULL; }
+        xmlCharEncoding enc);
 
 #ifndef NO_LOGS
     void debug(int a, std::string str) {

src/rules.cc

@@ -206,6 +206,7 @@ int Rules::merge(Driver *from) {
     this->secRuleEngine = from->secRuleEngine;
     this->secRequestBodyAccess = from->secRequestBodyAccess;
     this->secResponseBodyAccess = from->secResponseBodyAccess;
+    this->secXMLExternalEntity = from->secXMLExternalEntity;
     if (from->m_debugLog && this->m_debugLog &&
         from->m_debugLog->isLogFileSet()) {
         this->m_debugLog->setDebugLogFile(from->m_debugLog->getDebugLogFile());

src/variables/xml.cc

@@ -34,6 +34,8 @@
 #include <utility>
 
 #include "modsecurity/transaction.h"
+#include "modsecurity/rules_properties.h"
+#include "modsecurity/rules.h"
 
 #include "src/request_body_processor/xml.h"
 #include "src/actions/action.h"

test/test-cases/regression/config-xml_external_entity.json

@@ -0,0 +1,142 @@
+[
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing SecXMLExternalEntity/XXE 1",
+    "expected":{
+      "debug_log": "Target value: \" jo smith\""
+    },
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*",
+        "Cookie": "PHPSESSID=rAAAAAAA2t5uvjq435r4q7ib3vtdjq120",
+        "Content-Type": "text/xml"
+      },
+      "uri":"/?key=value&key=other_value",
+      "method":"POST",
+      "body": [
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+        "<!DOCTYPE author [",
+        "<!ELEMENT book (#PCDATA)>",
+        "<!ENTITY js \"Jo Smith\">",
+        "]>",
+        "<bookstore>",
+        "<book category=\"WEB\"> &js;</book>",
+        "</bookstore>"
+      ]
+
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "rules":[
+        "SecRuleEngine On",
+        "SecRequestBodyAccess On",
+        "SecXMLExternalEntity Off",
+        "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
+        "SecRule XML:/bookstore/book[text()] \".*\" \"id:500006,phase:3,t:none,t:lowercase,nolog,pass\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing SecXMLExternalEntity/XXE 2",
+    "expected":{
+      "debug_log": "XML: Failed to load DTD: test-cases/data/SoapEnvelope.dtd",
+      "http_code": 403
+    },
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*",
+        "Cookie": "PHPSESSID=rAAAAAAA2t5uvjq435r4q7ib3vtdjq120",
+        "Content-Type": "text/xml"
+      },
+      "uri":"/?key=value&key=other_value",
+      "method":"POST",
+      "body": [
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+        "<!DOCTYPE author [",
+        "<!ELEMENT book ANY>",
+        "<!ENTITY js SYSTEM \"/etc/passwd\">",
+        "]>",
+        "<bookstore>",
+        "<book category=\"WEB\"> &js;</book>",
+        "</bookstore>"
+      ]
+
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "rules":[
+        "SecRuleEngine On",
+        "SecRequestBodyAccess On",
+        "SecXMLExternalEntity Off",
+        "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
+        "SecRule XML:/bookstore/book \".*\" \"id:500006,phase:3,t:none,t:lowercase,nolog,pass,xmlns:soap='http://schemas.xmlsoap.org/soap/envelope/'\"",
+        "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope.dtd\" \"id:500007,phase:3,deny\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing SecXMLExternalEntity/XXE 3",
+    "expected":{
+      "debug_log": "XML Error: No declaration for element bookstore",
+      "http_code": 403
+    },
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*",
+        "Cookie": "PHPSESSID=rAAAAAAA2t5uvjq435r4q7ib3vtdjq120",
+        "Content-Type": "text/xml"
+      },
+      "uri":"/?key=value&key=other_value",
+      "method":"POST",
+      "body": [
+        "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+        "<!DOCTYPE author [",
+        "<!ELEMENT book ANY>",
+        "<!ENTITY js SYSTEM \"/etc/passwd\">",
+        "]>",
+        "<bookstore>",
+        "<book category=\"WEB\"> &js;</book>",
+        "</bookstore>"
+      ]
+
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "rules":[
+        "SecRuleEngine On",
+        "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
+        "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
+        "SecRule XML:/bookstore/book \".*\" \"id:500006,phase:3,t:none,t:lowercase,nolog,pass,xmlns:soap='http://schemas.xmlsoap.org/soap/envelope/'\"",
+        "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope.dtd\" \"id:500007,phase:3,deny\""
+    ]
+  }
+]
+

test/test-cases/regression/request-body-parser-xml-validade-dtd.json

@@ -39,6 +39,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope.dtd\" \"id:500007,phase:3,deny\""
     ]
@@ -84,6 +85,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope.dtd\" \"id:500007,phase:3,deny\""
     ]
@@ -129,6 +131,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope.dtd\" \"id:500007,phase:3,deny\""
     ]
@@ -138,7 +141,8 @@
     "version_min":300000,
     "title":"Testing XML request body parser - validateDTD (bad DTD)",
     "expected":{
-      "parser_error": "Line: 4. Column: 12. XML: Failed to load DTD: test-cases/data/SoapEnvelope-bad.dtd"
+      "debug_log": "Failed to load DTD: test-cases/data/SoapEnvelope-bad.dtd",
+      "http_code": 403
     },
     "client":{
       "ip":"200.249.12.31",
@@ -173,6 +177,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateDTD test-cases/data/SoapEnvelope-bad.dtd\" \"id:500007,phase:3,deny\""
     ]

test/test-cases/regression/request-body-parser-xml.json

@@ -43,6 +43,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500005,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateSchema test-cases/data/SoapEnvelope.xsd\" \"id:500007,phase:3,deny\""
     ]
@@ -92,6 +93,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateSchema test-cases/data/SoapEnvelope.xsd\" \"id:500007,phase:3,deny\""
     ]
@@ -141,6 +143,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateSchema test-cases/data/SoapEnvelope.xsd\" \"id:500007,phase:3,deny\""
     ]
@@ -190,6 +193,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateSchema test-cases/data/SoapEnvelope.xsd\" \"id:500007,phase:3,deny\""
     ]
@@ -199,7 +203,8 @@
     "version_min":300000,
     "title":"Testing XML request body parser (bad schema)",
     "expected":{
-      "parser_error": " XML: Failed to load Schema: test-cases/data/SoapEnvelope-bad.xsd. XML Error: Failed to parse the XML resource 'test-cases/data/SoapEnvelope-bad.xsd"
+      "debug_log": "XML: Failed to load Schema: test-cases/data/SoapEnvelope-bad.xsd. XML Error: Failed to parse the XML resource 'test-cases/data/SoapEnvelope-bad.xsd",
+      "http_code": 403
     },
     "client":{
       "ip":"200.249.12.31",
@@ -238,6 +243,7 @@
     "rules":[
         "SecRuleEngine On",
         "SecRequestBodyAccess On",
+        "SecXMLExternalEntity On",
         "SecRule REQUEST_HEADERS:Content-Type \"^text/xml$\" \"id:500008,phase:1,t:none,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML\"",
         "SecRule XML \"@validateSchema test-cases/data/SoapEnvelope-bad.xsd\" \"id:500007,phase:3,deny\""
     ]