Skip to content
Permalink
Browse files Browse the repository at this point in the history
XWIKI-18946: Improve the default XML parser
  • Loading branch information
tmortagne committed Sep 10, 2021
1 parent 3af8680 commit 947e892
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
Expand Up @@ -110,6 +110,18 @@
/** Xerces configuration parameter for disabling fetching and checking XMLs against their DTD. */
private static final String DISABLE_DTD_PARAM = "http://apache.org/xml/features/nonvalidating/load-external-dtd";

/** Xerces configuration parameter for prevent DOCTYPE definition. */
private static final String DISABLE_EXTERNAL_DOCTYPE_DECLARATION =
"http://apache.org/xml/features/disallow-doctype-decl";

/** Xerces configuration parameter for disabling inserting entities defined in external files. */
private static final String DISABLE_EXTERNAL_PARAMETER_ENTITIES =
"http://xml.org/sax/features/external-parameter-entities";

/** Xerces configuration parameter for disabling inserting entities defined in external files. */
private static final String DISABLE_EXTERNAL_GENERAL_ENTITIES =
"http://xml.org/sax/features/external-general-entities";

static {
DOMImplementationLS implementation = null;
try {
Expand Down Expand Up @@ -516,6 +528,17 @@ public static Document parse(LSInput source)
if (p.getDomConfig().canSetParameter(DISABLE_DTD_PARAM, false)) {
p.getDomConfig().setParameter(DISABLE_DTD_PARAM, false);
}

// Avoid XML eXternal Entity injection (XXE)
if (p.getDomConfig().canSetParameter(DISABLE_EXTERNAL_DOCTYPE_DECLARATION, false)) {
p.getDomConfig().setParameter(DISABLE_EXTERNAL_DOCTYPE_DECLARATION, false);
}
if (p.getDomConfig().canSetParameter(DISABLE_EXTERNAL_PARAMETER_ENTITIES, false)) {
p.getDomConfig().setParameter(DISABLE_EXTERNAL_PARAMETER_ENTITIES, false);
}
if (p.getDomConfig().canSetParameter(DISABLE_EXTERNAL_GENERAL_ENTITIES, false)) {
p.getDomConfig().setParameter(DISABLE_EXTERNAL_GENERAL_ENTITIES, false);
}
return p.parse(source);
} catch (Exception ex) {
LOGGER.warn("Cannot parse XML document: [{}]", ex.getMessage());
Expand Down
Expand Up @@ -19,10 +19,19 @@
*/
package org.xwiki.xml;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;

import org.apache.commons.io.FileUtils;
import org.apache.html.dom.HTMLDocumentImpl;
import org.junit.jupiter.api.Test;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.bootstrap.DOMImplementationRegistry;
import org.w3c.dom.html.HTMLElement;
import org.w3c.dom.ls.DOMImplementationLS;
import org.w3c.dom.ls.LSInput;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -279,4 +288,22 @@ void serializeNode()
serialize = XMLUtils.serialize(node, false);
assertEquals("<HTML><HEAD/><BODY class=\"toto\"/></HTML>", serialize);
}

@Test
void disableExternalEntities()
throws ClassNotFoundException, InstantiationException, IllegalAccessException, ClassCastException, IOException
{
File tempFile = File.createTempFile("file", ".txt");

FileUtils.write(tempFile, "external", StandardCharsets.UTF_8);

DOMImplementationLS ls =
(DOMImplementationLS) DOMImplementationRegistry.newInstance().getDOMImplementation("LS 3.0");
LSInput input = ls.createLSInput();
input.setStringData("<?xml version='1.0' encoding='UTF-8'?>" + "<!DOCTYPE root[<!ENTITY xxe SYSTEM 'file://"
+ tempFile.getAbsolutePath() + "' >]><root>&xxe;</root>");

Document result = XMLUtils.parse(input);
assertNotEquals("external", result.getDocumentElement().getTextContent());
}
}

0 comments on commit 947e892

Please sign in to comment.