Fixed #203 - XML bomb vulnerability
The current configuration of the SAXParser is vulnerable for an XML bomb attack. The feature "http://apache.org/xml/features/disallow-doctype-decl" of the SAXBuilder must be activated in order to fix the vulnerability.
This commit is contained in:
Patrick Gotthard
parent
46c0061a29
commit
2bc58a0d1a
9 changed files with 157 additions and 40 deletions
31
README.md
31
README.md
|
|
@ -5,16 +5,23 @@ RSS 0.91 Userland, RSS 0.92, RSS 0.93, RSS 0.94, RSS 1.0, RSS 2.0, Atom 0.3, Ato
|
|||
|
||||
More Information: http://rometools.github.io/rome/
|
||||
|
||||
## Buildstatus
|
||||
## Changelog
|
||||
|
||||
| Project | Status |
|
||||
| --------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| rome | [](http://jenkins.patrick-gotthard.de/job/rome/) |
|
||||
| rome-certiorem | [](http://jenkins.patrick-gotthard.de/job/rome-certiorem/) |
|
||||
| rome-certiorem-webapp | [](http://jenkins.patrick-gotthard.de/job/rome-certiorem-webapp/) |
|
||||
| rome-fetcher | [](http://jenkins.patrick-gotthard.de/job/rome-fetcher/) |
|
||||
| rome-modules | [](http://jenkins.patrick-gotthard.de/job/rome-modules/) |
|
||||
| rome-opml | [](http://jenkins.patrick-gotthard.de/job/rome-opml/) |
|
||||
| rome-parent | [](http://jenkins.patrick-gotthard.de/job/rome-parent/) |
|
||||
| rome-propono | [](http://jenkins.patrick-gotthard.de/job/rome-propono/) |
|
||||
| rome-utils | [](http://jenkins.patrick-gotthard.de/job/rome-utils/) |
|
||||
### 1.5.1
|
||||
|
||||
- solved an [XML bomb](https://en.wikipedia.org/wiki/Billion_laughs) vulnerability
|
||||
|
||||
Important note: due to the security fix ROME now forbids all Doctype declarations by default. This will break compatibility with RSS 0.91 Netscape
|
||||
because it requires a Doctype declaration. When you experience problems you have to activate the property **allowDoctypes** on the SyndFeedInput object. You
|
||||
should only use this possibility when the feeds that you process are absolutely trustful.
|
||||
|
||||
### 1.5.0
|
||||
|
||||
- many (untracked) enhancements
|
||||
- code cleanup
|
||||
- renamed packages (was required to be able to push to Maven Central after years again)
|
||||
- updated sourcecode to Java 1.6
|
||||
|
||||
### Prior to 1.5.0
|
||||
|
||||
- see [http://rometools.github.io/rome/ROMEReleases](http://rometools.github.io/rome/ROMEReleases)
|
||||
|
|
@ -100,6 +100,27 @@ public class SyndFeedInput {
|
|||
public boolean getXmlHealerOn() {
|
||||
return feedInput.getXmlHealerOn();
|
||||
}
|
||||
|
||||
/**
|
||||
* Indicates whether Doctype declarations are allowed.
|
||||
*
|
||||
* @return true when Doctype declarations are allowed, false otherwise
|
||||
*/
|
||||
public boolean isAllowDoctypes() {
|
||||
return feedInput.isAllowDoctypes();
|
||||
}
|
||||
|
||||
/**
|
||||
* Since ROME 1.5.1 we fixed a security vulnerability by disallowing Doctype declarations by default.
|
||||
* This change breaks the compatibility with at least RSS 0.91N because it requires a Doctype declaration.
|
||||
* You are able to allow Doctype declarations again with this property. You should only activate it
|
||||
* when the feeds that you process are absolutely trustful.
|
||||
*
|
||||
* @param allowDoctypes true when Doctype declarations should be allowed again, false otherwise
|
||||
*/
|
||||
public void setAllowDoctypes(boolean allowDoctypes) {
|
||||
feedInput.setAllowDoctypes(allowDoctypes);
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds SyndFeedImpl from a file.
|
||||
|
|
|
|||
|
|
@ -67,6 +67,7 @@ public class WireFeedInput {
|
|||
private final Locale locale;
|
||||
|
||||
private boolean xmlHealerOn;
|
||||
private boolean allowDoctypes = false;
|
||||
|
||||
private static FeedParsers getFeedParsers() {
|
||||
synchronized (WireFeedInput.class) {
|
||||
|
|
@ -163,6 +164,27 @@ public class WireFeedInput {
|
|||
public boolean getXmlHealerOn() {
|
||||
return xmlHealerOn;
|
||||
}
|
||||
|
||||
/**
|
||||
* Indicates whether Doctype declarations are allowed.
|
||||
*
|
||||
* @return true when Doctype declarations are allowed, false otherwise
|
||||
*/
|
||||
public boolean isAllowDoctypes() {
|
||||
return allowDoctypes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Since ROME 1.5.1 we fixed a security vulnerability by disallowing Doctype declarations by default.
|
||||
* This change breaks the compatibility with at least RSS 0.91N because it requires a Doctype declaration.
|
||||
* You are able to allow Doctype declarations again with this property. You should only activate it
|
||||
* when the feeds that you process are absolutely trustful.
|
||||
*
|
||||
* @param allowDoctypes true when Doctype declarations should be allowed again, false otherwise
|
||||
*/
|
||||
public void setAllowDoctypes(boolean allowDoctypes) {
|
||||
this.allowDoctypes = allowDoctypes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds an WireFeed (RSS or Atom) from a file.
|
||||
|
|
@ -322,40 +344,36 @@ public class WireFeedInput {
|
|||
//
|
||||
// Crimson is one parser which is known not to support these features.
|
||||
try {
|
||||
|
||||
final XMLReader parser = saxBuilder.createParser();
|
||||
try {
|
||||
parser.setFeature("http://xml.org/sax/features/external-general-entities", false);
|
||||
saxBuilder.setFeature("http://xml.org/sax/features/external-general-entities", false);
|
||||
} catch (final SAXNotRecognizedException e) {
|
||||
// ignore
|
||||
} catch (final SAXNotSupportedException e) {
|
||||
// ignore
|
||||
}
|
||||
|
||||
setFeature(saxBuilder, parser, "http://xml.org/sax/features/external-general-entities", false);
|
||||
setFeature(saxBuilder, parser, "http://xml.org/sax/features/external-parameter-entities", false);
|
||||
setFeature(saxBuilder, parser, "http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
|
||||
|
||||
try {
|
||||
parser.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
|
||||
saxBuilder.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
|
||||
} catch (final SAXNotRecognizedException e) {
|
||||
// ignore
|
||||
} catch (final SAXNotSupportedException e) {
|
||||
// ignore
|
||||
}
|
||||
|
||||
try {
|
||||
parser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
|
||||
saxBuilder.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
|
||||
} catch (final SAXNotRecognizedException e) {
|
||||
// ignore
|
||||
} catch (final SAXNotSupportedException e) {
|
||||
// ignore
|
||||
if(!allowDoctypes) {
|
||||
setFeature(saxBuilder, parser, "http://apache.org/xml/features/disallow-doctype-decl", true);
|
||||
}
|
||||
|
||||
} catch (final JDOMException e) {
|
||||
throw new IllegalStateException("JDOM could not create a SAX parser");
|
||||
}
|
||||
|
||||
|
||||
saxBuilder.setExpandEntities(false);
|
||||
|
||||
return saxBuilder;
|
||||
|
||||
}
|
||||
|
||||
private void setFeature(SAXBuilder saxBuilder, XMLReader parser, String feature, boolean value) {
|
||||
try {
|
||||
saxBuilder.setFeature(feature, true);
|
||||
parser.setFeature(feature, true);
|
||||
} catch (final SAXNotRecognizedException e) {
|
||||
// ignore
|
||||
} catch (final SAXNotSupportedException e) {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,6 +21,10 @@ public abstract class FeedOpsTest extends FeedTest {
|
|||
protected FeedOpsTest(final String feedType) {
|
||||
super(feedType + ".xml");
|
||||
}
|
||||
|
||||
protected FeedOpsTest(final String feedType, boolean allowDoctypes) {
|
||||
super(feedType + ".xml", allowDoctypes);
|
||||
}
|
||||
|
||||
// 1.2a
|
||||
public void testWireFeedEquals() throws Exception {
|
||||
|
|
|
|||
|
|
@ -21,7 +21,9 @@ import com.rometools.rome.io.WireFeedInput;
|
|||
*
|
||||
*/
|
||||
public abstract class FeedTest extends TestCase {
|
||||
|
||||
private final String feedFileName;
|
||||
private final boolean allowDoctypes;
|
||||
private Document jDomDoc = null;
|
||||
private WireFeed wireFeed = null;
|
||||
private SyndFeed syndFeed = null;
|
||||
|
|
@ -29,7 +31,12 @@ public abstract class FeedTest extends TestCase {
|
|||
private boolean preservingWireFeed = false;
|
||||
|
||||
protected FeedTest(final String feedFileName) {
|
||||
this(feedFileName, false);
|
||||
}
|
||||
|
||||
protected FeedTest(final String feedFileName, boolean allowDoctypes) {
|
||||
this.feedFileName = feedFileName;
|
||||
this.allowDoctypes = allowDoctypes;
|
||||
}
|
||||
|
||||
protected String getFeedFileName() {
|
||||
|
|
@ -50,12 +57,14 @@ public abstract class FeedTest extends TestCase {
|
|||
|
||||
protected WireFeed getWireFeed() throws Exception {
|
||||
final WireFeedInput in = new WireFeedInput();
|
||||
in.setAllowDoctypes(allowDoctypes);
|
||||
return in.build(getFeedReader());
|
||||
}
|
||||
|
||||
protected SyndFeed getSyndFeed(final boolean preserveWireFeed) throws Exception {
|
||||
final SyndFeedInput in = new SyndFeedInput();
|
||||
in.setPreserveWireFeed(preserveWireFeed);
|
||||
in.setAllowDoctypes(allowDoctypes);
|
||||
return in.build(getFeedReader());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -19,7 +19,11 @@ public abstract class SyndFeedTest extends FeedTest {
|
|||
}
|
||||
|
||||
protected SyndFeedTest(final String feedType, final String feedFileName) {
|
||||
super(feedFileName);
|
||||
this(feedType, feedFileName, false);
|
||||
}
|
||||
|
||||
protected SyndFeedTest(final String feedType, final String feedFileName, boolean allowDoctypes) {
|
||||
super(feedFileName, allowDoctypes);
|
||||
prefix = feedType;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ public class TestOpsRSS091N extends FeedOpsTest {
|
|||
}
|
||||
|
||||
public TestOpsRSS091N() {
|
||||
super("rss_0.91N");
|
||||
super("rss_0.91N", true);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ import com.rometools.rome.io.impl.DateParser;
|
|||
public class TestSyndFeedRSS091N extends SyndFeedTest {
|
||||
|
||||
public TestSyndFeedRSS091N() {
|
||||
super("rss_0.91N", "rss_0.91N.xml");
|
||||
super("rss_0.91N", "rss_0.91N.xml", true);
|
||||
}
|
||||
|
||||
protected TestSyndFeedRSS091N(final String type) {
|
||||
|
|
|
|||
54
src/test/java/com/rometools/rome/unittest/TestXmlBomb.java
Normal file
54
src/test/java/com/rometools/rome/unittest/TestXmlBomb.java
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
package com.rometools.rome.unittest;
|
||||
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.InputStream;
|
||||
import java.io.InputStreamReader;
|
||||
import java.io.Reader;
|
||||
import java.nio.charset.Charset;
|
||||
|
||||
import com.rometools.rome.io.ParsingFeedException;
|
||||
import com.rometools.rome.io.WireFeedInput;
|
||||
import junit.framework.TestCase;
|
||||
|
||||
public class TestXmlBomb extends TestCase {
|
||||
|
||||
public void testXmlBomb() throws Exception {
|
||||
|
||||
// https://en.wikipedia.org/wiki/Billion_laughs
|
||||
// https://msdn.microsoft.com/en-us/magazine/ee335713.aspx
|
||||
String content = "<?xml version=\"1.0\"?>\n" +
|
||||
"<!DOCTYPE lolz [\n" +
|
||||
" <!ENTITY lol \"lol\">\n" +
|
||||
" <!ELEMENT lolz (#PCDATA)>\n" +
|
||||
" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\n" +
|
||||
" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\n" +
|
||||
" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\n" +
|
||||
" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\n" +
|
||||
" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\n" +
|
||||
" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\n" +
|
||||
" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\n" +
|
||||
" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\n" +
|
||||
" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\n" +
|
||||
"]>\n" +
|
||||
"<feed>\n" +
|
||||
" <title>&lol9;</title>\n" +
|
||||
" <subtitle>subtitle</subtitle>\n" +
|
||||
" <entry>\n" +
|
||||
" <id>id1</id>\n" +
|
||||
" <title>title1</title>\n" +
|
||||
" </entry>\n" +
|
||||
"</feed>";
|
||||
|
||||
InputStream is = new ByteArrayInputStream(content.getBytes("UTF-8"));
|
||||
WireFeedInput feedInput = new WireFeedInput();
|
||||
Reader reader = new InputStreamReader(is, Charset.forName("UTF-8"));
|
||||
try {
|
||||
feedInput.build(reader);
|
||||
fail("Expected exception");
|
||||
}
|
||||
catch (ParsingFeedException ex) {
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in a new issue