Skip to content

Commit a95a906

Browse files
authored
fix(QTDI-1938): Resolving XML external entity in user-controlled data (#1106)
* Potential fix for code scanning alert no. 30: Resolving XML external entity in user-controlled data
1 parent 50e8a52 commit a95a906

File tree

2 files changed

+73
-5
lines changed

2 files changed

+73
-5
lines changed

component-runtime-manager/src/main/java/org/talend/sdk/component/runtime/manager/service/http/codec/JAXBDecoder.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@
2121

2222
import javax.xml.bind.JAXBContext;
2323
import javax.xml.bind.JAXBException;
24-
import javax.xml.transform.stream.StreamSource;
24+
import javax.xml.parsers.SAXParserFactory;
25+
import javax.xml.transform.sax.SAXSource;
2526

2627
import org.talend.sdk.component.api.service.http.Decoder;
28+
import org.xml.sax.InputSource;
29+
import org.xml.sax.XMLReader;
2730

2831
import lombok.AllArgsConstructor;
2932

@@ -35,13 +38,20 @@ public class JAXBDecoder implements Decoder {
3538
@Override
3639
public Object decode(final byte[] value, final Type expectedType) {
3740
try {
41+
// Harden against XXE by configuring XMLReader
42+
final SAXParserFactory spf = SAXParserFactory.newInstance();
43+
spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
44+
spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
45+
spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
46+
spf.setNamespaceAware(true);
47+
final XMLReader xmlReader = spf.newSAXParser().getXMLReader();
3848
final Class key = Class.class.cast(expectedType);
3949
return jaxbContexts
4050
.get(key)
4151
.createUnmarshaller()
42-
.unmarshal(new StreamSource(new ByteArrayInputStream(value)), key)
52+
.unmarshal(new SAXSource(xmlReader, new InputSource(new ByteArrayInputStream(value))), key)
4353
.getValue();
44-
} catch (final JAXBException e) {
54+
} catch (final JAXBException | org.xml.sax.SAXException | javax.xml.parsers.ParserConfigurationException e) {
4555
throw new IllegalArgumentException(e);
4656
}
4757

component-runtime-manager/src/test/java/org/talend/sdk/component/runtime/manager/service/HttpClientFactoryImplTest.java

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.io.IOException;
3232
import java.io.InputStream;
3333
import java.io.InputStreamReader;
34+
import java.io.StringReader;
3435
import java.lang.reflect.Type;
3536
import java.net.HttpURLConnection;
3637
import java.net.InetSocketAddress;
@@ -222,8 +223,8 @@ void requestWithXML() throws IOException {
222223

223224
try {
224225
server.start();
225-
final ResponseXml client = newDefaultFactory().create(ResponseXml.class, null);
226-
client.base("http://localhost:" + server.getAddress().getPort() + "/api");
226+
final ResponseXml client = newDefaultFactory().create(ResponseXml.class,
227+
"http://localhost:" + server.getAddress().getPort() + "/api");
227228

228229
final Response<XmlRecord> result = client.main("application/xml", new XmlRecord("xml content"));
229230
assertEquals("xml content", result.body().getValue());
@@ -234,6 +235,63 @@ void requestWithXML() throws IOException {
234235
}
235236
}
236237

238+
@Test
239+
void requestWithXMLXXE() throws IOException {
240+
final HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
241+
server.createContext("/").setHandler(httpExchange -> {
242+
final Headers headers = httpExchange.getResponseHeaders();
243+
headers.set("content-type", "application/xml;charset=UTF-8");
244+
final byte[] bytes;
245+
String xmlContent = "<!DOCTYPE foo [ <!ENTITY xxe SYSTEM \"file:///etc/passwd\"> ]><foo>&xxe;</foo>";
246+
try (final BufferedReader in =
247+
new BufferedReader(new StringReader(xmlContent))) {
248+
bytes = in.lines().collect(joining("\n")).getBytes(StandardCharsets.UTF_8);
249+
}
250+
httpExchange.sendResponseHeaders(HttpURLConnection.HTTP_OK, bytes.length);
251+
httpExchange.getResponseBody().write(bytes);
252+
httpExchange.close();
253+
});
254+
255+
try {
256+
server.start();
257+
final ResponseXml client = newDefaultFactory().create(ResponseXml.class,
258+
"http://localhost:" + server.getAddress().getPort() + "/api");
259+
final Response<XmlRecord> result = client.main("application/xml", new XmlRecord("xml content"));
260+
assertThrows(java.lang.IllegalArgumentException.class, result::body,
261+
"lineNumber: 1; columnNumber: 10; DOCTYPE is disallowed when the feature \"http://apache.org/xml/features/disallow-doctype-decl\" set to true.");
262+
} finally {
263+
server.stop(0);
264+
}
265+
}
266+
267+
@Test
268+
void requestWithInvalidXML() throws IOException {
269+
final HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
270+
server.createContext("/").setHandler(httpExchange -> {
271+
final Headers headers = httpExchange.getResponseHeaders();
272+
headers.set("content-type", "application/xml;charset=UTF-8");
273+
final byte[] bytes;
274+
String xmlContent = "<!ENTITY xxe SYSTEM \"file:///etc/passwd\"><foo>invalid;";
275+
try (final BufferedReader in =
276+
new BufferedReader(new StringReader(xmlContent))) {
277+
bytes = in.lines().collect(joining("\n")).getBytes(StandardCharsets.UTF_8);
278+
}
279+
httpExchange.sendResponseHeaders(HttpURLConnection.HTTP_OK, bytes.length);
280+
httpExchange.getResponseBody().write(bytes);
281+
httpExchange.close();
282+
});
283+
284+
try {
285+
server.start();
286+
final ResponseXml client = newDefaultFactory().create(ResponseXml.class,
287+
"http://localhost:" + server.getAddress().getPort() + "/api");
288+
final Response<XmlRecord> result = client.main("application/xml", new XmlRecord("xml content"));
289+
assertThrows(java.lang.IllegalArgumentException.class, result::body);
290+
} finally {
291+
server.stop(0);
292+
}
293+
}
294+
237295
@Test
238296
void requestWithJSON() throws IOException {
239297
final HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);

0 commit comments

Comments
 (0)