Restrict widget resource locations to component:// or an explicit allowlist

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>

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 @@ private 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 static boolean isValidUrl(String s) {
     }
 
     /**
-     * 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

framework/base/src/test/java/org/apache/ofbiz/base/util/UtilValidateTests.java

@@ -43,4 +43,21 @@ public void testUrlValidations() throws Exception {
         assertTrue(UtilValidate.isUrlInString("https://foo/bar"));
         assertTrue(UtilValidate.isUrlInString("component://foo/bar?param=http://moo/far"));
     }
+
+    @Test
+    public void testIsAllowedPathDefaultDenyWhenUnconfigured() throws Exception {
+        UtilProperties.setPropertyValueInMemory("security", "allowFilePaths", "");
+        assertFalse(UtilValidate.isAllowedPath("/opt/ofbiz/templates/foo.ftl"));
+        assertFalse(UtilValidate.isAllowedPath("/dev/fd/292"));
+        assertFalse(UtilValidate.isAllowedPath(""));
+    }
+
+    @Test
+    public void testIsAllowedPathHonorsConfiguredPattern() throws Exception {
+        UtilProperties.setPropertyValueInMemory("security", "allowFilePaths", "/opt/ofbiz/templates/.*");
+        assertTrue(UtilValidate.isAllowedPath("/opt/ofbiz/templates/foo.ftl"));
+        assertFalse(UtilValidate.isAllowedPath("/etc/passwd"));
+        // restore default-deny for other tests sharing the in-memory properties cache
+        UtilProperties.setPropertyValueInMemory("security", "allowFilePaths", "");
+    }
 }

framework/security/config/security.properties

@@ -315,6 +315,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.

framework/webapp/src/main/java/org/apache/ofbiz/webapp/WebAppUtil.java

@@ -159,8 +159,18 @@ public static void setAttributesFromRequestBody(ServletRequest request) {
             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));
             }
         }

framework/webapp/src/test/java/org/apache/ofbiz/webapp/WebAppUtilTests.java

@@ -0,0 +1,97 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *******************************************************************************/
+package org.apache.ofbiz.webapp;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.when;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import jakarta.servlet.ReadListener;
+import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletInputStream;
+import jakarta.servlet.http.HttpServletRequest;
+
+import org.junit.jupiter.api.Test;
+
+/** Covers the JSON-request-body attribute merge that fed the reported
+ * anonymous mainDecoratorLocation override (login-page widget-injection RCE):
+ * a request body must never be able to shadow a name the webapp already
+ * exposes as a trusted, application-owned ServletContext attribute. */
+public class WebAppUtilTests {
+
+    private static ServletInputStream inputStreamOf(String content) {
+        ByteArrayInputStream bytes = new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
+        return new ServletInputStream() {
+            @Override
+            public boolean isFinished() {
+                return bytes.available() == 0;
+            }
+
+            @Override
+            public boolean isReady() {
+                return true;
+            }
+
+            @Override
+            public void setReadListener(ReadListener readListener) {
+            }
+
+            @Override
+            public int read() {
+                return bytes.read();
+            }
+        };
+    }
+
+    @Test
+    public void doesNotOverrideAnExistingServletContextAttribute() throws IOException {
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        ServletContext servletContext = mock(ServletContext.class);
+        when(request.getServletContext()).thenReturn(servletContext);
+        when(request.getContentType()).thenReturn("application/json");
+        when(request.getInputStream()).thenReturn(
+                inputStreamOf("{\"mainDecoratorLocation\":\"file:/dev/fd/292\"}"));
+        when(servletContext.getAttribute("mainDecoratorLocation"))
+                .thenReturn("component://order/widget/ordermgr/CommonScreens.xml");
+
+        WebAppUtil.setAttributesFromRequestBody(request);
+
+        verify(request, never()).setAttribute("mainDecoratorLocation", "file:/dev/fd/292");
+    }
+
+    @Test
+    public void stillSetsAttributesThatDoNotShadowContextConfig() throws IOException {
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        ServletContext servletContext = mock(ServletContext.class);
+        when(request.getServletContext()).thenReturn(servletContext);
+        when(request.getContentType()).thenReturn("application/json");
+        when(request.getInputStream()).thenReturn(
+                inputStreamOf("{\"searchString\":\"widgets\"}"));
+        when(servletContext.getAttribute("searchString")).thenReturn(null);
+
+        WebAppUtil.setAttributesFromRequestBody(request);
+
+        verify(request).setAttribute("searchString", "widgets");
+    }
+}

framework/widget/src/main/java/org/apache/ofbiz/widget/model/FormFactory.java

@@ -28,6 +28,7 @@
 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 static ModelForm getFormFromLocation(String resourceName, String formName
         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 static ModelForm getFormFromWebappContext(String resourceName, String for
         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) {

framework/widget/src/main/java/org/apache/ofbiz/widget/model/GridFactory.java

@@ -29,6 +29,7 @@
 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 static ModelGrid getGridFromLocation(String resourceName, String gridName
         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 static ModelGrid getGridFromWebappContext(String resourceName, String gri
             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) {

framework/widget/src/main/java/org/apache/ofbiz/widget/model/MenuFactory.java

@@ -28,6 +28,7 @@
 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 static ModelMenu getMenuFromWebappContext(String resourceName, String men
         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 static ModelMenu getMenuFromLocation(String resourceName, String menuName
         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);
             }

framework/widget/src/main/java/org/apache/ofbiz/widget/model/ScreenFactory.java

@@ -200,7 +200,7 @@ public static void renderReferencedScreen(String name, String location, ModelScr
         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 {