Skip to content

Commit

Permalink
Modify S6373: Change text to the LaYC format (APPSEC-1108) (#3162)
Browse files Browse the repository at this point in the history
  • Loading branch information
gaetan-ferry-sonarsource authored Sep 28, 2023
1 parent 8ab8c5b commit f1c3564
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 87 deletions.
2 changes: 2 additions & 0 deletions rules/S6373/java/common/code-rationale.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The following code is vulnerable because it explicitly enables the `xinclude`
feature.
25 changes: 25 additions & 0 deletions rules/S6373/java/how-to-fix/dom4j.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
== How to fix it in Dom4j

=== Code examples

include::../common/code-rationale.adoc[]

==== Noncompliant code example

[source,java,diff-id=2,diff-type=noncompliant]
----
import org.dom4j.io.SAXReader;
SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----

==== Compliant solution

[source,java,diff-id=2,diff-type=compliant]
----
import org.dom4j.io.SAXReader;
SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/xinclude", false);
----
29 changes: 29 additions & 0 deletions rules/S6373/java/how-to-fix/java-se.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
== How to fix it in Java SE

=== Code examples

include::../common/code-rationale.adoc[]

==== Noncompliant code example

[source,java,diff-id=1,diff-type=noncompliant]
----
import javax.xml.parsers.SAXParserFactory;
SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setXIncludeAware(true); // Noncompliant
factory.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----

==== Compliant solution

[source,java,diff-id=1,diff-type=compliant]
----
import javax.xml.parsers.SAXParserFactory;
SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setXIncludeAware(false);
factory.setFeature("http://apache.org/xml/features/xinclude", false);
----
25 changes: 25 additions & 0 deletions rules/S6373/java/how-to-fix/jdom2.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
== How to fix it in Jdom2

=== Code examples

include::../common/code-rationale.adoc[]

==== Noncompliant code example

[source,java,diff-id=3,diff-type=noncompliant]
----
import org.jdom2.input.SAXBuilder;
SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----

==== Compliant solution

[source,java,diff-id=3,diff-type=compliant]
----
import org.jdom2.input.SAXBuilder;
SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/xinclude", false);
----
142 changes: 55 additions & 87 deletions rules/S6373/java/rule.adoc
Original file line number Diff line number Diff line change
@@ -1,112 +1,80 @@
== Why is this an issue?

include::../description.adoc[]

=== Noncompliant code example

For https://docs.oracle.com/javase/9/docs/api/javax/xml/parsers/DocumentBuilderFactory.html[DocumentBuilder], https://docs.oracle.com/javase/9/docs/api/javax/xml/parsers/SAXParserFactory.html[SAXParser], https://docs.oracle.com/javase/9/docs/api/javax/xml/stream/XMLInputFactory.html[XMLInput], https://docs.oracle.com/javase/9/docs/api/javax/xml/transform/TransformerFactory.html[Transformer] and https://docs.oracle.com/javase/9/docs/api/javax/xml/validation/SchemaFactory.html[Schema] JAPX factories:

[source,java]
----
factory.setXIncludeAware(true); // Noncompliant
// or
factory.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----
XML standard allows the inclusion of XML files with the `xinclude` element.
When an XML parser component is set up with the `\http://apache.org/xml/features/xinclude`
feature, it will follow the standard and allow the inclusion of remote files.

For https://dom4j.github.io/[Dom4j] library:

[source,java]
----
SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----

For http://www.jdom.org/[Jdom2] library:
== Why is this an issue?

[source,java]
----
SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/xinclude", true); // Noncompliant
----
When the XML parser will encounter an `xinclude` element, it will try to load
the file pointed to by the `href` attribute into the document. Included files
can either be local files found on the file system of the application server,
or remote files that are downloaded over HTTP, SMB, or other protocols,
depending on the capabilities of the application and server.

=== Compliant solution
The files that can be accessed that way are only limited by the entitlement of
the application on the local system and the network filtering the server is
subject to.

Xinclude is disabled by default and can be explicitely disabled like below.
This issue is particularly severe when the XML parser is used to parse untrusted
documents. For example, when user-submitted XML messages are parsed that way.

For https://docs.oracle.com/javase/9/docs/api/javax/xml/parsers/DocumentBuilderFactory.html[DocumentBuilder], https://docs.oracle.com/javase/9/docs/api/javax/xml/parsers/SAXParserFactory.html[SAXParser], https://docs.oracle.com/javase/9/docs/api/javax/xml/stream/XMLInputFactory.html[XMLInput], https://docs.oracle.com/javase/9/docs/api/javax/xml/transform/TransformerFactory.html[Transformer] and https://docs.oracle.com/javase/9/docs/api/javax/xml/validation/SchemaFactory.html[Schema] JAPX factories:
=== What is the potential impact?

[source,java]
----
factory.setXIncludeAware(false);
// or
factory.setFeature("http://apache.org/xml/features/xinclude", false);
----
Allowing the inclusion of arbitrary files in XML documents can have two main
consequences depending on what type of file is included: local or remote.

For https://dom4j.github.io/[Dom4j] library:
==== Sensitive file disclosure

[source,java]
----
SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/xinclude", false);
----
If the application allows the inclusion of arbitrary files through the use of
the `xinclude` element, it might be used to disclose arbitrary files from the
local file system. Depending on the application's permissions on the file
system, configuration files, runtime secrets, or Personally Identifiable
Information could be leaked.

For http://www.jdom.org/[Jdom2] library:
This is particularly true if the affected parser is used to process untrusted
XML documents.

[source,java]
----
SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/xinclude", false);
----
==== Server-side request forgery

=== Exceptions
When used to retrieve remote files, the application will send network requests
to remote hosts. Moreover, it will do so from its current network location,
which can have severe consequences if the application server is located on a
sensitive network, such as the company corporate network or a DMZ hosting other
applications.

This rule does not raise issues when Xinclude is enabled with a custom ``++EntityResolver++``:
Attackers exploiting this issue could try to access internal backend services or
corporate file shares. It could allow them to access more sensitive files,
bypass authentication mechanisms from frontend applications, or exploit further
vulnerabilities in the local services. Note that, in some cases, the requests
sent from the application can be automatically authenticated on federated
locations. This is often the case in Windows environments when using Active
Directory federated authentication.

For DocumentBuilderFactory:
include::how-to-fix/java-se.adoc[]

[source,java]
----
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setXIncludeAware(true);
// ...
DocumentBuilder builder = factory.newDocumentBuilder();
builder.setEntityResolver((publicId, systemId) -> new MySafeEntityResolver(publicId, systemId));
----
include::how-to-fix/dom4j.adoc[]

For SAXBuilder:
include::how-to-fix/jdom2.adoc[]

[source,java]
----
SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/xinclude", true);
builder.setEntityResolver((publicId, systemId) -> new MySafeEntityResolver(publicId, systemId));
----
=== How does this work?

For SAXReader:
The compliant code example explicitly prevents the inclusion of files in XML
documents by setting the `\http://apache.org/xml/features/xinclude` feature
property to `false`.

[source,java]
----
SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/xinclude", true);
xmlReader.setEntityResolver((publicId, systemId) -> new MySafeEntityResolver(publicId, systemId));
----
== Resources

For XMLInputFactory:
=== Documentation

[source,java]
----
XMLInputFactory factory = XMLInputFactory.newInstance();
factory.setProperty("http://apache.org/xml/features/xinclude", true);
factory.setXMLResolver(new MySafeEntityResolver());
----
* OWASP - https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java[OWASP XXE Prevention Cheat Sheet]
* Java documentation - https://docs.oracle.com/en/java/javase/13/security/java-api-xml-processing-jaxp-security-guide.html#GUID-8CD65EF5-D113-4D5C-A564-B875C8625FAC[XML External Entity Injection Attack]
* W3C - https://www.w3.org/TR/xinclude-11/[XML Inclusions (XInclude) Version 1.1]

== Resources
=== Standards

* https://docs.oracle.com/en/java/javase/13/security/java-api-xml-processing-jaxp-security-guide.html#GUID-8CD65EF5-D113-4D5C-A564-B875C8625FAC[Oracle Java Documentation] - XML External Entity Injection Attack
* https://owasp.org/www-project-top-ten/2017/A4_2017-XML_External_Entities_(XXE)[OWASP Top 10 2017 Category A4] - XML External Entities (XXE)
* https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java[OWASP XXE Prevention Cheat Sheet]
* https://cwe.mitre.org/data/definitions/611[MITRE, CWE-611] - Information Exposure Through XML External Entity Reference
* https://cwe.mitre.org/data/definitions/827[MITRE, CWE-827] - Improper Control of Document Type Definition
* OWASP - https://owasp.org/www-project-top-ten/2017/A4_2017-XML_External_Entities_(XXE)[Top 10 2017 - Category A4 - XML External Entities (XXE)]
* OWASP - https://owasp.org/Top10/A05_2021-Security_Misconfiguration/[Top 10 2021 - Category A5 - Security Misconfiguration]
* CWE - https://cwe.mitre.org/data/definitions/611[CWE-611 - Improper Restriction of XML External Entity Reference]
* CWE - https://cwe.mitre.org/data/definitions/827[CWE-827 - Improper Control of Document Type Definition]

ifdef::env-github,rspecator-view[]

Expand Down

0 comments on commit f1c3564

Please sign in to comment.