Skip to content
Permalink
Browse files Browse the repository at this point in the history
1) Take steps to guard against external XXE attacks (except, note tha…
…t <!DOCTYPE> cannot be disabled in XML plists).

2) Resolve the actual PLIST DTD from inside the JAR file itself, and prevent resolution of other external XML resources.
3) Make XMLPlistParser.getDocBuilder public
  • Loading branch information
David Solin authored and David Solin committed Oct 5, 2016
1 parent b9be0f4 commit 8c954e8
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 48 deletions.
3 changes: 2 additions & 1 deletion build.xml
Expand Up @@ -34,7 +34,8 @@

<target name="jar" depends="compile">
<jar destfile="${dist}/${jar}">
<fileset dir="${build}/classes"/>
<fileset dir="${build}/classes"/>
<fileset dir="${source}" includes="**/*.dtd"/>
</jar>
</target>

Expand Down
19 changes: 19 additions & 0 deletions src/main/java/com/dd/plist/PropertyList-1.0.dtd
@@ -0,0 +1,19 @@
<!ENTITY % plistObject "(array | data | date | dict | real | integer | string | true | false )" >
<!ELEMENT plist %plistObject;>
<!ATTLIST plist version CDATA "1.0" >

<!-- Collections -->
<!ELEMENT array (%plistObject;)*>
<!ELEMENT dict (key, %plistObject;)*>
<!ELEMENT key (#PCDATA)>

<!--- Primitive types -->
<!ELEMENT string (#PCDATA)>
<!ELEMENT data (#PCDATA)> <!-- Contents interpreted as Base-64 encoded -->
<!ELEMENT date (#PCDATA)> <!-- Contents should conform to a subset of ISO 8601 (in particular, YYYY '-' MM '-' DD 'T' HH ':' MM ':' SS 'Z'. Smaller units may be omitted with a loss of precision) -->

<!-- Numerical primitives -->
<!ELEMENT true EMPTY> <!-- Boolean constant true -->
<!ELEMENT false EMPTY> <!-- Boolean constant false -->
<!ELEMENT real (#PCDATA)> <!-- Contents should represent a floating point number matching ("+" | "-")? d+ ("."d*)? ("E" ("+" | "-") d+)? where d is a digit 0-9. -->
<!ELEMENT integer (#PCDATA)> <!-- Contents should represent a (possibly signed) integer number in base 10 -->
101 changes: 54 additions & 47 deletions src/main/java/com/dd/plist/XMLPropertyListParser.java
Expand Up @@ -32,6 +32,7 @@
import javax.xml.parsers.ParserConfigurationException;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.text.ParseException;
Expand All @@ -44,24 +45,25 @@
* @author Daniel Dreibrodt
*/
public class XMLPropertyListParser {

private static DocumentBuilderFactory docBuilderFactory = null;

/**
* Instantiation is prohibited by outside classes.
*/
protected XMLPropertyListParser() {
/** empty **/
}

/**
* Initialize the document builder factory so that it can be reused and does not need to
* be reinitialized for each parse action.
*/
private static synchronized void initDocBuilderFactory() {
docBuilderFactory = DocumentBuilderFactory.newInstance();
docBuilderFactory.setIgnoringComments(true);
docBuilderFactory.setCoalescing(true);
private static final DocumentBuilderFactory FACTORY = DocumentBuilderFactory.newInstance();
static {
try {
FACTORY.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
Float v1_7 = new Float("1.7");
Float javaVersion = new Float(System.getProperty("java.specification.version"));
if (javaVersion.compareTo(v1_7) >= 0) {
FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false);
FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
}
FACTORY.setXIncludeAware(false);
FACTORY.setExpandEntityReferences(false);
FACTORY.setNamespaceAware(false);
FACTORY.setIgnoringComments(true);
FACTORY.setCoalescing(true);
FACTORY.setValidating(false);
} catch (ParserConfigurationException e) {
throw new RuntimeException(e);
}
}

/**
Expand All @@ -72,22 +74,10 @@ private static synchronized void initDocBuilderFactory() {
* @throws javax.xml.parsers.ParserConfigurationException If a document builder for parsing a XML property list
* could not be created. This should not occur.
*/
private static synchronized DocumentBuilder getDocBuilder() throws ParserConfigurationException {
if (docBuilderFactory == null)
initDocBuilderFactory();
DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
docBuilder.setEntityResolver(new EntityResolver() {
public InputSource resolveEntity(String publicId, String systemId) {
if ("-//Apple Computer//DTD PLIST 1.0//EN".equals(publicId) || // older publicId
"-//Apple//DTD PLIST 1.0//EN".equals(publicId)) { // newer publicId
// return a dummy, zero length DTD so we don't have to fetch
// it from the network.
return new InputSource(new ByteArrayInputStream(new byte[0]));
}
return null;
}
});
return docBuilder;
public static synchronized DocumentBuilder getDocBuilder() throws ParserConfigurationException {
DocumentBuilder builder = FACTORY.newDocumentBuilder();
builder.setEntityResolver(new PlistDTDResolver());
return builder;
}

/**
Expand All @@ -103,12 +93,10 @@ public InputSource resolveEntity(String publicId, String systemId) {
* @throws com.dd.plist.PropertyListFormatException If the given property list has an invalid format.
* @throws java.text.ParseException If a date string could not be parsed.
*/
public static NSObject parse(File f) throws ParserConfigurationException, IOException, SAXException, PropertyListFormatException, ParseException {
DocumentBuilder docBuilder = getDocBuilder();

Document doc = docBuilder.parse(f);
public static NSObject parse(File f)
throws ParserConfigurationException, IOException, SAXException, PropertyListFormatException, ParseException {

return parse(doc);
return parse(getDocBuilder().parse(new FileInputStream(f)));
}

/**
Expand All @@ -123,9 +111,10 @@ public static NSObject parse(File f) throws ParserConfigurationException, IOExce
* @throws com.dd.plist.PropertyListFormatException If the given property list has an invalid format.
* @throws java.text.ParseException If a date string could not be parsed.
*/
public static NSObject parse(final byte[] bytes) throws ParserConfigurationException, ParseException, SAXException, PropertyListFormatException, IOException {
ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
return parse(bis);
public static NSObject parse(final byte[] bytes)
throws ParserConfigurationException, ParseException, SAXException, PropertyListFormatException, IOException {

return parse(new ByteArrayInputStream(bytes));
}

/**
Expand All @@ -141,12 +130,10 @@ public static NSObject parse(final byte[] bytes) throws ParserConfigurationExcep
* @throws com.dd.plist.PropertyListFormatException If the given property list has an invalid format.
* @throws java.text.ParseException If a date string could not be parsed.
*/
public static NSObject parse(InputStream is) throws ParserConfigurationException, IOException, SAXException, PropertyListFormatException, ParseException {
DocumentBuilder docBuilder = getDocBuilder();

Document doc = docBuilder.parse(is);
public static NSObject parse(InputStream is)
throws ParserConfigurationException, IOException, SAXException, PropertyListFormatException, ParseException {

return parse(doc);
return parse(getDocBuilder().parse(is));
}

/**
Expand Down Expand Up @@ -290,4 +277,24 @@ private static String getNodeTextContents(Node n) {
}
}
}

/**
* Resolves only the Apple PLIST DTD.
*/
static class PlistDTDResolver implements EntityResolver {
private static final String PLIST_SYSTEMID_1 = "-//Apple Computer//DTD PLIST 1.0//EN";
private static final String PLIST_SYSTEMID_2 = "-//Apple//DTD PLIST 1.0//EN";

PlistDTDResolver() {
}

// Implement EntityResolver

public InputSource resolveEntity(String publicId, String systemId) {
if (PLIST_SYSTEMID_1.equals(publicId) || PLIST_SYSTEMID_2.equals(publicId)) {
return new InputSource(XMLPropertyListParser.class.getResourceAsStream("PropertyList-1.0.dtd"));
}
return null;
}
}
}

0 comments on commit 8c954e8

Please sign in to comment.