WTP - Ignore external URIs by default

[fvgh]

Eclipse WTP XML formatter resolves automatically external URIs. URIs that cannot be accessed are ignored. If the referenced external DTD/XSD contains formatting instructions, the formatter results differs depending whether the URI is reachable or not.
This is not appropriate behaviour for Spotless, since it is e.g. used for continuous integration testing.
Normally all external URIs shall be included in a XML catalog during build.
But many users may not be aware that the XML formatter uses the DTDs and XSDs specified in an XML in the first place.
To prevent an spotlessCheck error due failure accessing an external DTD/XSD, the Spotless default behaviour has been changed, to ignore external URIs per default.

[fvgh]

@nedtwigg Let me know when you released the new version and I shall integrate it. Or feel ree to do the integration yourself.
@JLLeitschuh If you are still interested to do some investigation on #358, this PR might give you some ideas where to look.

[nedtwigg]

This looks great! Release is on the way..

[nedtwigg]

ext-eclipse-wtp 3.9.8 is now available on mavencentral

[nedtwigg]

Released in 3.20.0 / 1.20.0

[JLLeitschuh]

@fvgh Should I open an upstream issue with the Eclipse organization about this?

[fvgh]

@JLLeitschuh As stated in this PR description, I don't see that the Eclipse behaviour is a problem for an IDE, but I admit that it is unexpected for Spotless users and a problem for a build tool.
The Eclipse resolveExternalEntities configuration is only applied on the Apache Xerces validator (for the security reasons you pointed out) and we discussed in #358. On the other hand, the configuration item is in the Eclipse XML preferences below the "Validation" node. So I do not see a need on Eclipse side to change anything until someone finds a vulnerability in the formatter.

[JLLeitschuh]

As stated in this PR description, I don't see that the Eclipse behaviour is a problem for an IDE

Actually, it is. Maliciously compromising was one of the major discoveries of the Checkpoint team in their ParseDroid Vulnerability.

This led us to find multiple vulnerable implementations of the XML parser within other projects. Moreover, we identified that the most popular IDEs that are used for building Android applications are affected – including Intellij, Eclipse, and Android Studio.

By simply loading the malicious “AndroidManifest.xml” file as part of any Android project, the IDEs starts spitting out any file configured by the attacker.

To demonstrate this vulnerability, we have uploaded a malicious project library to GitHub and cloned it to an Android Studio project.
- https://research.checkpoint.com/parsedroid-targeting-android-development-research-community/

[fvgh]

Sorry @JLLeitschuh , as I stressed in #358:

I have no evidence and don't think that the Eclipse XMLSourceParser is vulnerable to the XXE scenario where a local file URI is used as external entity as described within this issue.

Its the question what you consider a security problem. If you consider the increase of an attack vector a security problem, yes allowing to process data obtained via HTTP is a problem.

Let's just agree to disagree 😉 ...

But don't get me wrong: I appreciate very much that you highlighted the topic that the user is probably not aware that DTD/XSDs are automatically downloaded. I am afraid I was already too used to this behaviour and catalogue usage...

[JLLeitschuh]

@fvgh Thanks for your professional discourse about this and for fixing the issue. I'll take care of reaching out to the Eclipse security team and see what they have to say.

Again, thank you so much for your hard work on this!