diff --git a/core/src/main/java/lucee/runtime/text/xml/XMLUtil.java b/core/src/main/java/lucee/runtime/text/xml/XMLUtil.java
index 2f6ba2a2378..3764f062375 100755
--- a/core/src/main/java/lucee/runtime/text/xml/XMLUtil.java
+++ b/core/src/main/java/lucee/runtime/text/xml/XMLUtil.java
@@ -328,6 +328,12 @@ private static DocumentBuilderFactory newDocumentBuilderFactory(InputSource vali
factory.setValidating(false);
}
+ // secure by default LDEV-3451
+ boolean featureSecure = true;
+ boolean disallowDocType = true;
+ boolean externalGeneralEntities = false;
+
+ // can be overriden per application
PageContext pc = ThreadLocalPageContext.get();
if (pc != null) {
ApplicationContextSupport ac = ((ApplicationContextSupport) pc.getApplicationContext());
@@ -335,54 +341,56 @@ private static DocumentBuilderFactory newDocumentBuilderFactory(InputSource vali
if (features != null) {
try { // handle feature aliases, e.g. secure
Object obj;
- boolean featureValue;
+
obj = features.get(KEY_FEATURE_SECURE, null);
- if (obj != null) {
- featureValue = Caster.toBoolean(obj);
- if (featureValue) {
- // set features per
- // https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
- factory.setFeature(XMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL, true);
- factory.setFeature(XMLConstants.FEATURE_EXTERNAL_GENERAL_ENTITIES, false);
- factory.setFeature(XMLConstants.FEATURE_EXTERNAL_PARAMETER_ENTITIES, false);
- factory.setFeature(XMLConstants.FEATURE_NONVALIDATING_LOAD_EXTERNAL_DTD, false);
- factory.setXIncludeAware(false);
- factory.setExpandEntityReferences(false);
- factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
- factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
- }
- features.remove(KEY_FEATURE_SECURE);
- }
+ if (obj != null) featureSecure = Caster.toBoolean(obj);
obj = features.get(KEY_FEATURE_DISALLOW_DOCTYPE_DECL, null);
- if (obj != null) {
- featureValue = Caster.toBoolean(obj);
- factory.setFeature(XMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL, featureValue);
- features.remove(KEY_FEATURE_DISALLOW_DOCTYPE_DECL);
- }
+ if (obj != null) disallowDocType = Caster.toBoolean(obj);
obj = features.get(KEY_FEATURE_EXTERNAL_GENERAL_ENTITIES, null);
- if (obj != null) {
- featureValue = Caster.toBoolean(obj);
- factory.setFeature(XMLConstants.FEATURE_EXTERNAL_GENERAL_ENTITIES, featureValue);
- features.remove(KEY_FEATURE_EXTERNAL_GENERAL_ENTITIES);
- }
+ if (obj != null) externalGeneralEntities = Caster.toBoolean(obj);
}
- catch (PageException | ParserConfigurationException ex) {
+ catch (PageException ex) {
throw new RuntimeException(ex);
}
-
- features.forEach((k, v) -> {
- try {
- factory.setFeature(k.toString().toLowerCase(), Caster.toBoolean(v));
- }
- catch (PageException | ParserConfigurationException ex) {
- throw new RuntimeException(ex);
- }
- });
}
}
-
+
+ try { // handle feature aliases, e.g. secure
+ if (featureSecure) {
+ // set features per
+ // https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+ factory.setFeature(XMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL, true);
+ factory.setFeature(XMLConstants.FEATURE_EXTERNAL_GENERAL_ENTITIES, false);
+ factory.setFeature(XMLConstants.FEATURE_EXTERNAL_PARAMETER_ENTITIES, false);
+ factory.setFeature(XMLConstants.FEATURE_NONVALIDATING_LOAD_EXTERNAL_DTD, false);
+ factory.setXIncludeAware(false);
+ factory.setExpandEntityReferences(false);
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+ }
+ //features.remove(KEY_FEATURE_SECURE);
+
+ factory.setFeature(XMLConstants.FEATURE_DISALLOW_DOCTYPE_DECL, disallowDocType);
+ //features.remove(KEY_FEATURE_DISALLOW_DOCTYPE_DECL);
+
+ factory.setFeature(XMLConstants.FEATURE_EXTERNAL_GENERAL_ENTITIES, externalGeneralEntities);
+ //features.remove(KEY_FEATURE_EXTERNAL_GENERAL_ENTITIES);
+ }
+ catch (ParserConfigurationException ex) {
+ throw new RuntimeException(ex);
+ }
+ /*
+ features.forEach((k, v) -> {
+ try {
+ factory.setFeature(k.toString().toLowerCase(), Caster.toBoolean(v));
+ }
+ catch (PageException | ParserConfigurationException ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+ */
return factory;
}
diff --git a/test/tickets/LDEV1676.cfc b/test/tickets/LDEV1676.cfc
index 12586a7a6f7..57fd22cdf9b 100644
--- a/test/tickets/LDEV1676.cfc
+++ b/test/tickets/LDEV1676.cfc
@@ -1,32 +1,125 @@
component extends = "org.lucee.cfml.test.LuceeTestCase" labels="xml" {
function beforeAll(){
variables.uri = createURI("LDEV1676");
+ //systemOutput(" ", true);
}
function run( testresults , testbox ) {
describe( "testcase for LDEV-1676", function () {
- it( title="Check xmlFeatures externalGeneralEntities=true",body = function ( currentSpec ){
+ it( title="Check xmlFeatures externalGeneralEntities=true, secure: false",body = function ( currentSpec ){
local.result = _InternalRequest(
- template : "#uri#\LDEV1676.cfm",
- forms : {scene=1}
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "externalGeneralEntities-True" }
).filecontent;
- expect(trim(result)).toBe("http://update.lucee.org/rest/update/provider/echoGet/cgi");
+ expect( trim( result ) ).toInclude("http://update.lucee.org/rest/update/provider/echoGet/cgi");
});
it( title="Check xmlFeatures externalGeneralEntities=false",body = function ( currentSpec ) {
local.result = _InternalRequest(
- template : "#uri#\LDEV1676.cfm",
- forms : {scene=2}
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "externalGeneralEntities-False" }
).filecontent;
- expect(trim(result)).toInclude("security restrictions set by XMLFeatures");
+ expect( trim( result ) ).toInclude("security restrictions set by XMLFeatures");
+ expect( trim( result ) ).toInclude("NullPointerException");
});
it( title="Check xmlFeatures disallowDoctypeDecl=true",body = function ( currentSpec ) {
local.result = _InternalRequest(
- template : "#uri#\LDEV1676.cfm",
- forms : {scene=3}
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "disallowDoctypeDecl-True" }
).filecontent;
- expect(trim(result)).toInclude("DOCTYPE");
+ expect( trim( result ) ).toInclude("DOCTYPE");
+ });
+ });
+
+ describe( "check combined xmlFeatures directives", function () {
+
+ it( title="Check xmlFeatures default, good xml",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : {
+ scene: "default",
+ doctype: false,
+ entity: false,
+ }
+ ).filecontent;
+ expect( trim( result ) ).toBe("lucee");
+ });
+
+ it( title="Check xmlFeatures default, bad xml",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : {
+ scene: "default",
+ doctype: true,
+ entity: true
+ }
+ ).filecontent;
+ expect( trim( result ) ).toInclude("DOCTYPE is disallowed when the feature");
+ });
+
+ it( title="Check xmlFeatures all secure, bad xml",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : {
+ scene: "all-secure",
+ doctype: true,
+ entity: true,
+ }
+ ).filecontent;
+ expect( trim( result ) ).toInclude("DOCTYPE is disallowed when the feature");
+ });
+
+ it( title="Check xmlFeatures all insecure, bad xml",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : {
+ scene: "all-insecure",
+ doctype: true,
+ entity: true
+ }
+ ).filecontent;
+ expect( trim( result ) ).toInclude("http://update.lucee.org/rest/update/provider/echoGet/cgi");
+ });
+
+ it( title="Check xmlFeatures all secure, good xml",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : {
+ scene: "all-secure",
+ doctype: false,
+ entity: false
+ }
+ ).filecontent;
+ expect( trim( result ) ).toBe("lucee");
+ });
+
+ });
+
+ xdescribe( "check bad config handling", function () {
+
+ it( title="Check xmlFeatures invalidConfig secure",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "invalidConfig-secure" }
+ ).filecontent;
+ expect( trim( result ) ).toInclude( "casterException" );
+ });
+
+ it( title="Check xmlFeatures invalidConfig docType",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "invalidConfig-docType" }
+ ).filecontent;
+ expect( trim( result ) ).toInclude( "casterException" );
+ });
+
+ it( title="Check xmlFeatures invalidConfig Entities",body = function ( currentSpec ) {
+ local.result = _InternalRequest(
+ template : "#uri#/LDEV1676.cfm",
+ forms : { scene: "invalidConfig-Entities" }
+ ).filecontent;
+ expect( trim( result ) ).toInclude( "casterException" );
});
});
@@ -36,4 +129,4 @@ component extends = "org.lucee.cfml.test.LuceeTestCase" labels="xml" {
var baseURI="/test/#listLast(getDirectoryFromPath(getCurrenttemplatepath()),"\/")#/";
return baseURI&""&calledName;
}
-}
\ No newline at end of file
+}
diff --git a/test/tickets/LDEV1676/Application.cfc b/test/tickets/LDEV1676/Application.cfc
index e302097293a..5a3889b39ab 100644
--- a/test/tickets/LDEV1676/Application.cfc
+++ b/test/tickets/LDEV1676/Application.cfc
@@ -1,24 +1,64 @@
component {
this.name="LDEV1676";
- param name="FORM.Scene" default="";
+ param name="FORM.Scene";
+ param name="FORM.docType" default="true";
+ param name="FORM.entity" default="true";
- if(FORM.Scene == 1) {
- this.xmlFeatures.externalGeneralEntities = true;
- }
-
- else if(FORM.Scene == 2) {
- this.xmlFeatures = {
- externalGeneralEntities: false,
- secure: true,
- disallowDoctypeDecl: false
- };
- }
-
- else if(FORM.Scene == 3) {
- this.xmlFeatures = {
- externalGeneralEntities: false,
- secure: true,
- disallowDoctypeDecl: true
- };
+ switch (FORM.Scene){
+ case "externalGeneralEntities-True":
+ this.xmlFeatures ={
+ "externalGeneralEntities": true,
+ "disallowDoctypeDecl": false,
+ "secure": false
+ }
+ break;
+ case "externalGeneralEntities-False":
+ this.xmlFeatures = {
+ "externalGeneralEntities": false,
+ "secure": true,
+ "disallowDoctypeDecl": false
+ };
+ break;
+ case "disallowDoctypeDecl-True":
+ this.xmlFeatures = {
+ "externalGeneralEntities": false,
+ "secure": true,
+ "disallowDoctypeDecl": true
+ };
+ break;
+ case "invalidConfig-Secure":
+ this.xmlFeatures = {
+ "secure": "lucee"
+ };
+ break;
+ case "invalidConfig-Doctype":
+ this.xmlFeatures = {
+ "disallowDoctypeDecl": "lucee"
+ };
+ break;
+ case "invalidConfig-Entities":
+ this.xmlFeatures = {
+ "disallowDoctypeDecl": "lucee"
+ };
+ break;
+ case "all-secure":
+ this.xmlFeatures = {
+ "externalGeneralEntities": false,
+ "secure": true,
+ "disallowDoctypeDecl": true
+ };
+ break;
+ case "all-insecure":
+ this.xmlFeatures = {
+ "externalGeneralEntities": true,
+ "secure": false,
+ "disallowDoctypeDecl": false
+ };
+ break;
+ case "default":
+ break;
+ default:
+ throw "unknown scene: #form.scene#";
+ break;
}
}
\ No newline at end of file
diff --git a/test/tickets/LDEV1676/LDEV1676.cfm b/test/tickets/LDEV1676/LDEV1676.cfm
index fa0ae7e0faa..baf29f238c1 100644
--- a/test/tickets/LDEV1676/LDEV1676.cfm
+++ b/test/tickets/LDEV1676/LDEV1676.cfm
@@ -1,14 +1,39 @@
-
-
- ]>
- &xxe;
-
-
-
-
- #cfcatch.message#
-
-
-
\ No newline at end of file
+
+
+
+
+
+ ]>
+
+
+ &xxe;
+
+ lucee
+
+
+
+ /*
+ settings = getApplicationSettings();
+
+ systemOutput( form.toJson(), true );
+ if (structKeyExists(settings, "xmlFeatures" ) ) {
+ systemOutput( settings.xmlFeatures.toJson(), true );
+ } else {
+ systemOutput("xmlFeatures not set", true);
+ }
+ systemOutput( "LDEV1676.cfc:" & CallStackGet( "array" )[ 2 ].linenumber, true );
+ systemOutput( xml, true );
+ */
+ try {
+ result = xmlSearch( xml, "/foo" )[1].xmltext;
+ //systemOutput( result, true );
+ echo( result );
+ } catch (e) {
+
+ //systemOutput(cfcatch.type & " " & cfcatch.message, true);
+
+ echo( cfcatch.type & " " & cfcatch.message );
+ }
+
\ No newline at end of file