This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 2026c88e91 Fixed: [codeQL] Resolving specific Java issues
(OFBIZ-12925)
2026c88e91 is described below
commit 2026c88e91ca0f6a7d4adb18a78e9593a897f099
Author: Jacques Le Roux <[email protected]>
AuthorDate: Mon Feb 24 08:31:53 2025 +0100
Fixed: [codeQL] Resolving specific Java issues (OFBIZ-12925)
This concerns a possible server-side request forgery reported by CodeQL
<<To fix the SSRF vulnerability, we need to ensure that the URL being used
in
the readXmlDocument method is validated and restricted to a set of allowed
URLs
or domains. This can be achieved by maintaining a whitelist of allowed URLs
or
domains and checking the user-provided URL against this list before
proceeding
with the request.>>
Fortunately we already have and can use the host-headers-allowed property in
security.properties. Here is the fix.
While a it improves RequestHandler by setting HOSTHEADERSALLOWED as static
---
.../base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java | 10 +++++++---
.../java/org/apache/ofbiz/webapp/control/RequestHandler.java | 8 +++-----
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java
index 0ac2e2ada8..a76fbe22b1 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java
@@ -87,6 +87,7 @@ public final class UtilXml {
private static final String MODULE = UtilXml.class.getName();
private static final XStream X_STREAM = createXStream();
private UtilXml() { }
+ private static final List<String> HOSTHEADERSALLOWED =
UtilMisc.getHostHeadersAllowed();
private static XStream createXStream() {
XStream xstream = new XStream();
@@ -395,9 +396,12 @@ public final class UtilXml {
public static Document readXmlDocument(URL url, boolean validate, boolean
withPosition)
throws SAXException, ParserConfigurationException,
java.io.IOException {
- if (url == null) {
- Debug.logWarning("[UtilXml.readXmlDocument] URL was null, doing
nothing", MODULE);
- return null;
+
+ if (!HOSTHEADERSALLOWED.contains(url.getHost())) {
+ Debug.logWarning("Domain " + url.getHost() + " not accepted to
prevent host header injection."
+ + " You need to set host-headers-allowed property in
security.properties file.", MODULE);
+ throw new IOException("Domain " + url.getHost() + " not accepted
to prevent host header injection."
+ + " You need to set host-headers-allowed property in
security.properties file.");
}
InputStream is = url.openStream();
Document document = readXmlDocument(is, validate, url.toString(),
withPosition);
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index 9ec279ad67..bc9f8faa3d 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -63,8 +63,8 @@ import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.apache.ofbiz.security.CsrfUtil;
-import org.apache.ofbiz.webapp.OfbizUrlBuilder;
import org.apache.ofbiz.webapp.OfbizPathShortener;
+import org.apache.ofbiz.webapp.OfbizUrlBuilder;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap;
import org.apache.ofbiz.webapp.event.EventFactory;
@@ -89,7 +89,7 @@ public final class RequestHandler {
private final URL controllerConfigURL;
private final boolean trackServerHit;
private final boolean trackVisit;
- private final List<String> hostHeadersAllowed;
+ private static final List<String> HOSTHEADERSALLOWED =
UtilMisc.getHostHeadersAllowed();
private ControllerConfig ccfg;
private RequestHandler(ServletContext context) {
@@ -106,8 +106,6 @@ public final class RequestHandler {
this.trackServerHit =
!"false".equalsIgnoreCase(context.getInitParameter("track-serverhit"));
this.trackVisit =
!"false".equalsIgnoreCase(context.getInitParameter("track-visit"));
- hostHeadersAllowed = UtilMisc.getHostHeadersAllowed();
-
}
public static RequestHandler getRequestHandler(ServletContext
servletContext) {
@@ -360,7 +358,7 @@ public final class RequestHandler {
public void doRequest(HttpServletRequest request, HttpServletResponse
response, String chain,
GenericValue userLogin, Delegator delegator) throws
RequestHandlerException, RequestHandlerExceptionAllowExternalRequests {
- if (!hostHeadersAllowed.contains(request.getServerName())) {
+ if (!HOSTHEADERSALLOWED.contains(request.getServerName())) {
Debug.logError("Domain " + request.getServerName() + " not
accepted to prevent host header injection."
+ " You need to set host-headers-allowed property in
security.properties file.", MODULE);
throw new RequestHandlerException("Domain " +
request.getServerName() + " not accepted to prevent host header injection."
