This is an automated email from the ASF dual-hosted git repository.
jleroux 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 505c88cf45 Improved: Check parameters passed in URLs (OFBIZ-13295)
505c88cf45 is described below
commit 505c88cf45958a67e4b1bada32963455c406b8fe
Author: Jacques Le Roux <[email protected]>
AuthorDate: Fri Oct 10 20:35:44 2025 +0200
Improved: Check parameters passed in URLs (OFBIZ-13295)
Thanks to Nicolas, improves previous commits that did not handle multipart
element
Also while at it, adds a space after "process" in deniedWebShellTokens of
security.properties. I stumbled upon it after trying to upload a file
created
by a Canon camera. Anyway the deniedWebShellTokens comment suggest that when
it's necessary.
Conflicts handled by hand
---
.../org/apache/ofbiz/common/UrlServletHelper.java | 3 ++
framework/security/config/security.properties | 2 +-
.../apache/ofbiz/webapp/control/ControlFilter.java | 36 +++++++++++-----------
3 files changed, 22 insertions(+), 19 deletions(-)
diff --git
a/framework/common/src/main/java/org/apache/ofbiz/common/UrlServletHelper.java
b/framework/common/src/main/java/org/apache/ofbiz/common/UrlServletHelper.java
index 63c6856aa8..98fdbd7888 100644
---
a/framework/common/src/main/java/org/apache/ofbiz/common/UrlServletHelper.java
+++
b/framework/common/src/main/java/org/apache/ofbiz/common/UrlServletHelper.java
@@ -50,6 +50,9 @@ public final class UrlServletHelper {
public static void setRequestAttributes(ServletRequest request, Delegator
delegator, ServletContext servletContext) {
HttpServletRequest httpRequest = (HttpServletRequest) request;
// check if multi tenant is enabled
+ if (delegator == null) {
+ delegator = WebAppUtil.getDelegator(servletContext);
+ }
boolean useMultitenant = EntityUtil.isMultiTenantEnabled();
if (useMultitenant) {
// get tenant delegator by domain name
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 6dcb56a9cd..ed9a412f29 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -280,7 +280,7 @@ csvformat=CSVFormat.DEFAULT
deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,body
,<form,<jsp:,<c:out,taglib,<prefix,<%@ page,<?php,exec(,alert(,\
%eval,@eval,eval(,runtime,import,passthru,shell_exec,assert,str_rot13,system,decode,include,page
,\
chmod,mkdir,fopen,fclose,new
file,upload,getfilename,download,getoutputstring,readfile,iframe,object,embed,onload,build,\
- python,perl ,/perl,ruby
,/ruby,process,function,class,InputStream,to_server,wget
,static,assign,webappPath,\
+ python,perl ,/perl,ruby ,/ruby,process
,function,class,InputStream,to_server,wget ,static,assign,webappPath,\
ifconfig,route,crontab,netstat,uname
,hostname,iptables,whoami,"cmd",*cmd|,+cmd|,=cmd|,localhost,thread,require(,gzdeflate,\
execute,println,calc,touch,curl,base64, tcp ,4444,base32,
tr , xxd ,bash, od ,|od ,printf,echo
diff --git
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
index 0cd5e94899..5aba5dd967 100644
---
a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
+++
b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlFilter.java
@@ -24,8 +24,8 @@ import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -45,9 +45,11 @@ import org.apache.commons.validator.routines.UrlValidator;
import org.apache.logging.log4j.ThreadContext;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.StringUtil;
+import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.base.util.UtilHttp;
import org.apache.ofbiz.base.util.UtilProperties;
import org.apache.ofbiz.base.util.UtilValidate;
+import org.apache.ofbiz.common.UrlServletHelper;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.security.SecuredFreemarker;
import org.apache.ofbiz.security.SecuredUpload;
@@ -169,35 +171,33 @@ public class ControlFilter extends HttpFilter {
// Prevents stream exploitation
if (!isSolrTest()) {
+ UrlServletHelper.setRequestAttributes(req, null,
req.getServletContext());
Map<String, Object> parameters = UtilHttp.getParameterMap(req);
boolean reject = false;
if (!parameters.isEmpty()) {
for (String key : parameters.keySet()) {
Object object = parameters.get(key);
- if (object.getClass().equals(String.class)) {
- String val = (String) object;
- if (val.contains("<")) {
+ if (object.getClass().equals(String.class)
+ || object instanceof Collection) {
+ try {
+ List<String> toCheck =
object.getClass().equals(String.class)
+ ? List.of((String) object)
+ :
UtilGenerics.checkCollection(object, String.class);
+ reject = toCheck.stream()
+ .anyMatch(val -> val.contains("<"));
+ } catch (IllegalArgumentException e) {
+ Debug.logWarning(e, MODULE);
reject = true;
}
- } else {
- @SuppressWarnings("unchecked")
- LinkedList<String> vals = (LinkedList<String>)
parameters.get(key);
- for (String aVal : vals) {
- if (aVal.contains("<")) {
- reject = true;
- }
- }
}
- }
- if (reject) {
- Debug.logError("For security reason this URL is not
accepted", MODULE);
- throw new RuntimeException("For security reason this URL
is not accepted");
+ if (reject) {
+ Debug.logError("For security reason this URL is not
accepted", MODULE);
+ throw new RuntimeException("For security reason this
URL is not accepted");
+ }
}
}
}
-
-
// Check if we are told to redirect everything.
if (redirectAll) {
// little trick here so we don't loop on ourselves
