You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2022/02/17 18:03:40 UTC
[ofbiz-framework] branch release22.01 updated (124eb38 -> 70c799d)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a change to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.
from 124eb38 Improved: Error in uploading very large files, ie >2GB (OFBIZ-11534)
new 52ae393 Fixed: Secure the uploads (OFBIZ-12080)
new 70c799d Fixed: Secure the uploads (OFBIZ-12080)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
framework/security/config/security.properties | 4 ++++
.../org/apache/ofbiz/security/SecuredUpload.java | 27 ++++++++++++++++++++++
2 files changed, 31 insertions(+)
[ofbiz-framework] 01/02: Fixed: Secure the uploads (OFBIZ-12080)
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit 52ae39313855324a13928063e09f1d52532b2b1c
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Thu Feb 17 18:48:34 2022 +0100
Fixed: Secure the uploads (OFBIZ-12080)
Prevents
* too long lines (10 000) by default
* linked images inside SVG
Adds a comment about double extensions not allowed
---
framework/security/config/security.properties | 4 ++++
.../org/apache/ofbiz/security/SecuredUpload.java | 27 ++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index e2c1473..7661561 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -255,6 +255,10 @@ deniedWebShellTokens=freemarker,<script,javascript,<body,<form,<jsp:,<c:out,tagl
python,perl ,/perl,ruby ,/ruby,process,function,class,InputStream,to_server,\
ifconfig,route,crontab,netstat,uname,hostname,iptables,whoami
+
+#-- Max line length for uploaded files, by default 10000
+maxLineLength=
+
#-- Popup last-visited time from database after user has logged in.
#-- So users can know of any unauthorised access to their accounts.
#-- Default is false.
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 f595f80..151c9c8 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
@@ -101,6 +101,7 @@ public class SecuredUpload {
private static final String MODULE = SecuredUpload.class.getName();
private static final List<String> DENIEDFILEEXTENSIONS = deniedFileExtensions();
private static final List<String> DENIEDWEBSHELLTOKENS = deniedWebShellTokens();
+ private static final Integer maxLineLength = UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000);
public static boolean isValidText(String content, List<String> allowed) throws IOException {
return DENIEDWEBSHELLTOKENS.stream().allMatch(token -> isValid(content, token, allowed));
@@ -114,6 +115,13 @@ public class SecuredUpload {
// Prevent double extensions
if (StringUtils.countMatches(fileToCheck, ".") > 1) {
+ Debug.logError("Double extensions are not allowed for security reason", MODULE);
+ return false;
+ }
+
+ // Check max line length, default 10000
+ if (!checkMaxLinesLength(fileToCheck)) {
+ Debug.logError("For security reason lines over " + maxLineLength.toString() + " are not allowed", MODULE);
return false;
}
@@ -646,6 +654,10 @@ public class SecuredUpload {
}
}
String content = new String(bytesFromFile);
+ if (content.toLowerCase().contains("xlink:href")) {
+ Debug.logError("Linked images inside SVG are not allowed for security reason", MODULE);
+ return false;
+ }
ArrayList<String> allowed = new ArrayList<>();
return isValidText(content, allowed);
}
@@ -671,4 +683,19 @@ public class SecuredUpload {
String deniedTokens = UtilProperties.getPropertyValue("security", "deniedWebShellTokens");
return UtilValidate.isNotEmpty(deniedTokens) ? StringUtil.split(deniedTokens, ",") : new ArrayList<>();
}
+
+ private static boolean checkMaxLinesLength(String fileToCheck) {
+ try {
+ File file = new File(fileToCheck);
+ List<String> lines = FileUtils.readLines(file, Charset.defaultCharset());
+ for (String line : lines) {
+ if (line.length() > maxLineLength) {
+ return false;
+ }
+ }
+ } catch (IOException e) {
+ return false;
+ }
+ return true;
+ }
}
[ofbiz-framework] 02/02: Fixed: Secure the uploads (OFBIZ-12080)
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release22.01
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit 70c799dc63312ae5953a501cd19e4eb838771868
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Thu Feb 17 18:48:34 2022 +0100
Fixed: Secure the uploads (OFBIZ-12080)
Prevents
* too long lines (10 000) by default
* linked images inside SVG
Adds a comment about double extensions not allowed
Conflicts handled by hand
SecuredUpload.java
---
.../src/main/java/org/apache/ofbiz/security/SecuredUpload.java | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
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 151c9c8..f592091 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
@@ -101,7 +101,7 @@ public class SecuredUpload {
private static final String MODULE = SecuredUpload.class.getName();
private static final List<String> DENIEDFILEEXTENSIONS = deniedFileExtensions();
private static final List<String> DENIEDWEBSHELLTOKENS = deniedWebShellTokens();
- private static final Integer maxLineLength = UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000);
+ private static final Integer MAXLINELENGTH = UtilProperties.getPropertyAsInteger("security", "maxLineLength", 10000);
public static boolean isValidText(String content, List<String> allowed) throws IOException {
return DENIEDWEBSHELLTOKENS.stream().allMatch(token -> isValid(content, token, allowed));
@@ -121,7 +121,7 @@ public class SecuredUpload {
// Check max line length, default 10000
if (!checkMaxLinesLength(fileToCheck)) {
- Debug.logError("For security reason lines over " + maxLineLength.toString() + " are not allowed", MODULE);
+ Debug.logError("For security reason lines over " + MAXLINELENGTH.toString() + " are not allowed", MODULE);
return false;
}
@@ -689,7 +689,7 @@ public class SecuredUpload {
File file = new File(fileToCheck);
List<String> lines = FileUtils.readLines(file, Charset.defaultCharset());
for (String line : lines) {
- if (line.length() > maxLineLength) {
+ if (line.length() > MAXLINELENGTH) {
return false;
}
}