Skip to content

Commit

Permalink
fix: resolve XML external entity in user-controlled data
Browse files Browse the repository at this point in the history
Solve the issues related to vulnerabilities raised by CodeQL scanning for the XML external entity in user-controlled data.

ING-4371
  • Loading branch information
emanuelaepure10 committed Jun 26, 2024
1 parent 93c6156 commit 88ff8cc
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
package de.fhg.igd.mapviewer.server.wms.capabilities;

import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.URI;
import java.net.URLConnection;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -254,14 +256,28 @@ else if (nodeName.equals("Abstract")) { //$NON-NLS-1$
* @throws ParserConfigurationException if an error occurred configuring the
* document parser
* @throws IOException if an error occurred reading the document
* @throws SAXException if an error occurred parsing the document
* @throws MalformedURLException if creating an URL from the given URI fails
*/
private static Document getDocument(URI uri)
throws ParserConfigurationException, MalformedURLException, SAXException, IOException {
throws ParserConfigurationException, MalformedURLException, IOException {
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(javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING, true);

DocumentBuilder builder = builderFactory.newDocumentBuilder();
Proxy proxy = ProxyUtil.findProxy(uri);
return builder.parse(uri.toURL().openConnection(proxy).getInputStream());
URLConnection connection = uri.toURL().openConnection(proxy);

// Ensure the input stream is closed properly
try (InputStream inputStream = connection.getInputStream()) {
return builder.parse(inputStream);
} catch (IOException | SAXException e) {
// Handle exceptions related to input stream and XML parsing
throw new IOException("Error parsing the document from the URI: " + uri, e);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
import java.awt.Dimension;
import java.awt.geom.Point2D;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URL;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathFactory;
Expand All @@ -24,6 +27,7 @@
import org.jdesktop.swingx.mapviewer.TileFactoryInfo;
import org.jdesktop.swingx.mapviewer.TileProvider;
import org.w3c.dom.Document;
import org.xml.sax.SAXException;

/**
* These are math utilities for converting between pixels, tiles, and geographic
Expand Down Expand Up @@ -179,27 +183,50 @@ public static GeoPosition getPositionForAddress(String[] fields) throws IOExcept
*/
public static GeoPosition getPositionForAddress(String street, String city, String state)
throws IOException {
URL load = new URL("http://api.local.yahoo.com/MapsService/V1/geocode?" + "appid=joshy688"
+ "&street=" + street.replace(' ', '+') + "&city=" + city.replace(' ', '+')
+ "&state=" + state.replace(' ', '+'));

HttpURLConnection connection = null;
try {
URL load = new URL("http://api.local.yahoo.com/MapsService/V1/geocode?"
+ "appid=joshy688" + "&street=" + street.replace(' ', '+') + "&city="
+ city.replace(' ', '+') + "&state=" + state.replace(' ', '+'));
// System.out.println("using address: " + load);
DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
Document doc = builder.parse(load.openConnection().getInputStream());
XPath xpath = XPathFactory.newInstance().newXPath();
// NodeList str =
// (NodeList)xpath.evaluate("//Result",doc,XPathConstants.NODESET);
Double lat = (Double) xpath.evaluate("//Result/Latitude/text()", doc,
XPathConstants.NUMBER);
Double lon = (Double) xpath.evaluate("//Result/Longitude/text()", doc,
XPathConstants.NUMBER);
// System.out.println("got address at: " + lat + " " + lon);
return new GeoPosition(lon, lat, GeoPosition.WGS_84_EPSG);
} catch (IOException e) {
throw e;
connection = (HttpURLConnection) load.openConnection();
connection.setRequestMethod("GET");

// Ensure secure XML processing
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(javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING, true);

DocumentBuilder builder = builderFactory.newDocumentBuilder();

// Parse the response using try-with-resources
try (InputStream inputStream = connection.getInputStream()) {
Document doc = builder.parse(inputStream);

// Use XPath to extract data
XPath xpath = XPathFactory.newInstance().newXPath();
Double lat = (Double) xpath.evaluate("//Result/Latitude/text()", doc,
XPathConstants.NUMBER);
Double lon = (Double) xpath.evaluate("//Result/Longitude/text()", doc,
XPathConstants.NUMBER);

return new GeoPosition(lon, lat, GeoPosition.WGS_84_EPSG);
}

} catch (ParserConfigurationException | SAXException e) {
throw new IOException("Error parsing XML response", e);
} catch (Exception e) {
throw new IOException(
"Failed to retrieve location information from the internet: " + e.toString());
throw new IOException("Failed to retrieve location information: " + e.getMessage(), e);
} finally {
if (connection != null) {
connection.disconnect();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,28 @@ public Document handleResponse(HttpResponse response)
}
DocumentBuilderFactory dbfac = DocumentBuilderFactory.newInstance();
try {
// Prevent XXE by disabling external entities and DTDs
dbfac.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbfac.setFeature("http://xml.org/sax/features/external-general-entities",
false);
dbfac.setFeature("http://xml.org/sax/features/external-parameter-entities",
false);
dbfac.setFeature(
"http://apache.org/xml/features/nonvalidating/load-external-dtd",
false);
dbfac.setFeature(javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING, true);

DocumentBuilder docBuilder = dbfac.newDocumentBuilder();
return docBuilder.parse(entity.getContent());

try (InputStream content = entity.getContent()) {
return docBuilder.parse(content);
}
} catch (ParserConfigurationException ex) {
throw new IllegalStateException(ex);
throw new IllegalStateException("Parser configuration error", ex);
} catch (SAXException ex) {
throw new ClientProtocolException("Malformed XML document", ex);
} catch (IOException ex) {
throw new ClientProtocolException("IO error while parsing XML document", ex);
}
}
});
Expand Down

0 comments on commit 88ff8cc

Please sign in to comment.