Skip to content

Commit 87172cf

Browse files
joehnijoehni
committed
Fix XXE vulnerability for JDom parsers.
1 parent 0311e0f commit 87172cf

File tree

4 files changed

+38
-16
lines changed

4 files changed

+38
-16
lines changed

‎xstream/src/java/com/thoughtworks/xstream/io/xml/JDom2Driver.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public JDom2Driver(final NameCoder nameCoder) {
4949
@Override
5050
public HierarchicalStreamReader createReader(final Reader reader) {
5151
try {
52-
final SAXBuilder builder = new SAXBuilder();
52+
final SAXBuilder builder = createBuilder();
5353
final Document document = builder.build(reader);
5454
return new JDom2Reader(document, getNameCoder());
5555
} catch (final IOException e) {
@@ -62,7 +62,7 @@ public HierarchicalStreamReader createReader(final Reader reader) {
6262
@Override
6363
public HierarchicalStreamReader createReader(final InputStream in) {
6464
try {
65-
final SAXBuilder builder = new SAXBuilder();
65+
final SAXBuilder builder = createBuilder();
6666
final Document document = builder.build(in);
6767
return new JDom2Reader(document, getNameCoder());
6868
} catch (final IOException e) {
@@ -75,7 +75,7 @@ public HierarchicalStreamReader createReader(final InputStream in) {
7575
@Override
7676
public HierarchicalStreamReader createReader(final URL in) {
7777
try {
78-
final SAXBuilder builder = new SAXBuilder();
78+
final SAXBuilder builder = createBuilder();
7979
final Document document = builder.build(in);
8080
return new JDom2Reader(document, getNameCoder());
8181
} catch (final IOException e) {
@@ -88,7 +88,7 @@ public HierarchicalStreamReader createReader(final URL in) {
8888
@Override
8989
public HierarchicalStreamReader createReader(final File in) {
9090
try {
91-
final SAXBuilder builder = new SAXBuilder();
91+
final SAXBuilder builder = createBuilder();
9292
final Document document = builder.build(in);
9393
return new JDom2Reader(document, getNameCoder());
9494
} catch (final IOException e) {
@@ -107,4 +107,16 @@ public HierarchicalStreamWriter createWriter(final Writer out) {
107107
public HierarchicalStreamWriter createWriter(final OutputStream out) {
108108
return new PrettyPrintWriter(new OutputStreamWriter(out));
109109
}
110+
111+
/**
112+
* Create and initialize the SAX builder.
113+
*
114+
* @return the SAX builder instance.
115+
* @since upcoming
116+
*/
117+
protected SAXBuilder createBuilder() {
118+
final SAXBuilder builder = new SAXBuilder();
119+
builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
120+
return builder;
121+
}
110122
}

‎xstream/src/java/com/thoughtworks/xstream/io/xml/JDomDriver.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public JDomDriver(final XmlFriendlyReplacer replacer) {
5858
@Override
5959
public HierarchicalStreamReader createReader(final Reader reader) {
6060
try {
61-
final SAXBuilder builder = new SAXBuilder();
61+
final SAXBuilder builder = createBuilder();
6262
final Document document = builder.build(reader);
6363
return new JDomReader(document, getNameCoder());
6464
} catch (final IOException e) {
@@ -71,7 +71,7 @@ public HierarchicalStreamReader createReader(final Reader reader) {
7171
@Override
7272
public HierarchicalStreamReader createReader(final InputStream in) {
7373
try {
74-
final SAXBuilder builder = new SAXBuilder();
74+
final SAXBuilder builder = createBuilder();
7575
final Document document = builder.build(in);
7676
return new JDomReader(document, getNameCoder());
7777
} catch (final IOException e) {
@@ -84,7 +84,7 @@ public HierarchicalStreamReader createReader(final InputStream in) {
8484
@Override
8585
public HierarchicalStreamReader createReader(final URL in) {
8686
try {
87-
final SAXBuilder builder = new SAXBuilder();
87+
final SAXBuilder builder = createBuilder();
8888
final Document document = builder.build(in);
8989
return new JDomReader(document, getNameCoder());
9090
} catch (final IOException e) {
@@ -97,7 +97,7 @@ public HierarchicalStreamReader createReader(final URL in) {
9797
@Override
9898
public HierarchicalStreamReader createReader(final File in) {
9999
try {
100-
final SAXBuilder builder = new SAXBuilder();
100+
final SAXBuilder builder = createBuilder();
101101
final Document document = builder.build(in);
102102
return new JDomReader(document, getNameCoder());
103103
} catch (final IOException e) {
@@ -117,4 +117,16 @@ public HierarchicalStreamWriter createWriter(final OutputStream out) {
117117
return new PrettyPrintWriter(new OutputStreamWriter(out));
118118
}
119119

120+
/**
121+
* Create and initialize the SAX builder.
122+
*
123+
* @return the SAX builder instance.
124+
* @since upcoming
125+
*/
126+
protected SAXBuilder createBuilder() {
127+
final SAXBuilder builder = new SAXBuilder();
128+
builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
129+
return builder;
130+
}
131+
120132
}

‎xstream/src/test/com/thoughtworks/xstream/io/xml/JDom2ReaderTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
import org.jdom2.Document;
1717
import org.jdom2.Element;
18-
import org.jdom2.input.JDOMParseException;
1918
import org.jdom2.input.SAXBuilder;
2019

2120
import java.io.StringReader;
@@ -49,10 +48,10 @@ public void testCanReadFromElementOfLargerDocument() throws Exception {
4948
public void testIsXXEVulnerable() throws Exception {
5049
try {
5150
super.testIsXXEVulnerable();
52-
fail("Thrown " + JDOMParseException.class.getName() + " expected");
53-
} catch (final JDOMParseException e) {
51+
fail("Thrown " + XStreamException.class.getName() + " expected");
52+
} catch (final XStreamException e) {
5453
final String message = e.getMessage();
55-
if (message.contains("Package")) {
54+
if (!message.contains("DOCTYPE")) {
5655
throw e;
5756
}
5857
}

‎xstream/src/test/com/thoughtworks/xstream/io/xml/JDomReaderTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import org.jdom.Document;
1818
import org.jdom.Element;
19-
import org.jdom.input.JDOMParseException;
2019
import org.jdom.input.SAXBuilder;
2120

2221
import java.io.StringReader;
@@ -50,10 +49,10 @@ public void testCanReadFromElementOfLargerDocument() throws Exception {
5049
public void testIsXXEVulnerable() throws Exception {
5150
try {
5251
super.testIsXXEVulnerable();
53-
fail("Thrown " + JDOMParseException.class.getName() + " expected");
54-
} catch (final JDOMParseException e) {
52+
fail("Thrown " + XStreamException.class.getName() + " expected");
53+
} catch (final XStreamException e) {
5554
final String message = e.getMessage();
56-
if (message.contains("Package")) {
55+
if (!message.contains("DOCTYPE")) {
5756
throw e;
5857
}
5958
}

0 commit comments

Comments
 (0)