Skip to content

Commit

Permalink
Fixed: [SECURITY] (CVE-2024-36104) Path traversal leading to RCE (OFB…
Browse files Browse the repository at this point in the history
…IZ-13092)

Adds a StringUtil::splitWithStringSeparator. I crossed issue using
StringUtil::split it's said that <<delim the delimiter character(s)>>
But it does not work as expected with several character(s).

Removes an allowedToken and add 3 others.

In ControlFilter::doFilter uses splitWithStringSeparator instead of split.
Uses decoded requestUri everywhere.

In ControlFilter::doFilter uses splitWithStringSeparator instead of split.
Uses decoded requestUri everywhere, and to split query string
"&amp;" rather than "Y&amp;".

Not backported (impossible), all by hand
  • Loading branch information
JacquesLeRoux committed Jan 17, 2025
1 parent 52bf8b3 commit e5ccc13
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -157,6 +158,19 @@ public static List<String> split(String str, String delim) {
return splitList;
}

/**
* Splits a String on a String Separator into a List of Strings.
* @param str the String to split
* @param separator the String Separator to split the str String
* @return a list of Strings or null if one of the parameters is null
*/
public static List<String> splitWithStringSeparator(String str, String separator) {
if (str == null || separator == null) {
return null;
}
return Arrays.asList(str.split(separator));
}

/**
* Splits a String on a delimiter into a List of Strings.
* @param str the String to split
Expand Down
2 changes: 1 addition & 1 deletion framework/security/config/security.properties
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ deniedWebShellTokens=java.,beans,freemarker,<script,javascript,<body,body ,<form
#-- SHA-1 versions of tokens containing (as String) at least one deniedWebShellTokens
#-- This is notably used to allow special values in query parameters.
#-- If you add a token beware that it does not content ",". It's the separator.
allowedTokens=$SHA$OFBiz$2naHrANKTniFcgLJk4oXr3IRQ48,$SHA$OFBiz$evAu1vcT5d1tjVXFTeVXU-6aNz8,$SHA$OFBiz$-MaMN-Dui294v86UT1T8BkG3v8k
allowedTokens=$SHA$OFBiz$2naHrANKTniFcgLJk4oXr3IRQ48,$SHA$OFBiz$evAu1vcT5d1tjVXFTeVXU-6aNz8,$SHA$OFBiz$ORZaKvS7a0ee4gZb9P5hHuHnEyE,$SHA$OFBiz$T5DBu6tPuZzDCfYNci_23SrUa3Q,$SHA$OFBiz$BXhGVix7t3kfHrhNB0z9I0H9_rQ

allowStringConcatenationInUploadedFiles=false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ && isSolrTest()) {
if (!requestUri.matches("/control/logout;jsessionid=[A-Z0-9]{32}\\.jvm1")) {
boolean bypass = true;
if (queryString != null) {
bypass = isAnyAllowedToken(StringUtil.split(queryString.toLowerCase(), "Y&amp;"), ALLOWEDTOKENS);
List<String> queryStringList = StringUtil.splitWithStringSeparator(queryString.toLowerCase(), "&amp;");
bypass = isAnyAllowedToken(queryStringList, ALLOWEDTOKENS);
}
if (requestUri != null && !bypass) { // "null" allows tests with Mockito. ControlFilterTests sends null.
try {
String url = new URI(((HttpServletRequest) request).getRequestURL().toString())
String url = new URI(requestUri)
.normalize().toString()
.replaceAll(";", "")
.replaceAll("(?i)%2e", "");
Expand Down

0 comments on commit e5ccc13

Please sign in to comment.