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 54c6010 Improved: Fix some bugs Spotbugs reports (OFBIZ-12386)
54c6010 is described below
commit 54c6010c3b6d9d43f72667412903a29eb4f26ee6
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Thu Dec 9 19:23:30 2021 +0100
Improved: Fix some bugs Spotbugs reports (OFBIZ-12386)
In SecuredUpload::isValidFile fixes a possible null dereferencement
---
.../org/apache/ofbiz/security/SecuredUpload.java | 46 +++++++++++-----------
1 file changed, 24 insertions(+), 22 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 05f784d..8e395b5 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
@@ -114,34 +114,36 @@ public class SecuredUpload {
String imageServerUrl =
EntityUtilProperties.getPropertyValue("catalog", "image.management.url",
delegator);
Path p = Paths.get(fileToCheck);
- String fileName = p.getFileName().toString(); // The file name is the
farthest element from the root in the directory hierarchy.
boolean wrongFile = true;
+ if (p != null) {
+ String fileName = p.getFileName().toString(); // The file name is
the farthest element from the root in the directory hierarchy.
+ if
(DENIEDFILEEXTENSIONS.contains(FilenameUtils.getExtension(fileToCheck))) {
+ Debug.logError("This file extension is not allowed for
security reason", MODULE);
+ deleteBadFile(fileToCheck);
+ return false;
+ }
- if
(DENIEDFILEEXTENSIONS.contains(FilenameUtils.getExtension(fileToCheck))) {
- Debug.logError("This file extension is not allowed for security
reason", MODULE);
- deleteBadFile(fileToCheck);
- return false;
- }
-
- if (org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS) {
- if (fileToCheck.length() > 259) { // More about that:
https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
- Debug.logError("Uploaded file name too long", MODULE);
- } else if (p.toString().contains(imageServerUrl.replaceAll("/",
"\\\\"))) {
- if (fileName.matches("[a-zA-Z0-9-_ ()]{1,249}.[a-zA-Z0-9-_
]{1,10}")) { // "(" and ")" for duplicates files
+ if (org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS) {
+ // More about that:
https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
+ if (fileToCheck.length() > 259) {
+ Debug.logError("Uploaded file name too long", MODULE);
+ } else if
(p.toString().contains(imageServerUrl.replaceAll("/", "\\\\"))) {
+ if (fileName.matches("[a-zA-Z0-9-_ ()]{1,249}.[a-zA-Z0-9-_
]{1,10}")) { // "(" and ")" for duplicates files
+ wrongFile = false;
+ }
+ } else if (fileName.matches("[a-zA-Z0-9-_
]{1,249}.[a-zA-Z0-9-_ ]{1,10}")) {
wrongFile = false;
}
- } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,249}.[a-zA-Z0-9-_
]{1,10}")) {
- wrongFile = false;
- }
- } else { // Suppose a *nix system
- if (fileToCheck.length() > 4096) {
- Debug.logError("Uploaded file name too long", MODULE);
- } else if (p.toString().contains(imageServerUrl)) {
- if (fileName.matches("[a-zA-Z0-9-_ ()]{1,4086}.[a-zA-Z0-9-_
]{1,10}")) { // "(" and ")" for duplicates files
+ } else { // Suppose a *nix system
+ if (fileToCheck.length() > 4096) {
+ Debug.logError("Uploaded file name too long", MODULE);
+ } else if (p.toString().contains(imageServerUrl)) {
+ if (fileName.matches("[a-zA-Z0-9-_
()]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) { // "(" and ")" for duplicates files
+ wrongFile = false;
+ }
+ } else if (fileName.matches("[a-zA-Z0-9-_
]{1,4086}.[a-zA-Z0-9-_ ]{1,10}")) {
wrongFile = false;
}
- } else if (fileName.matches("[a-zA-Z0-9-_ ]{1,4086}.[a-zA-Z0-9-_
]{1,10}")) {
- wrongFile = false;
}
}
