Text in squid:S2755 issue

mattharr · March 12, 2019, 7:56pm

Hi, we just upgraded to 7.6 from 7.4, and doing an analysis picked up a new issue around XXE, which is great.
I just wanted to note that in the text explaining the issue it reads:

To disable external entity processing for SAXParserFactory , XMLReader or DocumentBuilderFactory configure one of the properties XMLConstants.FEATURE_SECURE_PROCESSING or "http://apache.org/xml/features/disallow-doctype-decl" to true.

I wonder if this should read “… configure one of the features …” instead? Very minor, but this object has both setProperty and setFeature functions, so the wording as it is gave me a moment to pause, and just check what was meant.

Thanks,

Matt

Alexandre_Gigleux · March 12, 2019, 8:35pm

Hello @mattharr,

Welcome to the SonarQube Community!

For your first post, it’s a good catch. For SAXParserFactory, XMLReader or DocumentBuilderFactory, we should say: configure the feature using setFeature.

I changed the description of the rule accordingly. You will see the change in a couple of weeks when we will release SonarJava 5.12.

Regards

mattharr · March 12, 2019, 9:29pm

Hi, thanks very much. I’ve actually posted stuff before (probably a few years ago now), and it was either on an old forum, or I forgot my login details But yes, sounds good, that should make it clearer.

Cheers,

Matt

hosszubalazs · May 10, 2019, 7:29am

Hey guys!

To piggyback on this topic, the rule description contains a typo. It says:
“during parsing and igore any DTD’s included …”

It should read ignore instread of igore. Relevant source file linked:

github.com

SonarSource/sonar-java/blob/fa98a3068293ab9d0885cd456ba07883ccaf07a1/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S2755_java.html

<p>Allowing external entities in untrusted documents to be processed could lay your systems bare to attackers. Imagine if these entities were
parsed:</p>
<pre>
&lt;!ENTITY xxe SYSTEM "file:///etc/passwd" &gt;]&gt;&lt;foo&gt;&amp;xxe;&lt;/foo&gt;
&lt;!ENTITY xxe SYSTEM "http://www.attacker.com/text.txt" &gt;]&gt;&lt;foo&gt;&amp;xxe;&lt;/foo&gt;
</pre>
<p>If you must parse untrusted XML, the best way to protect yourself is to use a local, static DTD during parsing and igore any DTD's included in
included in the document.</p>
<p>This rule raises an issue when any of the following are used without first disabling external entity processing:
<code>javax.xml.validation.Validator</code>, JAXP's <code>DocumentBuilderFactory</code>, <code>SAXParserFactory</code>, Xerces 1 and Xerces 2 StAX's
<code>XMLInputFactory</code> and <code>XMLReaderFactory</code>.</p>
<p>To disable external entity processing for <code>XMLInputFactory</code>, configure one of the properties
<code>XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES</code> or <code>XMLInputFactory.SUPPORT_DTD</code> to false.</p>
<p>To disable external entity processing for <code>SAXParserFactory</code>, <code>XMLReader</code> or <code>DocumentBuilderFactory</code> configure
one of the features <code>XMLConstants.FEATURE_SECURE_PROCESSING</code> or <code>"http://apache.org/xml/features/disallow-doctype-decl"</code> to
true.</p>
<p>To disable external entity processing for <code>Validator</code>, configure both properties <code>XMLConstants.ACCESS_EXTERNAL_DTD</code>,
<code>XMLConstants.ACCESS_EXTERNAL_SCHEMA</code> to the empty string <code>""</code>.</p>
<h2>Noncompliant Code Example</h2>
<pre>

This file has been truncated. show original

@Alexandre_Gigleux should I open a new ticket or is this fine?

Thanks and cheers,
Balázs

saberduck · May 10, 2019, 8:30am

hello @hosszubalazs,

I fixed the typo, thanks. It will become visible with the next release of java analyzer.