Skip to content

Commit

Permalink
Fixed security bug in the XML extension that allows XML external enti…
Browse files Browse the repository at this point in the history
…ty attacks. Reported by Fabio Mancinelli.
  • Loading branch information
Thierry Boileau committed May 23, 2012
1 parent fc6bfe4 commit ef1836e
Show file tree
Hide file tree
Showing 19 changed files with 431 additions and 89 deletions.
3 changes: 3 additions & 0 deletions build/tmpl/text/changes.txt
Expand Up @@ -16,6 +16,9 @@ Changes log
JSON extension. JSON extension.
- Optimized conversions using the Jackson extension in order to reuse - Optimized conversions using the Jackson extension in order to reuse
configuration (issue #577). configuration (issue #577).
- Fixed security bug in the XML extension that allows XML external
entity attacks.
Reported by Fabio Mancinelli.


- 2.0.13 (2012-04-18) - 2.0.13 (2012-04-18)
- Bug fixed - Bug fixed
Expand Down
Expand Up @@ -93,7 +93,7 @@ public void write(org.restlet.ext.xml.XmlWriter writer)
} }
}; };
}; };

result.setNamespaceAware(true);
return result; return result;
} }


Expand Down
Expand Up @@ -113,6 +113,7 @@ public Categories(Context context, String categoriesUri) throws IOException {
*/ */
public Categories(Representation categoriesFeed) throws IOException { public Categories(Representation categoriesFeed) throws IOException {
super(categoriesFeed); super(categoriesFeed);
setNamespaceAware(true);
parse(new CategoriesContentReader(this)); parse(new CategoriesContentReader(this));
} }


Expand Down
Expand Up @@ -103,6 +103,7 @@ public class Entry extends SaxRepresentation {
*/ */
public Entry() { public Entry() {
super(MediaType.APPLICATION_ATOM); super(MediaType.APPLICATION_ATOM);
setNamespaceAware(true);
this.authors = null; this.authors = null;
this.categories = null; this.categories = null;
this.content = null; this.content = null;
Expand Down Expand Up @@ -155,6 +156,7 @@ public Entry(Context context, String entryUri) throws IOException {
*/ */
public Entry(Representation xmlEntry) throws IOException { public Entry(Representation xmlEntry) throws IOException {
super(xmlEntry); super(xmlEntry);
setNamespaceAware(true);
parse(new EntryContentReader(this)); parse(new EntryContentReader(this));
} }


Expand All @@ -170,6 +172,7 @@ public Entry(Representation xmlEntry) throws IOException {
public Entry(Representation xmlEntry, EntryReader entryReader) public Entry(Representation xmlEntry, EntryReader entryReader)
throws IOException { throws IOException {
super(xmlEntry); super(xmlEntry);
setNamespaceAware(true);
parse(new EntryContentReader(this, entryReader)); parse(new EntryContentReader(this, entryReader));
} }


Expand Down
Expand Up @@ -118,6 +118,7 @@ public class Feed extends SaxRepresentation {
*/ */
public Feed() { public Feed() {
super(MediaType.APPLICATION_ATOM); super(MediaType.APPLICATION_ATOM);
setNamespaceAware(true);
this.authors = null; this.authors = null;
this.categories = null; this.categories = null;
this.contributors = null; this.contributors = null;
Expand Down Expand Up @@ -171,6 +172,7 @@ public Feed(Context context, String feedUri) throws IOException {
*/ */
public Feed(Representation xmlFeed) throws IOException { public Feed(Representation xmlFeed) throws IOException {
super(xmlFeed); super(xmlFeed);
setNamespaceAware(true);
parse(new FeedContentReader(this)); parse(new FeedContentReader(this));
} }


Expand All @@ -186,6 +188,7 @@ public Feed(Representation xmlFeed) throws IOException {
public Feed(Representation xmlFeed, FeedReader feedReader) public Feed(Representation xmlFeed, FeedReader feedReader)
throws IOException { throws IOException {
super(xmlFeed); super(xmlFeed);
setNamespaceAware(true);
parse(new FeedContentReader(this, feedReader)); parse(new FeedContentReader(this, feedReader));
} }


Expand Down
Expand Up @@ -90,6 +90,7 @@ public class Service extends SaxRepresentation {
*/ */
public Service(Client clientDispatcher) { public Service(Client clientDispatcher) {
super(new MediaType("***")); super(new MediaType("***"));
setNamespaceAware(true);
this.clientDispatcher = clientDispatcher; this.clientDispatcher = clientDispatcher;
} }


Expand Down Expand Up @@ -122,6 +123,7 @@ public Service(Client clientDispatcher, String serviceUri)
public Service(Client clientDispatcher, String serviceUri, public Service(Client clientDispatcher, String serviceUri,
Representation xmlService) throws IOException { Representation xmlService) throws IOException {
super(xmlService); super(xmlService);
setNamespaceAware(true);
this.clientDispatcher = clientDispatcher; this.clientDispatcher = clientDispatcher;
this.reference = (serviceUri == null) ? null this.reference = (serviceUri == null) ? null
: new Reference(serviceUri); : new Reference(serviceUri);
Expand Down
Expand Up @@ -43,6 +43,7 @@
import javax.xml.bind.JAXBException; import javax.xml.bind.JAXBException;
import javax.xml.bind.ValidationEventHandler; import javax.xml.bind.ValidationEventHandler;
import javax.xml.bind.util.JAXBSource; import javax.xml.bind.util.JAXBSource;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.sax.SAXSource; import javax.xml.transform.sax.SAXSource;


import org.restlet.Context; import org.restlet.Context;
Expand Down Expand Up @@ -91,16 +92,22 @@ public static synchronized JAXBContext getContext(String contextPath,
return result; return result;
} }


/**
* The classloader to use for JAXB annotated classes.
*/
private volatile ClassLoader classLoader;

/** /**
* The list of Java package names that contain schema derived class and/or * The list of Java package names that contain schema derived class and/or
* Java to schema (JAXB-annotated) mapped classes. * Java to schema (JAXB-annotated) mapped classes.
*/ */
private volatile String contextPath; private volatile String contextPath;


/** /**
* The classloader to use for JAXB annotated classes. * Specifies that the parser will expand entity reference nodes. By default
* the value of this is set to true.
*/ */
private volatile ClassLoader classLoader; private volatile boolean expandingEntityRefs;


/** /**
* Indicates if the resulting XML data should be formatted with line breaks * Indicates if the resulting XML data should be formatted with line breaks
Expand All @@ -123,9 +130,30 @@ public static synchronized JAXBContext getContext(String contextPath,
/** The "xsi:schemaLocation" attribute in the generated XML data */ /** The "xsi:schemaLocation" attribute in the generated XML data */
private volatile String schemaLocation; private volatile String schemaLocation;


/** Limits potential XML overflow attacks. */
private boolean secureProcessing;

/**
* Indicates the desire for validating this type of XML representations
* against a DTD. Note that for XML schema or Relax NG validation, use the
* "schema" property instead.
*
* @see DocumentBuilderFactory#setValidating(boolean)
*/
private volatile boolean validatingDtd;

/** The JAXB validation event handler. */ /** The JAXB validation event handler. */
private volatile ValidationEventHandler validationEventHandler; private volatile ValidationEventHandler validationEventHandler;


/**
* Indicates the desire for processing <em>XInclude</em> if found in this
* type of XML representations. By default the value of this is set to
* false.
*
* @see DocumentBuilderFactory#setXIncludeAware(boolean)
*/
private volatile boolean xIncludeAware;

/** The source XML representation. */ /** The source XML representation. */
private volatile Representation xmlRepresentation; private volatile Representation xmlRepresentation;


Expand Down Expand Up @@ -161,6 +189,14 @@ private JaxbRepresentation(MediaType mediaType, T object,
this.object = object; this.object = object;
this.validationEventHandler = null; this.validationEventHandler = null;
this.xmlRepresentation = null; this.xmlRepresentation = null;
this.expandingEntityRefs = false;
this.formattedOutput = false;
this.fragment = false;
this.noNamespaceSchemaLocation = null;
this.schemaLocation = null;
this.secureProcessing = true;
this.validatingDtd = false;
this.xIncludeAware = false;
} }


/** /**
Expand Down Expand Up @@ -270,7 +306,6 @@ private JaxbRepresentation(Representation xmlRepresentation,
this.object = null; this.object = null;
this.validationEventHandler = validationHandler; this.validationEventHandler = validationHandler;
this.xmlRepresentation = xmlRepresentation; this.xmlRepresentation = xmlRepresentation;

} }


/** /**
Expand Down Expand Up @@ -374,8 +409,8 @@ public T getObject() throws IOException {
} }


try { try {
this.object = (T) u.unmarshal(this.xmlRepresentation this.object = (T) u.unmarshal(this,
.getReader()); this.xmlRepresentation.getReader());
} catch (JAXBException e) { } catch (JAXBException e) {
Context.getCurrentLogger().log(Level.WARNING, Context.getCurrentLogger().log(Level.WARNING,
"Unable to unmarshal the XML representation", e); "Unable to unmarshal the XML representation", e);
Expand Down Expand Up @@ -416,6 +451,16 @@ public ValidationEventHandler getValidationEventHandler() {
return this.validationEventHandler; return this.validationEventHandler;
} }


/**
* Indicates if the parser will expand entity reference nodes. By default
* the value of this is set to true.
*
* @return True if the parser will expand entity reference nodes.
*/
public boolean isExpandingEntityRefs() {
return expandingEntityRefs;
}

/** /**
* Indicates if the resulting XML data should be formatted with line breaks * Indicates if the resulting XML data should be formatted with line breaks
* and indentation. Defaults to false. * and indentation. Defaults to false.
Expand All @@ -437,6 +482,36 @@ public boolean isFragment() {
return fragment; return fragment;
} }


/**
* Indicates if it limits potential XML overflow attacks.
*
* @return True if it limits potential XML overflow attacks.
*/
public boolean isSecureProcessing() {
return secureProcessing;
}

/**
* Indicates the desire for validating this type of XML representations
* against an XML schema if one is referenced within the contents.
*
* @return True if the schema-based validation is enabled.
*/
public boolean isValidatingDtd() {
return validatingDtd;
}

/**
* Indicates the desire for processing <em>XInclude</em> if found in this
* type of XML representations. By default the value of this is set to
* false.
*
* @return The current value of the xIncludeAware flag.
*/
public boolean isXIncludeAware() {
return xIncludeAware;
}

/** /**
* Sets the list of Java package names that contain schema derived class * Sets the list of Java package names that contain schema derived class
* and/or Java to schema (JAXB-annotated) mapped classes. * and/or Java to schema (JAXB-annotated) mapped classes.
Expand All @@ -448,6 +523,17 @@ public void setContextPath(String contextPath) {
this.contextPath = contextPath; this.contextPath = contextPath;
} }


/**
* Indicates if the parser will expand entity reference nodes. By default
* the value of this is set to true.
*
* @param expandEntityRefs
* True if the parser will expand entity reference nodes.
*/
public void setExpandingEntityRefs(boolean expandEntityRefs) {
this.expandingEntityRefs = expandEntityRefs;
}

/** /**
* Indicates if the resulting XML data should be formatted with line breaks * Indicates if the resulting XML data should be formatted with line breaks
* and indentation. * and indentation.
Expand Down Expand Up @@ -504,6 +590,27 @@ public void setSchemaLocation(String schemaLocation) {
this.schemaLocation = schemaLocation; this.schemaLocation = schemaLocation;
} }


/**
* Indicates if it limits potential XML overflow attacks.
*
* @param secureProcessing
* True if it limits potential XML overflow attacks.
*/
public void setSecureProcessing(boolean secureProcessing) {
this.secureProcessing = secureProcessing;
}

/**
* Indicates the desire for validating this type of XML representations
* against an XML schema if one is referenced within the contents.
*
* @param validating
* The new validation flag to set.
*/
public void setValidatingDtd(boolean validating) {
this.validatingDtd = validating;
}

/** /**
* Sets the validation event handler. * Sets the validation event handler.
* *
Expand All @@ -515,6 +622,18 @@ public void setValidationEventHandler(
this.validationEventHandler = validationEventHandler; this.validationEventHandler = validationEventHandler;
} }


/**
* Indicates the desire for processing <em>XInclude</em> if found in this
* type of XML representations. By default the value of this is set to
* false.
*
* @param includeAware
* The new value of the xIncludeAware flag.
*/
public void setXIncludeAware(boolean includeAware) {
xIncludeAware = includeAware;
}

/** /**
* Writes the representation to a stream of characters. * Writes the representation to a stream of characters.
* *
Expand Down
Expand Up @@ -34,16 +34,14 @@
package org.restlet.ext.jaxb.internal; package org.restlet.ext.jaxb.internal;


import java.io.OutputStream; import java.io.OutputStream;
import java.io.StringWriter; import java.io.OutputStreamWriter;
import java.io.Writer; import java.io.Writer;
import java.util.logging.Level; import java.util.logging.Level;


import javax.xml.bind.JAXBException; import javax.xml.bind.JAXBException;
import javax.xml.bind.ValidationEventHandler;


import org.restlet.Context; import org.restlet.Context;
import org.restlet.ext.jaxb.JaxbRepresentation; import org.restlet.ext.jaxb.JaxbRepresentation;
import org.restlet.representation.StringRepresentation;


/** /**
* This is a utility class to assist in marshaling Java content trees into XML. * This is a utility class to assist in marshaling Java content trees into XML.
Expand Down Expand Up @@ -184,25 +182,7 @@ private javax.xml.bind.Marshaller getMarshaller() throws JAXBException {
*/ */
public void marshal(Object jaxbElement, OutputStream stream) public void marshal(Object jaxbElement, OutputStream stream)
throws JAXBException { throws JAXBException {
getMarshaller().marshal(jaxbElement, stream); marshal(jaxbElement, new OutputStreamWriter(stream));
}

/**
* Marshal the content tree rooted at {@code jaxbElement} into a Restlet
* String representation.
*
* @param jaxbElement
* The root of the content tree to be marshaled.
* @param rep
* The target string representation write the XML to.
* @throws JAXBException
* If any unexpected problem occurs during marshaling.
*/
public void marshal(Object jaxbElement, StringRepresentation rep)
throws JAXBException {
final StringWriter writer = new StringWriter();
marshal(jaxbElement, writer);
rep.setText(writer.toString());
} }


/** /**
Expand All @@ -216,20 +196,9 @@ public void marshal(Object jaxbElement, StringRepresentation rep)
* If any unexpected problem occurs during marshaling. * If any unexpected problem occurs during marshaling.
*/ */
public void marshal(Object jaxbElement, Writer writer) throws JAXBException { public void marshal(Object jaxbElement, Writer writer) throws JAXBException {
getMarshaller().setEventHandler(
getJaxbRepresentation().getValidationEventHandler());
getMarshaller().marshal(jaxbElement, writer); getMarshaller().marshal(jaxbElement, writer);
} }


/**
* Sets the validation handler for this marshaler.
*
* @param handler
* A validation handler.
* @throws JAXBException
* If an error was encountered while setting the event handler.
*/
public void setEventHandler(ValidationEventHandler handler)
throws JAXBException {
getMarshaller().setEventHandler(handler);
}

} }

0 comments on commit ef1836e

Please sign in to comment.