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 3ae0be74d4 Improved webtools data file viewer URL handling (#1707)
(#1708)
3ae0be74d4 is described below
commit 3ae0be74d40c42f6b4e0f8838b37516e3b295253
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Mon Aug 24 00:48:15 2026 +0530
Improved webtools data file viewer URL handling (#1707) (#1708)
Gate the fetch behind DATAFILE_MAINT, restrict fetched URLs to
http/https and public addresses, disable redirect following, and stop
echoing fetched content into exception messages.
Thank you Krishna Uprit for your help.
Co-authored-by: Krishnauprit18 <[email protected]>
(cherry picked from commit b2cf9fd373ee182f35579e50e29e60776750046c,
excluding jupiter test changes)
---
.../java/org/apache/ofbiz/base/util/UtilURL.java | 148 +++++++++++++++++++++
.../java/org/apache/ofbiz/datafile/Record.java | 19 +--
.../org/apache/ofbiz/datafile/RecordIterator.java | 12 +-
framework/security/config/security.properties | 6 +
.../ofbiz/webtools/datafile/ViewDataFile.groovy | 120 +++++++++--------
5 files changed, 241 insertions(+), 64 deletions(-)
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilURL.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilURL.java
index ab88e48ba0..2d9c111d59 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilURL.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilURL.java
@@ -19,10 +19,15 @@
package org.apache.ofbiz.base.util;
import java.io.File;
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
+import java.net.UnknownHostException;
+import java.util.Locale;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -154,6 +159,149 @@ public final class UtilURL {
return url;
}
+ /**
+ * Same as {@link #fromUrlString(String)}, except the result is rejected
unless it is safe to
+ * dereference: an http or https URL whose host does not resolve to a
loopback, link-local,
+ * private, or otherwise reserved address. Use this instead of {@link
#fromUrlString(String)}
+ * whenever the URL comes from an untrusted source (e.g. a request
parameter) and will be
+ * fetched by the server, to avoid server-side request forgery against the
local file system,
+ * loopback services, and the internal network.
+ *
+ * <p>An optional host allow-list can be configured with the
+ * {@code webtools.datafile.url.allowed.hosts} property in
security.properties: a
+ * comma-separated list of hostnames/domains. When set, only URLs whose
host matches an entry
+ * (or is a subdomain of one) are allowed, in addition to the address
checks above.
+ *
+ * @throws GeneralException if the URL is present but not allowed
+ */
+ public static URL fromCheckedUrlString(String urlString) throws
GeneralException {
+ if (urlString == null) {
+ return null;
+ }
+ URL url = fromUrlString(urlString);
+ if (url == null) {
+ return null;
+ }
+ checkUrlResourceAllowed(url);
+ return url;
+ }
+
+ /**
+ * Throws {@link GeneralException} unless {@code url} is an http or https
URL whose host
+ * resolves only to publicly routable addresses (and, when configured, is
in the
+ * {@code webtools.datafile.url.allowed.hosts} allow-list). See {@link
#fromCheckedUrlString}.
+ */
+ public static void checkUrlResourceAllowed(URL url) throws
GeneralException {
+ String protocol = url.getProtocol();
+ if (!"http".equalsIgnoreCase(protocol) &&
!"https".equalsIgnoreCase(protocol)) {
+ throw new GeneralException("URL only supports http/https
protocols; rejected: " + protocol);
+ }
+ String host = url.getHost();
+ if (UtilValidate.isEmpty(host)) {
+ throw new GeneralException("URL has no host component");
+ }
+
+ String allowedHostsStr = UtilProperties.getPropertyValue("security",
"webtools.datafile.url.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 host is not in the allowed
list: " + host);
+ }
+ }
+
+ InetAddress[] addresses;
+ try {
+ addresses = InetAddress.getAllByName(host);
+ } catch (UnknownHostException e) {
+ throw new GeneralException("URL host cannot be resolved: " + host);
+ }
+ if (addresses.length == 0) {
+ throw new GeneralException("URL 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, or otherwise reserved IP range (IPv4 and IPv6).
+ */
+ private static void checkNotPrivateOrReservedAddress(InetAddress addr)
throws GeneralException {
+ if (addr.isLoopbackAddress()) {
+ throw new GeneralException("URL target resolves to a loopback
address: " + addr.getHostAddress());
+ }
+ if (addr.isLinkLocalAddress()) {
+ throw new GeneralException("URL target resolves to a link-local
address: " + addr.getHostAddress());
+ }
+ if (addr.isSiteLocalAddress()) {
+ throw new GeneralException("URL target resolves to a private
(site-local) address: " + addr.getHostAddress());
+ }
+ if (addr.isAnyLocalAddress()) {
+ throw new GeneralException("URL target resolves to a wildcard
address: " + addr.getHostAddress());
+ }
+ if (addr.isMulticastAddress()) {
+ throw new GeneralException("URL 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 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 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 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 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 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 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 target contains an invalid
IPv4-mapped IPv6 address");
+ }
+ }
+ }
+ }
+
public static URL fromOfbizHomePath(String filename) {
String ofbizHome = System.getProperty("ofbiz.home");
if (ofbizHome == null) {
diff --git
a/framework/datafile/src/main/java/org/apache/ofbiz/datafile/Record.java
b/framework/datafile/src/main/java/org/apache/ofbiz/datafile/Record.java
index 20bbba49ec..8c7ab88861 100644
--- a/framework/datafile/src/main/java/org/apache/ofbiz/datafile/Record.java
+++ b/framework/datafile/src/main/java/org/apache/ofbiz/datafile/Record.java
@@ -604,13 +604,15 @@ public class Record implements Serializable {
try {
record.setString(modelField.getName(), strVal);
} catch (java.text.ParseException e) {
+ // Note: deliberately not including strVal here; it is
unvalidated content read from
+ // the target file/URL and this message can reach the
requesting user and the log.
throw new DataFileException(
- "Could not parse field " + modelField.getName() + ",
format string \"" + modelField.getFormat() + "\" with value " + strVal
- + " on line " + lineNum, e);
+ "Could not parse field " + modelField.getName() + ",
format string \"" + modelField.getFormat()
+ + "\" on line " + lineNum, e);
} catch (java.lang.NumberFormatException e) {
throw new DataFileException(
- "Number not valid for field " + modelField.getName() +
", format string \"" + modelField.getFormat() + "\" with value "
- + strVal + " on line " + lineNum, e);
+ "Number not valid for field " + modelField.getName() +
", format string \"" + modelField.getFormat()
+ + "\" on line " + lineNum, e);
}
}
return record;
@@ -674,13 +676,14 @@ public class Record implements Serializable {
}
record.setString(modelField.getName(), strVal);
} catch (java.text.ParseException e) {
+ // Note: deliberately not including strVal here; see
createRecord above.
throw new DataFileException(
- "Could not parse field " + modelField.getName() + ",
format string \"" + modelField.getFormat() + "\" with value " + strVal
- + " on line " + lineNum, e);
+ "Could not parse field " + modelField.getName() + ",
format string \"" + modelField.getFormat()
+ + "\" on line " + lineNum, e);
} catch (java.lang.NumberFormatException e) {
throw new DataFileException(
- "Number not valid for field " + modelField.getName() +
", format string \"" + modelField.getFormat() + "\" with value "
- + strVal + " on line " + lineNum, e);
+ "Number not valid for field " + modelField.getName() +
", format string \"" + modelField.getFormat()
+ + "\" on line " + lineNum, e);
}
}
return record;
diff --git
a/framework/datafile/src/main/java/org/apache/ofbiz/datafile/RecordIterator.java
b/framework/datafile/src/main/java/org/apache/ofbiz/datafile/RecordIterator.java
index a1ec2cb3d3..46bc8a3b68 100644
---
a/framework/datafile/src/main/java/org/apache/ofbiz/datafile/RecordIterator.java
+++
b/framework/datafile/src/main/java/org/apache/ofbiz/datafile/RecordIterator.java
@@ -22,7 +22,9 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.net.HttpURLConnection;
import java.net.URL;
+import java.net.URLConnection;
import java.nio.charset.Charset;
import java.util.Stack;
@@ -53,7 +55,15 @@ public class RecordIterator {
InputStream urlStream = null;
try {
- urlStream = fileUrl.openStream();
+ // Don't follow redirects: the caller may have already validated
fileUrl's host as safe
+ // to fetch from (see UtilURL.fromCheckedUrlString), and a
redirect could point
+ // anywhere, bypassing that check.
+ URLConnection connection = fileUrl.openConnection();
+ if (connection instanceof HttpURLConnection) {
+ HttpURLConnection httpConnection = (HttpURLConnection)
connection;
+ httpConnection.setInstanceFollowRedirects(false);
+ }
+ urlStream = connection.getInputStream();
} catch (IOException e) {
throw new DataFileException("Error open URL: " +
fileUrl.toString(), e);
}
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 7ec6b9f93a..f32e1f3c1f 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -183,6 +183,12 @@ 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
+# -- Allowed hosts for URLs fetched by the webtools data file viewer/loader
(comma-separated host
+# -- names). Both exact matches and subdomain matches are supported, as above.
Leave empty to skip
+# -- host filtering and apply only the scheme (http/https only) and IP-range
checks; loopback,
+# -- link-local, private and other reserved addresses are always blocked
regardless of this setting.
+webtools.datafile.url.allowed.hosts=
+
# -- 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.
diff --git
a/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/datafile/ViewDataFile.groovy
b/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/datafile/ViewDataFile.groovy
index ed9b28709e..258f46b793 100644
---
a/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/datafile/ViewDataFile.groovy
+++
b/framework/webtools/src/main/groovy/org/apache/ofbiz/webtools/datafile/ViewDataFile.groovy
@@ -19,6 +19,7 @@
package org.apache.ofbiz.webtools.datafile
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.UtilURL
import org.apache.ofbiz.datafile.DataFile
@@ -28,78 +29,87 @@ import org.apache.ofbiz.datafile.ModelDataFileReader
uiLabelMap = UtilProperties.getResourceBundleMap('WebtoolsUiLabels', locale)
messages = []
-dataFileSave = request.getParameter('DATAFILE_SAVE')
+// This screen builds URLs from raw request parameters and dereferences them;
that fetch must
+// never happen for a principal who lacks DATAFILE_MAINT, so the whole thing
is gated here,
+// ahead of any parameter handling, rather than left to the screen/template
permission checks
+// that only gate rendering after the fetch already ran.
+if (security.hasPermission('DATAFILE_MAINT', session)) {
+ dataFileSave = request.getParameter('DATAFILE_SAVE')
-entityXmlFileSave = request.getParameter('ENTITYXML_FILE_SAVE')
+ entityXmlFileSave = request.getParameter('ENTITYXML_FILE_SAVE')
-dataFileLoc = request.getParameter('DATAFILE_LOCATION')
-definitionLoc = request.getParameter('DEFINITION_LOCATION')
-definitionName = request.getParameter('DEFINITION_NAME')
-dataFileIsUrl = null != request.getParameter('DATAFILE_IS_URL')
-definitionIsUrl = null != request.getParameter('DEFINITION_IS_URL')
+ dataFileLoc = request.getParameter('DATAFILE_LOCATION')
+ definitionLoc = request.getParameter('DEFINITION_LOCATION')
+ definitionName = request.getParameter('DEFINITION_NAME')
+ dataFileIsUrl = null != request.getParameter('DATAFILE_IS_URL')
+ definitionIsUrl = null != request.getParameter('DEFINITION_IS_URL')
-try {
- dataFileUrl = dataFileIsUrl ? UtilURL.fromUrlString(dataFileLoc) :
UtilURL.fromFilename(dataFileLoc)
-}
-catch (java.net.MalformedURLException e) {
- messages.add(e.getMessage())
-}
-
-try {
- definitionUrl = definitionIsUrl ? UtilURL.fromUrlString(definitionLoc) :
UtilURL.fromFilename(definitionLoc)
-}
-catch (java.net.MalformedURLException e) {
- messages.add(e.getMessage())
-}
-
-definitionNames = null
-if (definitionUrl) {
+ dataFileUrl = null
try {
- ModelDataFileReader reader =
ModelDataFileReader.getModelDataFileReader(definitionUrl)
- if (reader) {
- definitionNames =
((Collection)reader.getDataFileNames()).iterator()
- context.put('definitionNames', definitionNames)
- }
+ dataFileUrl = dataFileIsUrl ?
UtilURL.fromCheckedUrlString(dataFileLoc) : UtilURL.fromFilename(dataFileLoc)
}
- catch (Exception e) {
+ catch (java.net.MalformedURLException | GeneralException e) {
messages.add(e.getMessage())
}
-}
-dataFile = null
-if (dataFileUrl && definitionUrl && definitionNames) {
+ definitionUrl = null
try {
- dataFile = DataFile.readFile(dataFileUrl, definitionUrl,
definitionName)
- context.put('dataFile', dataFile)
+ definitionUrl = definitionIsUrl ?
UtilURL.fromCheckedUrlString(definitionLoc) :
UtilURL.fromFilename(definitionLoc)
}
- catch (Exception e) {
- messages.add(e.toString()); Debug.log(e)
+ catch (java.net.MalformedURLException | GeneralException e) {
+ messages.add(e.getMessage())
}
-}
-if (dataFile) {
- modelDataFile = dataFile.getModelDataFile()
- context.put('modelDataFile', modelDataFile)
-}
+ definitionNames = null
+ if (definitionUrl) {
+ try {
+ ModelDataFileReader reader =
ModelDataFileReader.getModelDataFileReader(definitionUrl)
+ if (reader) {
+ definitionNames =
((Collection)reader.getDataFileNames()).iterator()
+ context.put('definitionNames', definitionNames)
+ }
+ }
+ catch (Exception e) {
+ messages.add(e.getMessage())
+ }
+ }
-if (dataFile && dataFileSave) {
- try {
- dataFile.writeDataFile(dataFileSave)
- messages.add(uiLabelMap.WebtoolsDataFileSavedTo + dataFileSave)
+ dataFile = null
+ if (dataFileUrl && definitionUrl && definitionNames) {
+ try {
+ dataFile = DataFile.readFile(dataFileUrl, definitionUrl,
definitionName)
+ context.put('dataFile', dataFile)
+ }
+ catch (Exception e) {
+ messages.add(e.getMessage())
+ Debug.logError(e, 'Error reading data file', 'ViewDataFile.groovy')
+ }
}
- catch (Exception e) {
- messages.add(e.getMessage())
+
+ if (dataFile) {
+ modelDataFile = dataFile.getModelDataFile()
+ context.put('modelDataFile', modelDataFile)
}
-}
-if (dataFile && entityXmlFileSave) {
- try {
- //dataFile.writeDataFile(entityXmlFileSave)
- DataFile2EntityXml.writeToEntityXml(entityXmlFileSave, dataFile)
- messages.add(uiLabelMap.WebtoolsDataEntityFileSavedTo +
entityXmlFileSave)
+ if (dataFile && dataFileSave) {
+ try {
+ dataFile.writeDataFile(dataFileSave)
+ messages.add(uiLabelMap.WebtoolsDataFileSavedTo + dataFileSave)
+ }
+ catch (Exception e) {
+ messages.add(e.getMessage())
+ }
}
- catch (Exception e) {
- messages.add(e.getMessage())
+
+ if (dataFile && entityXmlFileSave) {
+ try {
+ //dataFile.writeDataFile(entityXmlFileSave)
+ DataFile2EntityXml.writeToEntityXml(entityXmlFileSave, dataFile)
+ messages.add(uiLabelMap.WebtoolsDataEntityFileSavedTo +
entityXmlFileSave)
+ }
+ catch (Exception e) {
+ messages.add(e.getMessage())
+ }
}
}
context.messages = messages
