You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by as...@apache.org on 2026/08/14 16:04:12 UTC
(ofbiz-framework) branch release24.09 updated: Restrict widget resource locations to component:// or an explicit allowlist (#1650) (#1654)
This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya pushed a commit to branch release24.09
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release24.09 by this push:
new a76610be8e Restrict widget resource locations to component:// or an explicit allowlist (#1650) (#1654)
a76610be8e is described below
commit a76610be8eea9e80fde944a9762b1f5b9df9d1be
Author: Ashish Vijaywargiya <as...@apache.org>
AuthorDate: Fri Aug 14 21:34:06 2026 +0530
Restrict widget resource locations to component:// or an explicit allowlist (#1650) (#1654)
---
.../commonext/widget/ofbizsetup/ProfileScreens.xml | 4 +-
.../org/apache/ofbiz/base/util/UtilValidate.java | 26 +++++----
framework/security/config/security.properties | 9 ++++
.../java/org/apache/ofbiz/webapp/WebAppUtil.java | 10 ++++
.../org/apache/ofbiz/widget/model/FormFactory.java | 19 +++++--
.../org/apache/ofbiz/widget/model/GridFactory.java | 19 +++++--
.../org/apache/ofbiz/widget/model/MenuFactory.java | 17 +++++-
.../apache/ofbiz/widget/model/ScreenFactory.java | 2 +-
.../org/apache/ofbiz/widget/model/TreeFactory.java | 9 +++-
.../ofbiz/widget/model/WidgetSecureLocation.java | 61 ++++++++++++++++++----
10 files changed, 141 insertions(+), 35 deletions(-)
diff --git a/applications/commonext/widget/ofbizsetup/ProfileScreens.xml b/applications/commonext/widget/ofbizsetup/ProfileScreens.xml
index a2a5ae7232..e0269bd4db 100644
--- a/applications/commonext/widget/ofbizsetup/ProfileScreens.xml
+++ b/applications/commonext/widget/ofbizsetup/ProfileScreens.xml
@@ -90,8 +90,8 @@
<set field="helpAnchor" value="_help_for_view_organization_profile"/>
</actions>
<widgets>
- <include-screen name="Party" location="applications/party/widget/partymgr/ProfileScreens.xml"/>
- <include-screen name="Contact" location="applications/party/widget/partymgr/ProfileScreens.xml"/>
+ <include-screen name="Party" location="component://party/widget/partymgr/ProfileScreens.xml"/>
+ <include-screen name="Contact" location="component://party/widget/partymgr/ProfileScreens.xml"/>
</widgets>
</section>
</screen>
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
index 3feda0787e..dba02b2bb0 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
@@ -18,11 +18,11 @@
*******************************************************************************/
package org.apache.ofbiz.base.util;
-import java.nio.file.Path;
import java.nio.file.Paths;
import java.sql.Timestamp;
import java.util.Collection;
import java.util.Map;
+import java.util.regex.Pattern;
import org.apache.commons.validator.routines.EmailValidator;
import org.apache.commons.validator.routines.UrlValidator;
@@ -155,9 +155,6 @@ public final class UtilValidate {
public static final String CONTIGUOUS_US_STATE_CODES = "AL|AZ|AR|CA|CO|CT|DE|DC|FL|GA|ID|IL|IN|IA|KS|KY|LA|ME|MD|MA|MI|MN|MS|MO|MT|NE|NV|NH|"
+ "NJ|NM|NY|NC|ND|OH|OK|OR|PA|RI|SC|SD|TN|TX|UT|VT|VA|WA|WV|WI|WY";
- /** Paths from which loading files should be prevented */
- public static final String[] BLOCKED_PATHS = {"proc/self/fd"};
-
/** Check whether an object is empty, will see if it is a String, Map, Collection, etc. */
public static boolean isEmpty(Object o) {
return ObjectType.isEmpty(o);
@@ -662,22 +659,23 @@ public final class UtilValidate {
}
/**
- * isBlockedPath takes a String representing a filePath, normalizes it and checks it against a Blacklist
+ * isAllowedPath takes a String representing a non-component widget resource path, normalizes it and
+ * checks it against the administrator-configured <code>security.allowFilePaths</code> regular
+ * expression. Unset or blank configuration denies every path (secure by default): an administrator
+ * must explicitly opt in to loading widget resources from outside a <code>component://</code> location.
* @param rawPathString
- * @return true if its a blocked path, false otherwise or if it is empty
+ * @return true if it's an allowed path, false otherwise (including when unconfigured)
*/
- public static boolean isBlockedPath(String rawPathString) {
+ public static boolean isAllowedPath(String rawPathString) {
if (UtilValidate.isEmpty(rawPathString)) {
return false;
}
- Path normalized = Paths.get(rawPathString).normalize();
- String normalizedPath = normalized.toString();
- for (String blocked : BLOCKED_PATHS) {
- if (normalizedPath.contains(blocked)) {
- return true;
- }
+ String allowFilePaths = UtilProperties.getPropertyValue("security", "allowFilePaths", "");
+ if (UtilValidate.isEmpty(allowFilePaths)) {
+ return false;
}
- return false;
+ String normalizedPath = Paths.get(rawPathString).normalize().toString();
+ return Pattern.compile(allowFilePaths).matcher(normalizedPath).matches();
}
/** isYear returns true if string s is a valid
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index f70ec1d597..7ec6b9f93a 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -300,6 +300,15 @@ deniedFileExtensions=html,htm,php,php1,php2,hph3,php4,php5,php6,php7,phps,asp,as
#-- As it name says, allowAllUploads opens all possibilities
allowAllUploads=
+#--
+#-- Widget resources (screens, forms, grids, menus, trees) are only loaded from a component://
+#-- location by default. allowFilePaths is a regular expression an administrator can set to also
+#-- allow loading widget resources from bare filesystem paths outside any component, matched via
+#-- UtilValidate::isAllowedPath. Left blank (the default), every non-component location is denied.
+#-- A file: URI (in any letter case, e.g. file:/some/path) is never allowed here, regardless of
+#-- this setting: see WidgetSecureLocation.
+allowFilePaths=
+
#--
#-- Default characters that are allowed in file names and file extensions to guarantee safeness
#-- Uncomment to change. Note that allowing all characters is at risk.
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
index 80cbfe50f1..07f87a7b9d 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java
@@ -159,8 +159,18 @@ public final class WebAppUtil {
Debug.logWarning(ioe, MODULE);
}
if (requestBodyMap != null) {
+ ServletContext servletContext = request.getServletContext();
Set<String> parameterNames = requestBodyMap.keySet();
for (String parameterName: parameterNames) {
+ // A request body is anonymous, attacker-controlled input. Never let it shadow a name
+ // the webapp already exposes as a trusted, application-owned ServletContext attribute
+ // (e.g. mainDecoratorLocation, set from web.xml at filter init) - doing so let an
+ // unauthenticated JSON request redirect trusted widget/screen locations.
+ if (servletContext.getAttribute(parameterName) != null) {
+ Debug.logWarning("Ignoring request body attribute [%s]: it shadows an existing"
+ + " ServletContext attribute of the same name", MODULE, parameterName);
+ continue;
+ }
request.setAttribute(parameterName, requestBodyMap.get(parameterName));
}
}
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java
index 80753d4d07..8987407dcc 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java
@@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.UtilXml;
@@ -71,7 +72,13 @@ public class FormFactory {
String cacheKey = sb.toString();
ModelForm modelForm = FORM_LOCATION_CACHE.get(cacheKey);
if (modelForm == null) {
- URL formFileUrl = FlexibleLocation.resolveLocation(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of form [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, formName, resourceName);
+ throw new IllegalArgumentException("Abort form rendering due to unallowed form location");
+ }
+ URL formFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
if (formFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(formFileUrl.toString())) {
throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
}
@@ -104,11 +111,17 @@ public class FormFactory {
if (modelForm == null) {
Delegator delegator = (Delegator) request.getAttribute("delegator");
LocalDispatcher dispatcher = (LocalDispatcher) request.getAttribute("dispatcher");
- URL formFileUrl = request.getServletContext().getResource(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of form [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, formName, resourceName);
+ throw new IllegalArgumentException("Abort form rendering due to unallowed form location");
+ }
+ URL formFileUrl = request.getServletContext().getResource(sanitizedLocation);
Document formFileDoc = UtilXml.readXmlDocument(formFileUrl, true, true);
Element formElement = UtilXml.firstChildElement(formFileDoc.getDocumentElement(), "form", "name", formName);
modelForm = createModelForm(formElement, delegator.getModelReader(), visualTheme, dispatcher.getDispatchContext(),
- resourceName, formName);
+ sanitizedLocation, formName);
modelForm = FORM_WEBAPP_CACHE.putIfAbsentAndGet(cacheKey, modelForm);
}
if (modelForm == null) {
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java
index a615114ad6..febc4b18f1 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java
@@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.UtilXml;
@@ -73,7 +74,13 @@ public class GridFactory {
String cacheKey = sb.toString();
ModelGrid modelGrid = GRID_LOCATION_CACHE.get(cacheKey);
if (modelGrid == null) {
- URL gridFileUrl = FlexibleLocation.resolveLocation(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of grid [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, gridName, resourceName);
+ throw new IllegalArgumentException("Abort grid rendering due to unallowed grid location");
+ }
+ URL gridFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
if (gridFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(gridFileUrl.toString())) {
throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
}
@@ -108,11 +115,17 @@ public class GridFactory {
ServletContext servletContext = request.getServletContext();
Delegator delegator = (Delegator) request.getAttribute("delegator");
LocalDispatcher dispatcher = (LocalDispatcher) request.getAttribute("dispatcher");
- URL gridFileUrl = servletContext.getResource(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of grid [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, gridName, resourceName);
+ throw new IllegalArgumentException("Abort grid rendering due to unallowed grid location");
+ }
+ URL gridFileUrl = servletContext.getResource(sanitizedLocation);
Document gridFileDoc = UtilXml.readXmlDocument(gridFileUrl, true, true);
Element gridElement = UtilXml.firstChildElement(gridFileDoc.getDocumentElement(), "grid", "name", gridName);
modelGrid = createModelGrid(gridElement, delegator.getModelReader(), visualTheme,
- dispatcher.getDispatchContext(), resourceName, gridName);
+ dispatcher.getDispatchContext(), sanitizedLocation, gridName);
modelGrid = GRID_WEBAPP_CACHE.putIfAbsentAndGet(cacheKey, modelGrid);
}
if (modelGrid == null) {
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java
index 91455398cf..0e176a0fc5 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java
@@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.UtilXml;
@@ -65,7 +66,13 @@ public class MenuFactory {
if (modelMenuMap == null) {
ServletContext servletContext = request.getServletContext();
- URL menuFileUrl = servletContext.getResource(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of menu [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, menuName, resourceName);
+ throw new IllegalArgumentException("Abort menu rendering due to unallowed menu location");
+ }
+ URL menuFileUrl = servletContext.getResource(sanitizedLocation);
Document menuFileDoc = UtilXml.readXmlDocument(menuFileUrl, true, true);
modelMenuMap = readMenuDocument(menuFileDoc, location, visualTheme);
MENU_WEBAPP_CACHE.putIfAbsent(cacheKey, modelMenuMap);
@@ -106,7 +113,13 @@ public class MenuFactory {
String keyName = resourceName + "::" + visualTheme.getVisualThemeId();
Map<String, ModelMenu> modelMenuMap = MENU_LOCATION_CACHE.get(keyName);
if (modelMenuMap == null) {
- URL menuFileUrl = FlexibleLocation.resolveLocation(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of menu [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, menuName, resourceName);
+ throw new IllegalArgumentException("Abort menu rendering due to unallowed menu location");
+ }
+ URL menuFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
if (menuFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(menuFileUrl.toString())) {
throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
}
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
index 8e0537721c..157a7328b1 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java
@@ -199,7 +199,7 @@ public class ScreenFactory {
if (UtilValidate.isNotEmpty(location)) {
String sanitizedLocation = WidgetSecureLocation.sanitize(location);
if (sanitizedLocation == null) {
- Debug.logWarning("The location of screen [%s] isn't an allowed Path. Abort rendering. Raw location [%s]", MODULE, name, location);
+ Debug.logWarning("The location of screen [%s] isn't an allowed path. Abort rendering. Raw location [%s]", MODULE, name, location);
throw new IllegalArgumentException("Abort screen rendering due to unallowed screen location");
}
try {
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java
index c32e381741..c58d531dfa 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/TreeFactory.java
@@ -26,6 +26,7 @@ import java.util.Map;
import javax.xml.parsers.ParserConfigurationException;
import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.UtilXml;
import org.apache.ofbiz.base.util.cache.UtilCache;
@@ -50,7 +51,13 @@ public class TreeFactory {
throws IOException, SAXException, ParserConfigurationException {
Map<String, ModelTree> modelTreeMap = TREE_LOCATION_CACHE.get(resourceName);
if (modelTreeMap == null) {
- URL treeFileUrl = FlexibleLocation.resolveLocation(resourceName);
+ String sanitizedLocation = WidgetSecureLocation.sanitize(resourceName);
+ if (sanitizedLocation == null) {
+ Debug.logWarning("The location of tree [%s] isn't an allowed path. Abort rendering. Raw location [%s]",
+ MODULE, treeName, resourceName);
+ throw new IllegalArgumentException("Abort tree rendering due to unallowed tree location");
+ }
+ URL treeFileUrl = FlexibleLocation.resolveLocation(sanitizedLocation);
if (treeFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(treeFileUrl.toString())) {
throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);
}
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
index 8c87b7b640..16a6e2f85e 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/WidgetSecureLocation.java
@@ -19,29 +19,72 @@
package org.apache.ofbiz.widget.model;
import java.nio.file.Paths;
+
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilValidate;
+/**
+ * Central gatekeeper for every widget XML resource location (screen, form, grid, menu, tree) before
+ * it is resolved and parsed. A widget location can be influenced by request context (for example a
+ * decorator's {@code location} attribute is commonly a Flexible String Expression such as
+ * {@code ${parameters.mainDecoratorLocation}}), so this class treats every location as untrusted and
+ * only allows it through when it is either:
+ * <ul>
+ * <li>a {@code component://} location with no {@code ..} traversal segment, or</li>
+ * <li>a bare filesystem path matched by the administrator-configured
+ * {@code security.allowFilePaths} allowlist (denied by default).</li>
+ * </ul>
+ * Any {@code file:} scheme location - single-slash or otherwise, in any letter case - is rejected
+ * outright at the protocol layer: it is never a legitimate widget location, and on Linux it is the
+ * carrier used to reach live process-descriptor aliases such as {@code file:/dev/fd/N} or
+ * {@code file:/proc/thread-self/fd/N}.
+ */
public final class WidgetSecureLocation {
private static final String MODULE = WidgetSecureLocation.class.getName();
- private static final String COMPO_TYPE = "component://";
+ private static final String COMPONENT_PROTOCOL = "component://";
+
+ private WidgetSecureLocation() { }
+ /**
+ * Sanitizes a widget resource location.
+ * @param location the raw, potentially untrusted location
+ * @return the (possibly normalized) location if it is allowed, or {@code null} if it must be rejected
+ */
public static String sanitize(String location) {
- if (UtilValidate.isEmpty(location)
- || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(location)
- || location.startsWith("file:/")) {
+ if (UtilValidate.isEmpty(location)) {
+ Debug.logWarning("Unable to sanitize an empty widget location", MODULE);
+ return null;
+ }
+ if (isFileScheme(location) || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(location)) {
Debug.logWarning(String.format("Unable to sanitize location: [%s]", location), MODULE);
return null;
}
- if (location.startsWith(COMPO_TYPE) && location.length() > 12) {
- if (location.indexOf("..") > 0) {
- Debug.logWarning(String.format("For security reason traversal sequence '..' is not allowed: [%s]", location), MODULE);
+ if (location.startsWith(COMPONENT_PROTOCOL)) {
+ String componentRelativePath = location.substring(COMPONENT_PROTOCOL.length());
+ if (componentRelativePath.contains("..")) {
+ Debug.logWarning(String.format("Traversal sequence '..' is not allowed in location: [%s]", location), MODULE);
return null;
}
- return COMPO_TYPE + Paths.get(location.substring(12)).normalize();
+ return COMPONENT_PROTOCOL + Paths.get(componentRelativePath).normalize();
}
+ if (UtilValidate.isAllowedPath(location)) {
+ return location;
+ }
+ Debug.logWarning(String.format("Location isn't on the configured allowed file path: [%s]", location), MODULE);
+ return null;
+ }
- return location.startsWith(COMPO_TYPE) ? location : null;
+ /**
+ * Detects the {@code file:} URI scheme regardless of case or slash count, e.g. {@code file:/},
+ * {@code File:/}, {@code FILE://}. Java's URL/{@code FlexibleLocation} scheme resolution is
+ * case-insensitive, so this check must be too.
+ */
+ private static boolean isFileScheme(String location) {
+ int colonIndex = location.indexOf(':');
+ if (colonIndex < 0) {
+ return false;
+ }
+ return "file".equalsIgnoreCase(location.substring(0, colonIndex));
}
}