This is an automated email from the ASF dual-hosted git repository. jacopoc pushed a commit to branch release24.09 in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit 25dfaec580a2c4a595e59da88e3dbce6ba51db58 Author: Jacopo Cappellato <[email protected]> AuthorDate: Wed Apr 8 10:48:18 2026 +0200 Improved: Move file path validation to SecurityUtil (cherry picked from commit 05fbeb5c2c4991550d9d99207531308796befeef) --- .../ofbiz/content/data/DataResourceWorker.java | 79 ++-------------------- .../org/apache/ofbiz/security/SecurityUtil.java | 74 ++++++++++++++++++++ 2 files changed, 79 insertions(+), 74 deletions(-) diff --git a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java index 01400c8067..1b749d0326 100644 --- a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java +++ b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java @@ -84,6 +84,7 @@ import org.apache.ofbiz.entity.util.EntityQuery; import org.apache.ofbiz.entity.util.EntityUtilProperties; import org.apache.ofbiz.service.GenericServiceException; import org.apache.ofbiz.service.LocalDispatcher; +import org.apache.ofbiz.security.SecurityUtil; import org.apache.ofbiz.service.ServiceUtil; import org.apache.ofbiz.widget.model.FormFactory; import org.apache.ofbiz.widget.model.ModelForm; @@ -468,76 +469,6 @@ public class DataResourceWorker implements org.apache.ofbiz.widget.content.DataR return prefix; } - /** - * Checks that the given file is within one of the directories listed in - * {@code content.data.local.file.allowed.paths} (security.properties). - * Use {@code ${ofbiz.home}} as a portable placeholder for the OFBiz home directory. - */ - private static void checkLocalFileAllowList(File file) throws GeneralException { - try { - String canonicalFilePath = file.getCanonicalPath(); - String ofbizHome = System.getProperty("ofbiz.home"); - String allowedPathsStr = UtilProperties.getPropertyValue("security", - "content.data.local.file.allowed.paths", "${ofbiz.home}"); - if (UtilValidate.isNotEmpty(allowedPathsStr)) { - boolean inAllowedPath = false; - for (String allowedPath : allowedPathsStr.split(",")) { - allowedPath = allowedPath.trim().replace("${ofbiz.home}", ofbizHome); - if (UtilValidate.isEmpty(allowedPath)) { - continue; - } - String canonicalAllowedDir = new File(allowedPath).getCanonicalPath(); - if (canonicalFilePath.startsWith(canonicalAllowedDir + File.separator) - || canonicalFilePath.equals(canonicalAllowedDir)) { - inAllowedPath = true; - break; - } - } - if (!inAllowedPath) { - throw new GeneralException("Access to file denied: path is not within an allowed directory"); - } - } - } catch (IOException e) { - throw new GeneralException("Unable to validate file path: " + e.getMessage()); - } - } - - /** - * Checks that the given file is within the OFBiz home directory and within one of the - * subdirectories listed in {@code content.data.ofbiz.file.allowed.paths} (security.properties). - */ - private static void checkOfbizFileAllowList(File file) throws GeneralException { - try { - String canonicalHome = new File(System.getProperty("ofbiz.home")).getCanonicalPath(); - String canonicalFilePath = file.getCanonicalPath(); - if (!canonicalFilePath.startsWith(canonicalHome + File.separator)) { - throw new GeneralException("Access to file denied: path resolves outside of the OFBiz home directory"); - } - String allowedPathsStr = UtilProperties.getPropertyValue("security", - "content.data.ofbiz.file.allowed.paths", "applications/,themes/,plugins/,runtime/"); - if (UtilValidate.isNotEmpty(allowedPathsStr)) { - boolean inAllowedPath = false; - for (String relPath : allowedPathsStr.split(",")) { - relPath = relPath.trim().replaceAll("^/+", ""); - if (UtilValidate.isEmpty(relPath)) { - continue; - } - String canonicalAllowedDir = new File(canonicalHome, relPath).getCanonicalPath(); - if (canonicalFilePath.startsWith(canonicalAllowedDir + File.separator) - || canonicalFilePath.equals(canonicalAllowedDir)) { - inAllowedPath = true; - break; - } - } - if (!inAllowedPath) { - throw new GeneralException("Access to file denied: path is not within an allowed directory"); - } - } - } catch (IOException e) { - throw new GeneralException("Unable to validate file path: " + e.getMessage()); - } - } - /** * Checks that the given file is within the provided context root directory. */ @@ -692,7 +623,7 @@ public class DataResourceWorker implements org.apache.ofbiz.widget.content.DataR if (!file.isAbsolute()) { throw new GeneralException("File (" + objectInfo + ") is not absolute"); } - checkLocalFileAllowList(file); + SecurityUtil.checkLocalFileAllowList(file); } else if ("OFBIZ_FILE".equals(dataResourceTypeId) || "OFBIZ_FILE_BIN".equals(dataResourceTypeId)) { String prefix = System.getProperty("ofbiz.home"); @@ -704,7 +635,7 @@ public class DataResourceWorker implements org.apache.ofbiz.widget.content.DataR if (!file.exists()) { throw new FileNotFoundException("No file found: " + (prefix + sep + objectInfo)); } - checkOfbizFileAllowList(file); + SecurityUtil.checkOfbizFileAllowList(file); } else if ("CONTEXT_FILE".equals(dataResourceTypeId) || "CONTEXT_FILE_BIN".equals(dataResourceTypeId)) { if (UtilValidate.isEmpty(contextRoot)) { throw new GeneralException("Cannot find CONTEXT_FILE with an empty context root!"); @@ -1257,7 +1188,7 @@ public class DataResourceWorker implements org.apache.ofbiz.widget.content.DataR if (!file.exists()) { throw new FileNotFoundException("No file found: " + file.getAbsolutePath()); } - checkLocalFileAllowList(file); + SecurityUtil.checkLocalFileAllowList(file); try (InputStreamReader in = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) { UtilIO.copy(in, out); } @@ -1271,7 +1202,7 @@ public class DataResourceWorker implements org.apache.ofbiz.widget.content.DataR if (!file.exists()) { throw new FileNotFoundException("No file found: " + file.getAbsolutePath()); } - checkOfbizFileAllowList(file); + SecurityUtil.checkOfbizFileAllowList(file); try (InputStreamReader in = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) { UtilIO.copy(in, out); } diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java index 0f6adbd14f..ce61571985 100644 --- a/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java +++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecurityUtil.java @@ -19,12 +19,16 @@ package org.apache.ofbiz.security; +import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; import org.apache.ofbiz.base.util.Debug; +import org.apache.ofbiz.base.util.GeneralException; +import org.apache.ofbiz.base.util.UtilProperties; import org.apache.ofbiz.base.util.StringUtil; import org.apache.ofbiz.base.util.UtilMisc; import org.apache.ofbiz.base.util.UtilValidate; @@ -135,6 +139,76 @@ public final class SecurityUtil { return false; } + /** + * Checks that the given file is within one of the directories listed in + * {@code content.data.local.file.allowed.paths} (security.properties). + * Use {@code ${ofbiz.home}} as a portable placeholder for the OFBiz home directory. + */ + public static void checkLocalFileAllowList(File file) throws GeneralException { + try { + String canonicalFilePath = file.getCanonicalPath(); + String ofbizHome = System.getProperty("ofbiz.home"); + String allowedPathsStr = UtilProperties.getPropertyValue("security", + "content.data.local.file.allowed.paths", "${ofbiz.home}"); + if (UtilValidate.isNotEmpty(allowedPathsStr)) { + boolean inAllowedPath = false; + for (String allowedPath : allowedPathsStr.split(",")) { + allowedPath = allowedPath.trim().replace("${ofbiz.home}", ofbizHome); + if (UtilValidate.isEmpty(allowedPath)) { + continue; + } + String canonicalAllowedDir = new File(allowedPath).getCanonicalPath(); + if (canonicalFilePath.startsWith(canonicalAllowedDir + File.separator) + || canonicalFilePath.equals(canonicalAllowedDir)) { + inAllowedPath = true; + break; + } + } + if (!inAllowedPath) { + throw new GeneralException("Access to file denied: path is not within an allowed directory"); + } + } + } catch (IOException e) { + throw new GeneralException("Unable to validate file path: " + e.getMessage()); + } + } + + /** + * Checks that the given file is within the OFBiz home directory and within one of the + * subdirectories listed in {@code content.data.ofbiz.file.allowed.paths} (security.properties). + */ + public static void checkOfbizFileAllowList(File file) throws GeneralException { + try { + String canonicalHome = new File(System.getProperty("ofbiz.home")).getCanonicalPath(); + String canonicalFilePath = file.getCanonicalPath(); + if (!canonicalFilePath.startsWith(canonicalHome + File.separator)) { + throw new GeneralException("Access to file denied: path resolves outside of the OFBiz home directory"); + } + String allowedPathsStr = UtilProperties.getPropertyValue("security", + "content.data.ofbiz.file.allowed.paths", "applications/,themes/,plugins/,runtime/"); + if (UtilValidate.isNotEmpty(allowedPathsStr)) { + boolean inAllowedPath = false; + for (String relPath : allowedPathsStr.split(",")) { + relPath = relPath.trim().replaceAll("^/+", ""); + if (UtilValidate.isEmpty(relPath)) { + continue; + } + String canonicalAllowedDir = new File(canonicalHome, relPath).getCanonicalPath(); + if (canonicalFilePath.startsWith(canonicalAllowedDir + File.separator) + || canonicalFilePath.equals(canonicalAllowedDir)) { + inAllowedPath = true; + break; + } + } + if (!inAllowedPath) { + throw new GeneralException("Access to file denied: path is not within an allowed directory"); + } + } + } catch (IOException e) { + throw new GeneralException("Unable to validate file path: " + e.getMessage()); + } + } + /** * Return a JWToken for authenticate a userLogin with salt the token by userLoginId and currentPassword */
