This is an automated email from the ASF dual-hosted git repository.
jacopoc pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new af8ee514a2 Improved: Enhance data resource validation and permission
checks
af8ee514a2 is described below
commit af8ee514a26d4785b3bfe8a3c042957f316af432
Author: Jacopo Cappellato <[email protected]>
AuthorDate: Wed Mar 11 08:28:13 2026 +0100
Improved: Enhance data resource validation and permission checks
---
applications/content/servicedef/services_data.xml | 2 +
.../ofbiz/content/data/DataResourceWorker.java | 272 ++++++++++++++++++++-
framework/security/config/security.properties | 29 +++
3 files changed, 298 insertions(+), 5 deletions(-)
diff --git a/applications/content/servicedef/services_data.xml
b/applications/content/servicedef/services_data.xml
index c6a22a5f56..d9432eeaf5 100644
--- a/applications/content/servicedef/services_data.xml
+++ b/applications/content/servicedef/services_data.xml
@@ -89,6 +89,7 @@
<service name="createDataResourceAndText" engine="java"
default-entity-name="DataResource" auth="true"
location="org.apache.ofbiz.content.data.DataServices"
invoke="createDataResourceAndText">
<description>Create a DataResource and, possibly, ElectronicText or
ImageDataResource</description>
+ <permission-service service-name="genericDataResourcePermission"
main-action="CREATE"/>
<auto-attributes include="pk" mode="INOUT" optional="true"/>
<auto-attributes include="nonpk" mode="IN" optional="true"/>
<attribute name="textData" mode="IN" optional="true" type="String"
allow-html="safe"/>
@@ -99,6 +100,7 @@
<service name="updateDataResourceAndText" engine="java"
default-entity-name="DataResource" auth="true"
location="org.apache.ofbiz.content.data.DataServices"
invoke="updateDataResourceAndText">
<description>Create a DataResource and, possibly, ElectronicText or
ImageDataResource</description>
+ <permission-service service-name="genericDataResourcePermission"
main-action="UPDATE"/>
<auto-attributes include="pk" mode="IN" optional="true"/>
<auto-attributes include="nonpk" mode="IN" optional="true"/>
<attribute name="textData" mode="IN" type="String" optional="true"
allow-html="safe"/>
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 d690f483bc..58008249c4 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
@@ -27,13 +27,17 @@ import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringWriter;
import java.io.Writer;
+import java.net.HttpURLConnection;
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
import java.net.URL;
import java.net.URLConnection;
+import java.net.UnknownHostException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
-import java.sql.Timestamp;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList;
@@ -55,14 +59,12 @@ import
org.apache.commons.fileupload2.core.FileUploadException;
import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload;
import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.input.BoundedInputStream;
import org.apache.ofbiz.base.location.FlexibleLocation;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.FileUtil;
import org.apache.ofbiz.base.util.GeneralException;
-import org.apache.ofbiz.base.util.StringUtil;
-import org.apache.ofbiz.base.util.StringUtil.StringWrapper;
import org.apache.ofbiz.base.util.UtilCodec;
-import org.apache.ofbiz.base.util.UtilDateTime;
import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilIO;
@@ -469,6 +471,218 @@ 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.
+ */
+ private static void checkContextFileBoundary(File file, String
contextRoot) throws GeneralException {
+ try {
+ String canonicalAllowed = new File(contextRoot).getCanonicalPath();
+ String canonicalFilePath = file.getCanonicalPath();
+ if (!canonicalFilePath.startsWith(canonicalAllowed +
File.separator)) {
+ throw new GeneralException("Access to file denied: path
resolves outside of the allowed directory");
+ }
+ } catch (IOException e) {
+ throw new GeneralException("Unable to validate file path: " +
e.getMessage());
+ }
+ }
+
+ /**
+ * Validates a URL for the URL_RESOURCE data type against SSRF
(Server-Side Request Forgery)
+ * attacks. Enforces:
+ * <ul>
+ * <li>Protocol restricted to http/https only</li>
+ * <li>Host must match the configured allow-list when
+ * {@code content.data.url.resource.allowed.hosts}
(security.properties) is non-empty;
+ * both exact and subdomain matches are supported</li>
+ * <li>All resolved IP addresses must not be private, loopback,
link-local, multicast,
+ * or otherwise reserved (mitigates DNS-rebinding)</li>
+ * </ul>
+ */
+ private static void checkUrlResourceAllowed(URL url) throws
GeneralException {
+ // 1. Protocol: only http and https are permitted
+ String protocol = url.getProtocol();
+ if (!"http".equalsIgnoreCase(protocol) &&
!"https".equalsIgnoreCase(protocol)) {
+ throw new GeneralException("URL_RESOURCE only supports http/https
protocols; rejected: " + protocol);
+ }
+ String host = url.getHost();
+ if (UtilValidate.isEmpty(host)) {
+ throw new GeneralException("URL_RESOURCE URL has no host
component");
+ }
+
+ // 2. Allow-list: if configured, the host must match one of the entries
+ String allowedHostsStr = UtilProperties.getPropertyValue("security",
+ "content.data.url.resource.allowed.hosts", "");
+ if (UtilValidate.isNotEmpty(allowedHostsStr)) {
+ String lcHost = host.toLowerCase(Locale.ROOT);
+ boolean hostAllowed = false;
+ for (String entry : allowedHostsStr.split(",")) {
+ String allowedEntry = entry.trim().toLowerCase(Locale.ROOT);
+ if (UtilValidate.isEmpty(allowedEntry)) {
+ continue;
+ }
+ if (lcHost.equals(allowedEntry) || lcHost.endsWith("." +
allowedEntry)) {
+ hostAllowed = true;
+ break;
+ }
+ }
+ if (!hostAllowed) {
+ throw new GeneralException("URL_RESOURCE host is not in the
allowed list: " + host);
+ }
+ }
+
+ // 3. DNS resolution: block private/reserved IP ranges (SSRF /
DNS-rebinding mitigation)
+ InetAddress[] addresses;
+ try {
+ addresses = InetAddress.getAllByName(host);
+ } catch (UnknownHostException e) {
+ throw new GeneralException("URL_RESOURCE host cannot be resolved:
" + host);
+ }
+ if (addresses == null || addresses.length == 0) {
+ throw new GeneralException("URL_RESOURCE host resolved to no
addresses: " + host);
+ }
+ for (InetAddress addr : addresses) {
+ checkNotPrivateOrReservedAddress(addr);
+ }
+ }
+
+ /**
+ * Throws {@link GeneralException} if {@code addr} belongs to a private,
loopback,
+ * link-local, multicast, or otherwise reserved IP range (IPv4 and IPv6).
+ */
+ private static void checkNotPrivateOrReservedAddress(InetAddress addr)
throws GeneralException {
+ if (addr.isLoopbackAddress()) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
loopback address: " + addr.getHostAddress());
+ }
+ if (addr.isLinkLocalAddress()) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
link-local address: " + addr.getHostAddress());
+ }
+ if (addr.isSiteLocalAddress()) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
private (site-local) address: " + addr.getHostAddress());
+ }
+ if (addr.isAnyLocalAddress()) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
wildcard address: " + addr.getHostAddress());
+ }
+ if (addr.isMulticastAddress()) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
multicast address: " + addr.getHostAddress());
+ }
+ byte[] b = addr.getAddress();
+ if (addr instanceof Inet4Address) {
+ int i0 = b[0] & 0xFF;
+ int i1 = b[1] & 0xFF;
+ // 0.0.0.0/8 – "this" network (RFC 1122)
+ if (i0 == 0) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
reserved network address (0.0.0.0/8): " + addr.getHostAddress());
+ }
+ // 100.64.0.0/10 – shared address space / CGNAT (RFC 6598)
+ if (i0 == 100 && i1 >= 64 && i1 <= 127) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
shared address space (CGNAT, 100.64.0.0/10): " + addr.getHostAddress());
+ }
+ // 192.0.0.0/24 – IETF protocol assignments (RFC 6890)
+ if (i0 == 192 && i1 == 0 && (b[2] & 0xFF) == 0) {
+ throw new GeneralException("URL_RESOURCE target resolves to an
IETF reserved address (192.0.0.0/24): " + addr.getHostAddress());
+ }
+ // 198.18.0.0/15 – network benchmarking (RFC 2544)
+ if (i0 == 198 && (i1 == 18 || i1 == 19)) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
benchmarking address (198.18.0.0/15): " + addr.getHostAddress());
+ }
+ // 240.0.0.0/4 – reserved for future use (RFC 1112)
+ if ((i0 & 0xF0) == 240) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
reserved address (240.0.0.0/4): " + addr.getHostAddress());
+ }
+ } else if (addr instanceof Inet6Address) {
+ // fc00::/7 – Unique Local Addresses (ULA), private IPv6 (RFC 4193)
+ if ((b[0] & 0xFE) == 0xFC) {
+ throw new GeneralException("URL_RESOURCE target resolves to a
unique-local (private) IPv6 address: " + addr.getHostAddress());
+ }
+ // ::ffff:0:0/96 – IPv4-mapped IPv6; re-validate the embedded IPv4
address
+ boolean isIpv4Mapped = true;
+ for (int i = 0; i < 10; i++) {
+ if (b[i] != 0) {
+ isIpv4Mapped = false;
+ break;
+ }
+ }
+ if (isIpv4Mapped && (b[10] & 0xFF) == 0xFF && (b[11] & 0xFF) ==
0xFF) {
+ try {
+ checkNotPrivateOrReservedAddress(
+ InetAddress.getByAddress(new byte[]{b[12], b[13],
b[14], b[15]}));
+ } catch (UnknownHostException e) {
+ throw new GeneralException("URL_RESOURCE target contains
an invalid IPv4-mapped IPv6 address");
+ }
+ }
+ }
+ }
+
public static File getContentFile(String dataResourceTypeId, String
objectInfo, String contextRoot)
throws GeneralException, FileNotFoundException {
File file = null;
@@ -481,6 +695,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
if (!file.isAbsolute()) {
throw new GeneralException("File (" + objectInfo + ") is not
absolute");
}
+ checkLocalFileAllowList(file);
} else if ("OFBIZ_FILE".equals(dataResourceTypeId) ||
"OFBIZ_FILE_BIN".equals(dataResourceTypeId)) {
String prefix = System.getProperty("ofbiz.home");
@@ -492,6 +707,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);
} 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!");
@@ -505,6 +721,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
if (!file.exists()) {
throw new FileNotFoundException("No file found: " +
(contextRoot + sep + objectInfo));
}
+ checkContextFileBoundary(file, contextRoot);
}
return file;
@@ -722,6 +939,8 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
// FTL template
if ("FTL".equals(dataTemplateTypeId)) {
+ throw new GeneralException("Error rendering template: FTL
template type is no longer supported for data resources.");
+ /*
try {
// get the template data for rendering
String templateText = getDataResourceText(dataResource,
targetMimeTypeId, locale, templateContext, delegator, cache);
@@ -757,6 +976,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
} catch (TemplateException e) {
throw new GeneralException("Error rendering FTL template",
e);
}
+ */
} else if ("XSLT".equals(dataTemplateTypeId)) {
File targetFileLocation = new
File(System.getProperty("ofbiz.home") + "/runtime/tempfiles/docbook.css");
@@ -1039,6 +1259,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
if (!file.exists()) {
throw new FileNotFoundException("No file found: " +
file.getAbsolutePath());
}
+ checkLocalFileAllowList(file);
try (InputStreamReader in = new InputStreamReader(new
FileInputStream(file), StandardCharsets.UTF_8)) {
UtilIO.copy(in, out);
}
@@ -1052,6 +1273,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
if (!file.exists()) {
throw new FileNotFoundException("No file found: " +
file.getAbsolutePath());
}
+ checkOfbizFileAllowList(file);
try (InputStreamReader in = new InputStreamReader(new
FileInputStream(file), StandardCharsets.UTF_8)) {
UtilIO.copy(in, out);
}
@@ -1065,6 +1287,7 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
if (!file.exists()) {
throw new FileNotFoundException("No file found: " +
file.getAbsolutePath());
}
+ checkContextFileBoundary(file, rootDir);
try (InputStreamReader in = new InputStreamReader(new
FileInputStream(file), StandardCharsets.UTF_8)) {
if (Debug.infoOn()) {
String enc = in.getEncoding();
@@ -1183,8 +1406,47 @@ public class DataResourceWorker implements
org.apache.ofbiz.widget.content.DataR
url = UtilURL.fromUrlString(newUrl);
}
+ // SSRF prevention: validate protocol, optional host
allow-list, and resolved IP ranges
+ checkUrlResourceAllowed(url);
+
+ int connectTimeout = (int)
UtilProperties.getPropertyNumber("security",
+ "content.data.url.resource.connect.timeout", 10000.0);
+ int readTimeout = (int)
UtilProperties.getPropertyNumber("security",
+ "content.data.url.resource.read.timeout", 30000.0);
+ long maxResponseSize = (long)
UtilProperties.getPropertyNumber("security",
+ "content.data.url.resource.max.response.size",
(double) (10L * 1024 * 1024));
+
URLConnection con = url.openConnection();
- return UtilMisc.toMap("stream", con.getInputStream(),
"length", (long) con.getContentLength());
+ con.setConnectTimeout(connectTimeout);
+ con.setReadTimeout(readTimeout);
+ // Disable automatic redirect-following to prevent SSRF bypass
via redirect to private addresses
+ if (con instanceof HttpURLConnection) ((HttpURLConnection)
con).setInstanceFollowRedirects(false);
+ con.connect();
+
+ // Reject redirects outright; we cannot safely re-validate an
arbitrary Location header
+ if (con instanceof HttpURLConnection) {
+ HttpURLConnection httpCon = (HttpURLConnection) con;
+ int responseCode = httpCon.getResponseCode();
+ if (responseCode >= 300 && responseCode < 400) {
+ httpCon.disconnect();
+ throw new GeneralException("URL_RESOURCE request
returned a redirect (" + responseCode
+ + "); redirects are not followed for security
reasons");
+ }
+ }
+
+ long contentLength = con.getContentLengthLong();
+ if (contentLength > maxResponseSize) {
+ if (con instanceof HttpURLConnection) ((HttpURLConnection)
con).disconnect();
+ throw new GeneralException("URL_RESOURCE response
Content-Length (" + contentLength
+ + " bytes) exceeds the configured maximum of " +
maxResponseSize + " bytes");
+ }
+
+ // Wrap with a bounded stream to enforce the size cap
regardless of the Content-Length header
+ InputStream limitedStream = BoundedInputStream.builder()
+ .setInputStream(con.getInputStream())
+ .setMaxCount(maxResponseSize)
+ .get();
+ return UtilMisc.toMap("stream", limitedStream, "length",
contentLength);
}
throw new GeneralException("No objectInfo found for URL_RESOURCE
type; cannot stream");
}
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 2e0d3ba0ed..10d2e5562d 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -175,6 +175,35 @@ security.token.key=
# During validation, only tokens containing this audience value will be
considered valid.
#security.token.audience=
+# -- Allowed subdirectories (relative to ofbiz.home) for the OFBIZ_FILE /
OFBIZ_FILE_BIN data resource types.
+# -- Comma-separated list, no spaces after commas, no leading slashes.
+# -- Only files whose resolved canonical path starts with one of these entries
will be served.
+# -- Set to empty to allow any path within ofbiz.home (the ofbiz.home
containment check still applies).
+content.data.ofbiz.file.allowed.paths=applications/,themes/,plugins/,runtime/
+
+# -- Allowed directories for the LOCAL_FILE / LOCAL_FILE_BIN data resource
types (absolute paths).
+# -- Comma-separated, no spaces after commas. Use ${ofbiz.home} as a portable
placeholder.
+# -- Only files whose resolved canonical path starts with one of these entries
will be served.
+# -- Set to empty to disable this check (NOT recommended).
+content.data.local.file.allowed.paths=${ofbiz.home}
+
+# -- Allowed hosts for the URL_RESOURCE data resource type (comma-separated
host names or host:port values).
+# -- Both exact matches and subdomain matches are supported: "example.com"
also permits "cdn.example.com".
+# -- Leave empty to skip host filtering and apply only IP-range checks
(private/loopback/link-local/multicast
+# -- addresses are always blocked regardless of this setting).
+content.data.url.resource.allowed.hosts=
+
+# -- Connection timeout in milliseconds for URL_RESOURCE fetches. Default:
10000 (10 s).
+content.data.url.resource.connect.timeout=10000
+
+# -- Read timeout in milliseconds for URL_RESOURCE fetches. Default: 30000 (30
s).
+content.data.url.resource.read.timeout=30000
+
+# -- Maximum response body size in bytes for URL_RESOURCE fetches. Default:
10485760 (10 MB).
+# -- Responses advertising a larger Content-Length are refused before the
connection body is read;
+# -- the returned InputStream is additionally capped at this size regardless
of the declared length.
+content.data.url.resource.max.response.size=10485760
+
# -- List of domains or IP addresses to be checked to prevent Host Header
Injection,
# -- no spaces after commas,no wildcard, can be extended of course...
host-headers-allowed=localhost,127.0.0.1,demo-trunk.ofbiz.apache.org,demo-stable.ofbiz.apache.org,demo-next.ofbiz.apache.org,demo-trunk.ofbiz-test.apache.org,demo-stable-test.ofbiz.apache.org,demo-next.ofbiz-test.apache.org
