Skip to content

Commit 1df5596

Browse files
committed
CAMEL-8312: XML External Entity (XXE) injection in XPath. Thanks to Stephan Siano for the patch.
1 parent 504cf03 commit 1df5596

File tree

4 files changed

+40
-36
lines changed

4 files changed

+40
-36
lines changed

Diff for: camel-core/src/main/java/org/apache/camel/builder/xml/XPathBuilder.java

-21
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.io.File;
2020
import java.io.InputStream;
21-
import java.io.StringReader;
2221
import java.util.HashSet;
2322
import java.util.LinkedHashMap;
2423
import java.util.List;
@@ -51,7 +50,6 @@
5150
import org.apache.camel.Predicate;
5251
import org.apache.camel.RuntimeExpressionException;
5352
import org.apache.camel.WrappedFile;
54-
import org.apache.camel.component.bean.BeanInvocation;
5553
import org.apache.camel.impl.DefaultExchange;
5654
import org.apache.camel.spi.Language;
5755
import org.apache.camel.spi.NamespaceAware;
@@ -1113,25 +1111,6 @@ protected Object doGetDocument(Exchange exchange, Object body) throws Exception
11131111
}
11141112
}
11151113

1116-
// okay we can try to remedy the failed conversion by some special types
1117-
if (answer == null) {
1118-
// let's try coercing some common types into something JAXP work with the best for special types
1119-
if (body instanceof WrappedFile) {
1120-
// special for files so we can work with them out of the box
1121-
InputStream is = exchange.getContext().getTypeConverter().convertTo(InputStream.class, exchange, body);
1122-
answer = new InputSource(is);
1123-
} else if (body instanceof BeanInvocation) {
1124-
// if its a null bean invocation then handle that specially
1125-
BeanInvocation bi = exchange.getContext().getTypeConverter().convertTo(BeanInvocation.class, exchange, body);
1126-
if (bi.getArgs() != null && bi.getArgs().length == 1 && bi.getArgs()[0] == null) {
1127-
// its a null argument from the bean invocation so use null as answer
1128-
answer = null;
1129-
}
1130-
} else if (body instanceof String) {
1131-
answer = new InputSource(new StringReader((String) body));
1132-
}
1133-
}
1134-
11351114
if (type == null && answer == null) {
11361115
// fallback to get the body as is
11371116
answer = body;

Diff for: camel-core/src/test/java/org/apache/camel/builder/xml/XPathFeatureTest.java

+32-10
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@
1919

2020
import java.io.FileNotFoundException;
2121

22-
import javax.xml.xpath.XPathExpressionException;
23-
2422
import org.apache.camel.ContextTestSupport;
2523
import org.apache.camel.Exchange;
24+
import org.apache.camel.NoTypeConversionAvailableException;
25+
import org.apache.camel.RuntimeCamelException;
26+
import org.apache.camel.TypeConversionException;
2627
import org.apache.camel.converter.jaxp.XmlConverter;
28+
import org.xml.sax.SAXParseException;
2729

2830
import static org.apache.camel.builder.xml.XPathBuilder.xpath;
2931

@@ -32,18 +34,19 @@ public class XPathFeatureTest extends ContextTestSupport {
3234

3335
public static final String XML_DATA = " <!DOCTYPE foo [ "
3436
+ " <!ELEMENT foo ANY > <!ENTITY xxe SYSTEM \"file:///bin/test.sh\" >]> <test> &xxe; </test>";
35-
36-
37+
public static final String XML_DATA_INVALID = " <!DOCTYPE foo [ "
38+
+ " <!ELEMENT foo ANY > <!ENTITY xxe SYSTEM \"file:///bin/test.sh\" >]> <test> &xxe; </test><notwellformed>";
39+
3740
@Override
3841
public boolean isUseRouteBuilder() {
3942
return false;
4043
}
41-
44+
4245
public void testXPathResult() throws Exception {
4346
String result = (String)xpath("/").stringResult().evaluate(createExchange(XML_DATA));
4447
assertEquals("Get a wrong result", " ", result);
4548
}
46-
49+
4750
public void testXPath() throws Exception {
4851

4952
// Set this feature will enable the external general entities
@@ -52,16 +55,35 @@ public void testXPath() throws Exception {
5255
try {
5356
xpath("/").stringResult().evaluate(createExchange(XML_DATA));
5457
fail("Expect an Exception here");
55-
} catch (Exception ex) {
56-
assertTrue("Get a wrong exception cause.", ex instanceof InvalidXPathExpression);
57-
assertTrue("Get a wrong exception cause.", ex.getCause() instanceof XPathExpressionException);
58+
} catch (TypeConversionException ex) {
59+
assertTrue("Get a wrong exception cause.", ex.getCause() instanceof RuntimeCamelException);
5860
assertTrue("Get a wrong exception cause.", ex.getCause().getCause() instanceof FileNotFoundException);
5961
} finally {
6062
System.clearProperty(DOM_BUILER_FACTORY_FEATRUE + ":"
6163
+ "http://xml.org/sax/features/external-general-entities");
6264
}
6365
}
64-
66+
67+
public void testXPathNoTypeConverter() throws Exception {
68+
try {
69+
// define a class without type converter as document type
70+
xpath("/").documentType(Exchange.class).stringResult().evaluate(createExchange(XML_DATA));
71+
fail("Expect an Exception here");
72+
} catch (RuntimeCamelException ex) {
73+
assertTrue("Get a wrong exception cause.", ex.getCause() instanceof NoTypeConversionAvailableException);
74+
}
75+
}
76+
77+
public void testXPathResultOnInvalidData() throws Exception {
78+
try {
79+
xpath("/").stringResult().evaluate(createExchange(XML_DATA_INVALID));
80+
fail("Expect an Exception here");
81+
} catch (TypeConversionException ex) {
82+
assertTrue("Get a wrong exception cause.", ex.getCause() instanceof RuntimeCamelException);
83+
assertTrue("Get a wrong exception cause.", ex.getCause().getCause() instanceof SAXParseException);
84+
}
85+
}
86+
6587
protected Exchange createExchange(Object xml) {
6688
Exchange exchange = createExchangeWithBody(context, xml);
6789
return exchange;

Diff for: components/camel-saxon/src/test/java/org/apache/camel/component/xslt/SaxonXsltDTDTest.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,22 @@ private void sendEntityMessage(Object message) throws Exception {
6161
Exchange exchange = list.get(0);
6262
String xml = exchange.getIn().getBody(String.class);
6363
assertTrue("Get a wrong transformed message", xml.indexOf("<transformed subject=\"\">") > 0);
64-
65-
64+
65+
endpoint.reset();
66+
endpoint.expectedMessageCount(1);
6667

6768
try {
6869
template.sendBody("direct:start2", message);
69-
fail("Expect an exception here");
70+
list = endpoint.getReceivedExchanges();
71+
exchange = list.get(0);
72+
xml = exchange.getIn().getBody(String.class);
73+
assertTrue("Get a wrong transformed message", xml.indexOf("<transformed subject=\"\">") > 0);
7074
} catch (Exception ex) {
7175
// expect an exception here
7276
assertTrue("Get a wrong exception", ex instanceof CamelExecutionException);
7377
// the file could not be found
7478
assertTrue("Get a wrong exception cause", ex.getCause() instanceof TransformerException);
7579
}
76-
7780
}
7881

7982

Diff for: components/camel-saxon/src/test/resources/org/apache/camel/language/xpath/XPathLanguageTest.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
<route>
4343
<from uri="direct:testSaxonWithFactory"/>
4444
<setBody>
45-
<xpath factoryRef="saxonFactory" documentType="org.xml.sax.InputSource" resultType="java.lang.String" logNamespaces="true">tokenize(a, '\|')</xpath>
45+
<xpath factoryRef="saxonFactory" resultType="java.lang.String" logNamespaces="true">tokenize(a, '\|')</xpath>
4646
</setBody>
4747
<log message="Test Saxon with factory: ${body}"/>
4848
<to uri="mock:testSaxonWithFactoryResult"/>

0 commit comments

Comments
 (0)