Parser options for DISABLE_EXTERNAL_ENTITIES and ENTITY_RESOLVER

Description

It is possible to perform an XML External Entity Injection attack in an XML files parsed by Geotools. Further details on XEE can be found here: https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet

A quick project put together to demonstrate the bug can be found here: https://github.com/aaronwaddell/geotools-xml-entity-injection

The original StackExchange question regarding the issue can be found here: http://gis.stackexchange.com/questions/209377/prevent-xml-external-entity-injections-in-geotools-wms-client

OWASP suggest disabling DTDs in order to prevent this kind of attack. It appears that Geotools was once doing this but the lines have since been commented out: org.geotools.xml.DocumentFactory:152-153. It is possible that this was done as a part of development some time ago and was never picked up on because of the lack unit tests for this class (the methods are static so I assume this is the reason).

If disabling DTDs won't have a negative impact on dependent modules then I suggest we comment out these lines which had been confirmed to fix the issue. I also suggest that the use of PowerMock or the like is considered in order to allow for testing for static methods so that there is no regression.

Environment

None

Activity

Show:
David Blasby
September 21, 2016, 6:00 PM

Hi, Andrea (et al),

Thanks for putting out these changes so quickly!

I heard that this issue could be affecting WFS, WPS, and GWC - do you have
any information on this?
I see some changes in the PRs in WPS and WFS (but nothing in GWC).

Or do these two PRs fix everything?

Also, does this issue only affect cascading WMS?

Thanks for the info - I'm hearing conflicting rumours and trying to track
down what's actually the case.

Also, I'd like to do some testing on this - but not quite sure how. If you
(or someone else) can give use some guidance on testing (including possible
regressions), I can do some manual testing...

Cheers,
Dave

On Wed, Sep 21, 2016 at 9:57 AM, Andrea Aime <andrea.aime@geo-solutions.it>

------------------------------------------------------------------------------

_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Jody Garnett
September 28, 2016, 11:15 PM
Edited

Thanks for the hard work on this Andrea - renamed the issue for the release notes.

So the fix has gone live with two approaches:

  • the nuke from orbit option to disable entity resolution (as per original patch)

  • entity resolver option, combined with an entityresolver that does not serve out files. This is the one used by gt-wms and the geoserver codebase - in part to support the WMS 1.1.1 specification that requires DTD support

There were several changes to the gt-wms codebase to allow hints to be supplied and used.

Jody Garnett
October 5, 2016, 12:07 AM

Reviewing the use of hints from GeoWebCache I have a problem, the following code does not quite work as expected when passing in a ENTITY_RESOLVER to WebMapService version negotiation:

In stead we want to preserve the provided entity resolver hint, and make use of the VALIDATION_HINT and DEFAULT_NAMESPACE_HINT_KEY required for GetCapabilities parsing.

Going to try out the following:

I will need to review each time the if( hints == null ) check is performed.

Andrea Aime
February 15, 2017, 11:34 AM

Mass transitioning all resolved issues that have not been updated in the last month to closed state

Former user
March 9, 2017, 1:20 PM

that really is a bad idea!
Since you are talking about modifying a geotools 15.1 library I assume
you are already running geoserver 2.9.1, so upgrading to 2.9.4 is a
trivial exercise and faily risk-free.

-M

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
_______________________________________________
Geoserver-users mailing list
Geoserver-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-users

Fixed

Assignee

Andrea Aime

Reporter

Aaron Waddell

Triage

None

Components

Fix versions

Affects versions

Priority

High