Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"DOCTYPE disallowed" on ETF release candidate WMS INSPIRE ETS execution #222

Open
carlospzurita opened this issue Mar 16, 2021 · 5 comments

Comments

@carlospzurita
Copy link
Contributor

Description

During the testing of the new release candidate of the ETF, an error has been encountered during the execution of the the INSPIRE WMS Executable Test Suite. This error happens on GetCapabiltiesOperation > at.05 > no request parameter , as shown on the attached TestReport.

The logs for the TestRuns displays this information

ERROR DOCTYPE is disallowed when the feature "http://apache.org/xml/features/disallow-doctype-decl" set to true.

The message does not appear on the production instance of the INSPIRE Reference validator. The mentioned requirement is correctly marked as passed for this WMS endpoint.

Operating systems and browser

Steps to Reproduce

  1. Load INSPIRE ETS on ETF
  2. Create a Test Run using the WMS ETS
  3. Use https://services.bgr.de/wms/inspire_nz/gerseis/?REQUEST=GetCapabilities&SERVICE=WMS&VERSION=1.3.0 as the service endpoint
  4. Check result on GetCapabiltiesOperation > at.05 > no request parameter
  5. Check log for test run

Expected behavior:

Test should marked as passed
Actual behavior:
Test fails due to unexpected error on XML parsing. Error on DOCTYPE declaration appears on log file.

Causes and alternatives

The main suspect to be the source of this issue is the default configuration for the XMLSlurper library that is used on Groovy to parse the contents of the GetCapabilities XML. We have tried to override this check using the solution provided on this post, without success

parser=new XmlSlurper()
parser.setFeature("http://apache.org/xml/features/disallow-doctype-decl", false) 
parser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
parser.parse(it)
@carlospzurita
Copy link
Contributor Author

As further clarification for the issue, what we are trying to accomplish for this feature is to override the check and try to execute the validation as usual for the services ETS, given that the DOCTYPE check is not enforced in the Implementing Rules. This is what we tried on our first approach disabling the feature on the XmlSlurper.

However, we understand that this element can cause some troubles on the ETF, and maybe it is best to leave the check in place and try to circumvent the error somehow.

Do you think the check for the DOCTYPE element is mandatory? And in that case, should we raise an exception as usual wrapping the instance of the XmlSlurper in a try-catch and send an error message to the user in the Test Report? Any feedback that you can offer on this matter is welcome.

@jonherrmann
Copy link
Collaborator

Do you think the check for the DOCTYPE element is mandatory?

Yes, it is. At least for public instances. See this page of the Open Web Application Security Project.

And in that case, should we raise an exception as usual wrapping the instance of the XmlSlurper in a try-catch and send an error message to the user in the Test Report?

It would be good for the user to know what the problem is and the easiest way would be for her/him to see it in the report.

@cportele
Copy link
Contributor

I think there is an added complication that may need a discussion in INSPIRE. INSPIRE has Technical Guidance for both WMS 1.1.1 and WMS 1.3.0.

WMS 1.1.1 is so old, it uses DTD and requires the DOCTYPE declaration. WMS 1.3.0 uses XML Schema and not DTD / DOCTYPE. For security reasons, DOCTYPE and DTD should not be allowed (see the link above), but that is not compatible with WMS 1.1.1. Maybe it would be time to deprecate WMS 1.1.1 in INSPIRE?

As an aside, the behaviour of the WMS is incorrect. The response that creates the issue is a WMS 1.1.1 exception report (hence the DTD reference), but since the service supports WMS 1.3.0 and the request has not requested version=1.1.1 it should have been a WMS 1.3.0 service exception report, which would not have used DOCTYPE and created no failure. However, the general issue of supporting WMS 1.1.1 at all is separate from this.

@jonherrmann
Copy link
Collaborator

jonherrmann commented Mar 19, 2021

Thank you, Clemens. Let's summarise that this issue reveals three points:

  1. the "outdated" INSPIRE TG for WMS
  2. the resulting DTD XXE security issue
  3. the test case that does not check for the correct version in the exception report.

At the implementation level, the problem could at least be mitigated by point 3. That would probably be a good first step, with little effort.

@carlospzurita
Copy link
Contributor Author

Dear all,

After discussing this issue and how to proceed regarding the INSPIRE validator, we have decided to handle the !DOCTYPE declaration at ETS level for WMS.

We are rejecting the file if the response contains that string, before parsing it as an XML, and then inform the user of the issue and sharing the recommendations from OWASP.

This way we reduce the impact on the source code and can be maintained in a simpler manner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants