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 eee6ccb71b Improved: Prevent URL parameters manipulation (OFBIZ-13147)
eee6ccb71b is described below
commit eee6ccb71b40637b4055ed507f81502479091051
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Tue Nov 26 07:56:44 2024 +0100
Improved: Prevent URL parameters manipulation (OFBIZ-13147)
This follows Leïla comment. It simplifies the
SecuredUpload::isValidEncodedText
content and renames it isNotValidEncodedText
Also 3 new deniedWebShellTokens are added, some more to come
To quickly test on trunk demo the fix from OFBIZ-13162 is temporarily
reverted
---
framework/security/config/security.properties | 3 ++-
.../main/java/org/apache/ofbiz/security/SecuredUpload.java | 11 +++++------
.../apache/ofbiz/widget/renderer/macro/MacroMenuRenderer.java | 7 +------
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/framework/security/config/security.properties
b/framework/security/config/security.properties
index 0dd78172a1..64556a3380 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -274,7 +274,8 @@ csvformat=CSVFormat.DEFAULT
#--
#-- If you are sure you are safe for a token you can remove it, etc.
#-- If you add a token beware that it does not content ",". It's the separator.
-deniedWebShellTokens=$SHA$OFBiz$c_93W08vqLMlJHjOZ7_A6Wcaenw,$SHA$OFBiz$SigPYIfwat32A80hsAOakW0uH5A,$SHA$OFBiz$--GB0cWVhqHm-dWklW-zlGAIMkU,$SHA$OFBiz$4LL0rcLbpJHftX4g1WeF8ThuKyQ,$SHA$OFBiz$pUBkkg8Z-CiOTIYhIR1kU3DgXqY,$SHA$OFBiz$kpcFR3kDCOtNybDHn8ZPLuCVrOk,$SHA$OFBiz$zadWo3Yv2v9ArAgtj5Hdy1yjjAA,$SHA$OFBiz$gcjailglxcjBO361A7K66-4daLs,$SHA$OFBiz$5z4tXuvujvU8WlSrn3i11oUNFZo,$SHA$OFBiz$uYjP2BSE6bJ8V2QuXPWgiiwcss0,$SHA$OFBiz$fjfa3KJJBB3t7rGS5wh6outrKoY,$SHA$OFBiz$z-t-R4DxwjsPhagQBrQRCBdf3BY,$SH
[...]
+deniedWebShellTokens=$SHA$OFBiz$c_93W08vqLMlJHjOZ7_A6Wcaenw,$SHA$OFBiz$SigPYIfwat32A80hsAOakW0uH5A,$SHA$OFBiz$--GB0cWVhqHm-dWklW-zlGAIMkU,$SHA$OFBiz$4LL0rcLbpJHftX4g1WeF8ThuKyQ,$SHA$OFBiz$pUBkkg8Z-CiOTIYhIR1kU3DgXqY,$SHA$OFBiz$kpcFR3kDCOtNybDHn8ZPLuCVrOk,$SHA$OFBiz$zadWo3Yv2v9ArAgtj5Hdy1yjjAA,$SHA$OFBiz$gcjailglxcjBO361A7K66-4daLs,$SHA$OFBiz$5z4tXuvujvU8WlSrn3i11oUNFZo,$SHA$OFBiz$uYjP2BSE6bJ8V2QuXPWgiiwcss0,$SHA$OFBiz$fjfa3KJJBB3t7rGS5wh6outrKoY,$SHA$OFBiz$z-t-R4DxwjsPhagQBrQRCBdf3BY,$SH
[...]
+
#-- SHA-1 versions of tokens containing (as String) at least one
deniedWebShellTokens
#-- This is notably used to allow special values in query parameters.
diff --git
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
index ed997d36aa..f158262c3f 100644
---
a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
+++
b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -129,19 +129,18 @@ public class SecuredUpload {
* @return
* @throws IOException
*/
- public static boolean isValidEncodedText(String content, List<String>
allowed) throws IOException {
+ public static boolean isNotValidEncodedText(String content, List<String>
allowed) throws IOException {
try {
- return
!isValidText(String.valueOf(Base64.getDecoder().decode(content)), allowed)
- ||
!isValidText(String.valueOf(Base64.getMimeDecoder().decode(content)), allowed)
- ||
!isValidText(String.valueOf(Base64.getUrlDecoder().decode(content)), allowed);
+ return isValidText(new String(Base64.getDecoder().decode(content),
StandardCharsets.UTF_8), allowed);
} catch (IllegalArgumentException e) {
// the encoded text isn't a Base64, allow it because there is no
security risk
- return true;
+ return false;
}
}
// Cover method of the same name below. Historically used with 84
references when below was created
// This is used for checking there is no web shell in an uploaded file
+ // A file containing a reverse shell, base64 encoded or not, will be
rejected.
public static boolean isValidText(String content, List<String> allowed)
throws IOException {
return isValidText(content, allowed, false);
}
@@ -159,7 +158,7 @@ public class SecuredUpload {
}
}
// This is used for checking there is no reverse shell in a query
string
- if (testEncodeContent && !isValidEncodedText(content, allowed)) {
+ if (testEncodeContent && isNotValidEncodedText(content, allowed)) {
return false;
} else if (testEncodeContent) {
// e.g. split parameters of an at all non encoded HTTP query
string
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroMenuRenderer.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroMenuRenderer.java
index c989c32819..0a5b96310d 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroMenuRenderer.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroMenuRenderer.java
@@ -268,12 +268,7 @@ public class MacroMenuRenderer implements
MenuStringRenderer {
targetParameters.append(parameter.getKey());
targetParameters.append("'");
targetParameters.append(",'value':'");
- UtilCodec.SimpleEncoder simpleEncoder =
(UtilCodec.SimpleEncoder) context.get("simpleEncoder");
- if (simpleEncoder != null) {
-
targetParameters.append(simpleEncoder.encode(parameter.getValue()));
- } else {
- targetParameters.append(parameter.getValue());
- }
+ targetParameters.append(parameter.getValue());
targetParameters.append("'}");
}
targetParameters.append("]");
