SAXReader uses system XMLReaderFactory.createXMLReader() or SAXParserFactory.newInstance().newSAXParser() which has unsecure defaults

[FilipJirsak]

The constructor new org.dom4j.io.SAXReader() calls one of the factory method form Java runtime library – org.xml.sax.helpers.XMLReaderFactory.createXMLReader() or javax.xml.parsers.SAXParserFactory.newInstance().newSAXParser(). These factory methods do not have safe defaults, such as downloading external entities.
Create the new factory method org.dom4j.io.SAXReader.createDefault() which overrides Java runtime library defaults and sets following features:

reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);	
reader.setFeature("http://xml.org/sax/features/external-general-entities", false);	
reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false);

[FilipJirsak]

Fixed in 2.0.3 and 2.1.3.

[kpshek]

For posterity, this issue (and commit a822852) fixes CVE-2020-10683.

I have requested an update to the description and references for CVE-2020-10683 to note that this issue was fixed with both 2.0.3 and 2.1.3. I also have submitted a request to update the CPE for the entry in the NVD so that it shows this fix on 2.0.3 as well as 2.1.3.

[carlosame]

This is a bit confusing. I understand that the mentioned commit probably fixes (in 2.0.3) the issue that was present in 2.0.2, but 2.1.1 was not vulnerable. I mean, 2.1.3 effectively applies the 'fix' twice.

Because in 2.1.3, createDefault() calls SAXReader.setFeature(String, boolean) which in turn calls getXMLReader().

And because the xmlReader field was not initialized by the SAXReader() constructor called at the beginning of createDefault(), createXMLReader() is called, which results in SAXHelper.createXMLReader(boolean) being executed, and there you have (and already had in 2.1.1):

// external entites
SAXHelper.setParserFeature(reader, "http://xml.org/sax/properties/external-general-entities", false);
SAXHelper.setParserFeature(reader, "http://xml.org/sax/properties/external-parameter-entities", false);

// external DTD
SAXHelper.setParserFeature(reader,"http://apache.org/xml/features/nonvalidating/load-external-dtd", false);

So your desired defaults are applied here, and then again by the SAXReader.setFeature(String, boolean) call(s) which triggered the aforementioned events.

Moreover, as I argued in #51 the disabling of load-external-dtd is the wrong fix to apply. Silently dropping the entities is a problem by itself, and preventing a possible security problem by silently losing data does not appear like a good tradeoff to me.

The correct behaviour IMHO is to throw an exception if you find entities and you aren't using an EntityResolver, and let the parsing proceed (with safe defaults) if one is set. In css4j's equivalent of SAXReader, called XMLDocumentBuilder, I configure the SAXParserFactory for this.

Once you have an EntityResolver configured, the responsibility for which sources you are accepting is shifted to it, and you may want to use one like my DefaultEntityResolver. The configuration setting that I mentioned should enable the relevant DoS protections in the SAX parser (for hypothetical 'billion laughs attacks' coming from such trusted sources), although a relatively recent parser should be used.

Also, the parser configuration that I'm suggesting is valid for any compliant parser, while the load-external-dtd only applies to Xerces (and apparently there are AElfred forks being used out there, if not other different parsers).

[carlosame]

In commit css4j/css4j-dom4j@23ced62 there are two tests that could be useful in checking whether the load-external-dtd was disabled or not (and thus the dom4j version being vulnerable or not). One is about using the SAXReader() default constructor, and the other checks DocumentHelper.parseText(String). A timeout is also a failure, as it means that a remote DTD retrieval was attempted.

Versions 2.0.3, 2.1.1 and 2.1.3 pass both tests, 2.0.2 fails (haven't checked 2.1.0 but it would fail, and that was covered by an earlier CVE).