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 <[email protected]>
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));
}
}
