Skip to content

Commit

Permalink
PDFBOX-4505: unify the usage of a XMLParser (DRY) to avoid a XML Exte…
Browse files Browse the repository at this point in the history 
…rnal Entity (XXE) attack as reported by Kurt Boberg of DocuSign CVE-2019-0228

git-svn-id: https://svn.apache.org/repos/asf/pdfbox/trunk@1856954 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@lehmi
lehmi committed Apr 4, 2019
1 parent 75af3c7 commit be36ef0
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 46 deletions.
29 changes: 3 additions & 26 deletions pdfbox/src/main/java/org/apache/pdfbox/pdmodel/fdf/FDFAnnotationStamp.java
View file
Expand Up @@ -20,9 +20,6 @@
import java.io.IOException;
import java.io.OutputStream;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
Expand All @@ -38,12 +35,11 @@
import org.apache.pdfbox.cos.COSName;
import org.apache.pdfbox.cos.COSStream;
import org.apache.pdfbox.util.Hex;

import org.apache.pdfbox.util.XMLUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

/**
* This represents a Stamp FDF annotation.
Expand Down Expand Up @@ -122,7 +118,8 @@ public FDFAnnotationStamp(Element element) throws IOException
{
LOG.debug("Decoded XML: " + new String(decodedAppearanceXML));

Document stampAppearance = getStampAppearanceDocument(decodedAppearanceXML);
Document stampAppearance = XMLUtil
.parse(new ByteArrayInputStream(decodedAppearanceXML));

Element appearanceEl = stampAppearance.getDocumentElement();

Expand All @@ -137,26 +134,6 @@ public FDFAnnotationStamp(Element element) throws IOException
}
}

/**
* Parse the <param>xmlString</param> to DOM Document tree from XML content
*/
private Document getStampAppearanceDocument(byte[] xml) throws IOException
{
try
{
// Obtain DOM Document instance and create DocumentBuilder with default configuration
DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();

// Parse the content to Document object
return builder.parse(new ByteArrayInputStream(xml));
}
catch (ParserConfigurationException | SAXException ex)
{
LOG.error("Error while converting appearance xml to document: " + ex);
throw new IOException(ex);
}
}

/**
* This will create an Appearance dictionary from an appearance XML document.
*
Expand Down
4 changes: 2 additions & 2 deletions pdfbox/src/main/java/org/apache/pdfbox/pdmodel/fdf/FDFDocument.java
View file
Expand Up @@ -33,6 +33,7 @@
import org.apache.pdfbox.cos.COSName;
import org.apache.pdfbox.pdfparser.FDFParser;
import org.apache.pdfbox.pdfwriter.COSWriter;
import org.apache.pdfbox.util.XMLUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;

Expand Down Expand Up @@ -239,8 +240,7 @@ public static FDFDocument loadXFDF(File file) throws IOException
*/
public static FDFDocument loadXFDF(InputStream input) throws IOException
{
Document doc = XMLUtil.parse(input);
return new FDFDocument(doc);
return new FDFDocument(XMLUtil.parse(input));
}

/**
Expand Down
1 change: 1 addition & 0 deletions pdfbox/src/main/java/org/apache/pdfbox/pdmodel/fdf/FDFField.java
View file
Expand Up @@ -34,6 +34,7 @@
import org.apache.pdfbox.pdmodel.interactive.action.PDActionFactory;
import org.apache.pdfbox.pdmodel.interactive.action.PDAdditionalActions;
import org.apache.pdfbox.pdmodel.interactive.annotation.PDAppearanceDictionary;
import org.apache.pdfbox.util.XMLUtil;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
Expand Down
16 changes: 3 additions & 13 deletions pdfbox/src/main/java/org/apache/pdfbox/pdmodel/interactive/form/PDXFAResource.java
View file
Expand Up @@ -21,8 +21,6 @@
import java.io.IOException;
import java.io.InputStream;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;

import org.apache.pdfbox.cos.COSArray;
Expand Down Expand Up @@ -136,17 +134,9 @@ private static byte[] getBytesFromStream(final COSStream stream) throws IOExcept
* @throws IOException if something went wrong when reading the XFA content.
*
*/
public Document getDocument() throws ParserConfigurationException, SAXException, IOException
public Document getDocument() throws IOException
{
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
factory.setNamespaceAware(true);
DocumentBuilder builder = factory.newDocumentBuilder();
return builder.parse(new ByteArrayInputStream(this.getBytes()));
return org.apache.pdfbox.util.XMLUtil //
.parse(new ByteArrayInputStream(this.getBytes()), true);
}
}
28 changes: 23 additions & 5 deletions ...rg/apache/pdfbox/pdmodel/fdf/XMLUtil.java → .../java/org/apache/pdfbox/util/XMLUtil.java
View file
Expand Up @@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.pdfbox.pdmodel.fdf;
package org.apache.pdfbox.util;

import java.io.InputStream;
import java.io.IOException;
Expand All @@ -36,7 +36,7 @@
*
* @author Ben Litchfield
*/
final class XMLUtil
public final class XMLUtil
{
/**
* Utility class, should not be instantiated.
Expand All @@ -54,16 +54,33 @@ private XMLUtil()
* @throws IOException It there is an error creating the dom.
*/
public static Document parse(InputStream is) throws IOException
{
return parse(is, false);
}

/**
* This will parse an XML stream and create a DOM document.
*
* @param is The stream to get the XML from.
* @param nsAware activates namespace awareness of the parser
* @return The DOM document.
* @throws IOException It there is an error creating the dom.
*/
public static Document parse(InputStream is, boolean nsAware) throws IOException
{
try
{
DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance();
builderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
builderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
builderFactory.setFeature("http://xml.org/sax/features/external-general-entities",
false);
builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities",
false);
builderFactory.setFeature(
"http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
builderFactory.setXIncludeAware(false);
builderFactory.setExpandEntityReferences(false);
builderFactory.setNamespaceAware(nsAware);
DocumentBuilder builder = builderFactory.newDocumentBuilder();
return builder.parse(is);
}
Expand Down Expand Up @@ -94,4 +111,5 @@ public static String getNodeValue(Element node)
}
return sb.toString();
}

}

0 comments on commit be36ef0

Please sign in to comment.