Improved: Sanitize all widget xml ressource location

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,12 +18,12 @@
  *******************************************************************************/
 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;
 import org.apache.ofbiz.base.lang.IsEmpty;
@@ -156,7 +156,7 @@ private UtilValidate() { }
             + "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"};
+    private static Pattern allowedPathsPattern = null;
 
     /** Check whether an object is empty, will see if it is a String, Map, Collection, etc. */
     public static boolean isEmpty(Object o) {
@@ -661,23 +661,23 @@ public static boolean isValidUrl(String s) {
         return UrlValidator.getInstance().isValid(s);
     }
 
+    private static Pattern initAllowedPathPattern() {
+        return Pattern.compile(UtilProperties.getPropertyValue("security", "allowFilePaths", ""));
+    }
+
     /**
-     * isBlockedPath takes a String representing a filePath, normalizes it and checks it against a Blacklist
+     * isAllowedPath takes a String representing a filePath, normalizes it and checks it if allowed
      * @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
      */
-    public static boolean isBlockedPath(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;
-            }
-        }
-        return false;
+    public static boolean isAllowedPath(String rawPathString) {
+        if (allowedPathsPattern == null) {
+            allowedPathsPattern = initAllowedPathPattern();
+        }
+        return UtilValidate.isNotEmpty(rawPathString)
+                && allowedPathsPattern.matcher(Paths.get(rawPathString)
+                        .normalize().toString())
+                .matches();
     }
 
     /** isYear returns true if string s is a valid

framework/security/config/security.properties

@@ -315,6 +315,9 @@ deniedFileExtensions=html,htm,php,php1,php2,hph3,php4,php5,php6,php7,phps,asp,as
 #-- As it name says, allowAllUploads opens all possibilities
 allowAllUploads=
 
+#-- RegExp for ofbiz to allow some file access denied by default like function to UtilValidate::isAllowedPath
+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/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 form 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 form 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 grid 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 grid 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 menu 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 menu 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

@@ -198,12 +198,14 @@ public static void renderReferencedScreen(String name, String location, ModelScr
 
         ModelScreen modelScreen = null;
         if (UtilValidate.isNotEmpty(location)) {
-            if (UtilValidate.isBlockedPath(location)) {
-                Debug.logWarning("The location of screen [%s] is on a blocked Path. Abbort rendering. Raw location [%s]", MODULE, name, location);
-                throw new IllegalArgumentException("Abort screenrendering due to screenlocation pointing to a blocked path");
+            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);
+                throw new IllegalArgumentException("Abort screen rendering due to screen unallowed screen location");
             }
             try {
-                modelScreen = ScreenFactory.getScreenFromLocation(location, name);
+                modelScreen = ScreenFactory.getScreenFromLocation(sanitizedLocation, name);
             } catch (IOException | SAXException | ParserConfigurationException e) {
                 String errMsg = "Error rendering included screen named [" + name + "] at location [" + location + "]: " + e.toString();
                 Debug.logError(e, errMsg, MODULE);

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

@@ -26,6 +26,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.UtilValidate;
 import org.apache.ofbiz.base.util.UtilXml;
 import org.apache.ofbiz.base.util.cache.UtilCache;
@@ -50,6 +51,12 @@ public static ModelTree getTreeFromLocation(String resourceName, String treeName
             throws IOException, SAXException, ParserConfigurationException {
         Map<String, ModelTree> modelTreeMap = TREE_LOCATION_CACHE.get(resourceName);
         if (modelTreeMap == null) {
+            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 tree unallowed tree location");
+            }
             URL treeFileUrl = FlexibleLocation.resolveLocation(resourceName);
             if (treeFileUrl == null || UtilValidate.isUrlInStringAndDoesNotStartByComponentProtocol(treeFileUrl.toString())) {
                 throw new IllegalArgumentException("Could not resolve location to URL: " + resourceName);

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

@@ -0,0 +1,47 @@
+/*******************************************************************************
+ * 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.widget.model;
+
+import java.nio.file.Paths;
+import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.UtilValidate;
+
+public final class WidgetSecureLocation {
+
+    private static final String MODULE = WidgetSecureLocation.class.getName();
+    private static final String COMPO_TYPE = "component://";
+
+    public static String sanitize(String location) {
+        if (UtilValidate.isEmpty(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 raison traversal sequence '..' is not allowed : [%s]", location), MODULE);
+                return null;
+            }
+            return COMPO_TYPE + Paths.get(location.substring(12)).normalize();
+        }
+
+        return UtilValidate.isAllowedPath(location)
+                ? location
+                : null;
+    }
+}