This is an automated email from the ASF dual-hosted git repository.
jleroux 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 357da7f17e Improved: Check parameters passed in URLs (OFBIZ-13295)
357da7f17e is described below
commit 357da7f17e3dd63d58177b514769d21aa56d036d
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.
---
.../java/org/apache/ofbiz/base/util/UtilHttp.java | 9 ++++---
.../org/apache/ofbiz/common/UrlServletHelper.java | 19 ++++++++-------
framework/security/config/security.properties | 2 +-
.../apache/ofbiz/webapp/control/ControlFilter.java | 28 +++++++++++-----------
4 files changed, 30 insertions(+), 28 deletions(-)
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
index 04465aaf9b..83bbe85d5e 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
@@ -65,16 +65,12 @@ import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.net.ssl.SSLContext;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-import jakarta.servlet.http.HttpSession;
import org.apache.commons.fileupload2.core.DiskFileItem;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
import org.apache.commons.fileupload2.core.FileItem;
import org.apache.commons.fileupload2.core.FileUploadException;
import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload;
-
import org.apache.commons.lang.RandomStringUtils;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;
@@ -94,6 +90,10 @@ import org.apache.ofbiz.widget.renderer.VisualTheme;
import com.google.re2j.Matcher;
import com.google.re2j.Pattern;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.HttpSession;
+
/**
* HttpUtil - Misc HTTP Utility Functions
*/
@@ -230,7 +230,6 @@ public final class UtilHttp {
if (isMultiPart) {
// create the progress listener and add it to the session
String encoding = request.getCharacterEncoding();
- Charset charset = Charset.forName(encoding);
JakartaServletFileUpload<DiskFileItem, DiskFileItemFactory> upload
= UtilHttp.getServletFileUpload(request);
FileUploadProgressListener listener = new
FileUploadProgressListener();
upload.setProgressListener(listener);
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 b4b9c2c674..8857cb547a 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
@@ -21,14 +21,6 @@ package org.apache.ofbiz.common;
import java.io.IOException;
import java.util.List;
-import jakarta.servlet.RequestDispatcher;
-import jakarta.servlet.ServletContext;
-import jakarta.servlet.ServletException;
-import jakarta.servlet.ServletRequest;
-import jakarta.servlet.ServletResponse;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilValidate;
@@ -41,6 +33,14 @@ import org.apache.ofbiz.entity.util.EntityUtil;
import org.apache.ofbiz.webapp.WebAppUtil;
import org.apache.ofbiz.webapp.website.WebSiteWorker;
+import jakarta.servlet.RequestDispatcher;
+import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
public final class UrlServletHelper {
private static final String MODULE = UrlServletHelper.class.getName();
@@ -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 b5653c6518..a67d10be92 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -283,7 +283,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 4002d823fc..da80c8b9c5 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;
@@ -37,9 +37,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;
@@ -164,24 +166,24 @@ public class ControlFilter extends HttpFilter {
HttpSession session = req.getSession();
// Prevents stream exploitation
+ 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) {
@@ -190,8 +192,6 @@ public class ControlFilter extends HttpFilter {
}
}
-
-
// Check if we are told to redirect everything.
if (redirectAll) {
// little trick here so we don't loop on ourselves
