Skip to content

Commit

Permalink
LDEV-3451 disable XXE by default
Browse files Browse the repository at this point in the history 
https://luceeserver.atlassian.net/browse/LDEV-3451
  • Loading branch information
@zspitzer
zspitzer committed Jul 18, 2023
1 parent cb1c6b2 commit d09e664
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 81 deletions.
84 changes: 46 additions & 38 deletions core/src/main/java/lucee/runtime/text/xml/XMLUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,61 +328,69 @@ 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());
Struct features = ac == null ? null : ac.getXmlFeatures();
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;
}

Expand Down
115 changes: 104 additions & 11 deletions test/tickets/LDEV1676.cfc
View file
Original file line number Diff line number Diff line change
@@ -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" );
});

});
Expand All @@ -36,4 +129,4 @@ component extends = "org.lucee.cfml.test.LuceeTestCase" labels="xml" {
var baseURI="/test/#listLast(getDirectoryFromPath(getCurrenttemplatepath()),"\/")#/";
return baseURI&""&calledName;
}
}
}
78 changes: 59 additions & 19 deletions test/tickets/LDEV1676/Application.cfc
View file
Original file line number Diff line number Diff line change
@@ -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;
}
}
51 changes: 38 additions & 13 deletions test/tickets/LDEV1676/LDEV1676.cfm
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,39 @@
<cfoutput>
<cfsavecontent variable="xml"><?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE foo [
<!ELEMENT foo ANY >
<!ENTITY xxe SYSTEM "http://update.lucee.org/rest/update/provider/echoGet/cgi" >]>
<foo>&xxe;</foo>
</cfsavecontent>
<cftry>
<cfset results = xmlSearch(xml, "/foo")>
<cfcatch>
#cfcatch.message#
</cfcatch>
</cftry>
</cfoutput>
<cfif form.docType>
<!DOCTYPE foo [
<!ELEMENT foo ANY >
<cfif FORM.entity>
<!ENTITY xxe SYSTEM "http://update.lucee.org/rest/update/provider/echoGet/cgi" >
</cfif>
]>
</cfif>
<cfif form.entity>
<foo>&xxe;</foo>
<cfelse>
<foo>lucee</foo>
</cfif>
</cfsavecontent>
<cfscript>
/*
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 );
}
</cfscript>

0 comments on commit d09e664

Please sign in to comment.