OFBiz › OFBiz - Commits

[ofbiz-framework] branch release17.12 updated: Fixed: Secure the uploads (OFBIZ-12080)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

1 message

jleroux@apache.org

[ofbiz-framework] branch release17.12 updated: Fixed: Secure the uploads (OFBIZ-12080)

This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch release17.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

The following commit(s) were added to refs/heads/release17.12 by this push:
new f741bb2 Fixed: Secure the uploads (OFBIZ-12080)
f741bb2 is described below

commit f741bb22774ad5ee9ac2f805a838acfac4b27ecf
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Mon Nov 30 19:06:33 2020 +0100

Fixed: Secure the uploads (OFBIZ-12080)

2020/08/10 the OFBiz security team received a security report by Harshit Shukla
<[hidden email]>, roughly it was (quoting part of it to simplify):

<<I have identified a Remote Code Execution (RCE) Vulnerability. The reason
behind this RCE is lack of file extension check at
catalog/control/UploadCategoryImage?productCategoryId=CATALOG1_BEST_SELL&pload_file_type=category>>

Using this post-auth RCE in OFBiz demos, Harshit was able to get some AWS
credentials by uploading a webshell (based on [0]).
By security, it was then decided by the Infra and OFBiz security teams to shut
down the demos.

After discussing the elements reported with Mark J Cox (VP of ASF security team)
we in common decided that no CVE was necessary.

# Conflicts handled by hand in R18:
# applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
# applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java
# applications/product/groovyScripts/catalog/category/EditCategory.groovy
# applications/product/groovyScripts/catalog/config/EditProductConfigItemContent.groovy
# applications/product/groovyScripts/catalog/imagemanagement/ImageUpload.groovy
# applications/product/groovyScripts/catalog/imagemanagement/SetDefaultImage.groovy
# applications/product/groovyScripts/catalog/product/EditProductContent.groovy
# applications/product/src/main/java/org/apache/ofbiz/product/image/ScaleImage.java
# applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java
# applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/ImageManagementServices.java
# applications/product/src/main/java/org/apache/ofbiz/product/product/ProductServices.java
# build.gradle
# framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
# framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java
# framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
# framework/security/config/security.properties

# Conflicts handled by hand in R17:
# applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
# applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java
# applications/datamodel/data/seed/ContentSeedData.xml
# build.gradle
# framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java
---
.../apache/ofbiz/content/data/DataServices.java | 51 +-
.../datamodel/data/seed/ContentSeedData.xml | 11 +-
.../catalog/category/EditCategory.groovy | 11 +-
.../config/EditProductConfigItemContent.groovy | 11 +-
.../catalog/imagemanagement/ImageUpload.groovy | 11 +-
.../catalog/product/EditProductContent.groovy | 18 +-
.../org/apache/ofbiz/product/image/ScaleImage.java | 30 +-
.../ofbiz/product/imagemanagement/FrameImage.java | 11 +-
.../imagemanagement/ImageManagementServices.java | 29 +-
.../ofbiz/product/product/ProductServices.java | 25 +-
.../shipment/FedexShipRequestTemplate.xml.ftl | 4 +-
.../FedexSubscriptionRequestTemplate.xml.ftl | 4 +-
build.gradle | 4 +
.../java/org/apache/ofbiz/base/util/FileUtil.java | 25 +
.../ofbiz/base/util/HttpRequestFileUpload.java | 37 +-
.../ofbiz/base/util/template/FreeMarkerWorker.java | 3 +-
framework/common/config/SecurityUiLabels.xml | 20 +
.../ofbiz/entity/model/ModelEntityChecker.java | 2 +-
framework/security/config/security.properties | 35 +-
.../org/apache/ofbiz/security/SecuredUpload.java | 548 +++++++++++++++++++++
.../ofbiz/service/ServiceSynchronization.java | 14 +-
21 files changed, 833 insertions(+), 71 deletions(-)

diff --git a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java
index 5000207..5b0bdce 100644
--- a/applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java
+++ b/applications/content/src/main/java/org/apache/ofbiz/content/data/DataServices.java
@@ -32,6 +32,7 @@ import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

+import org.apache.commons.imaging.ImageReadException;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.GeneralException;
import org.apache.ofbiz.base.util.UtilDateTime;
@@ -196,7 +197,7 @@ public class DataServices {
}

public static Map<String, Object> createFileMethod(DispatchContext dctx, Map<String, ? extends Object> context) {
- //GenericValue dataResource = (GenericValue) context.get("dataResource");
+ Delegator delegator = dctx.getDelegator();
String dataResourceTypeId = (String) context.get("dataResourceTypeId");
String objectInfo = (String) context.get("objectInfo");
ByteBuffer binData = (ByteBuffer) context.get("binData");
@@ -248,7 +249,11 @@ public class DataServices {
OutputStreamWriter out = new OutputStreamWriter(new FileOutputStream(file), UtilIO.getUtf8());
) {
out.write(textData);
- } catch (IOException e) {
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "Text", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedTextFileFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
+ } catch (IOException | ImageReadException e) {
Debug.logWarning(e, module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ContentUnableWriteCharacterDataToFile", UtilMisc.toMap("fileName", file.getAbsolutePath()), locale));
}
@@ -257,7 +262,13 @@ public class DataServices {
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.write(binData.array());
out.close();
- } catch (FileNotFoundException e) {
+ // Check if a webshell is not uploaded
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "All", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedFileIncludingSvgFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
+
+ } catch (FileNotFoundException | ImageReadException e) {
Debug.logError(e, module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName", file.getAbsolutePath()), locale));
} catch (IOException e) {
@@ -397,6 +408,7 @@ public class DataServices {
}

public static Map<String, Object> updateFileMethod(DispatchContext dctx, Map<String, ? extends Object> context) throws GenericServiceException {
+ Delegator delegator = dctx.getDelegator();
Map<String, Object> result = new HashMap<>();
Locale locale = (Locale) context.get("locale");
String dataResourceTypeId = (String) context.get("dataResourceTypeId");
@@ -437,7 +449,11 @@ public class DataServices {
OutputStreamWriter out = new OutputStreamWriter(new FileOutputStream(file),UtilIO.getUtf8());
) {
out.write(textData);
- } catch (IOException e) {
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "Text", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedTextFileFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
+ } catch (IOException | ImageReadException e) {
Debug.logWarning(e, module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ContentUnableWriteCharacterDataToFile", UtilMisc.toMap("fileName", file.getAbsolutePath()), locale));
}
@@ -447,7 +463,12 @@ public class DataServices {
out.setLength(binData.array().length);
out.write(binData.array());
out.close();
- } catch (FileNotFoundException e) {
+ // Check if a webshell is not uploaded
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "All", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedFileIncludingSvgFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
+ } catch (FileNotFoundException | ImageReadException e) {
Debug.logError(e, module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName", file.getAbsolutePath()), locale));
} catch (IOException e) {
@@ -577,12 +598,14 @@ public class DataServices {
}

public static Map<String, Object> createBinaryFileMethod(DispatchContext dctx, Map<String, ? extends Object> context) throws GenericServiceException {
+ Delegator delegator = dctx.getDelegator();
Map<String, Object> result = new HashMap<>();
GenericValue dataResource = (GenericValue) context.get("dataResource");
String dataResourceTypeId = (String) dataResource.get("dataResourceTypeId");
String objectInfo = (String) dataResource.get("objectInfo");
byte [] imageData = (byte []) context.get("imageData");
String rootDir = (String)context.get("rootDir");
+ Locale locale = (Locale) context.get("locale");
File file = null;
if (Debug.infoOn()) {
Debug.logInfo("in createBinaryFileMethod, dataResourceTypeId:" + dataResourceTypeId, module);
@@ -604,10 +627,15 @@ public class DataServices {
FileOutputStream out = new FileOutputStream(file);
) {
out.write(imageData);
+ // Check if a webshell is not uploaded
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "All", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedFileIncludingSvgFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
if (Debug.infoOn()) {
Debug.logInfo("in createBinaryFileMethod, length:" + file.length(), module);
}
- } catch (IOException e) {
+ } catch (IOException | ImageReadException e) {
Debug.logWarning(e, module);
throw new GenericServiceException(e.getMessage());
}
@@ -630,12 +658,14 @@ public class DataServices {
}

public static Map<String, Object> updateBinaryFileMethod(DispatchContext dctx, Map<String, ? extends Object> context) throws GenericServiceException {
+ Delegator delegator = dctx.getDelegator();
Map<String, Object> result = new HashMap<>();
GenericValue dataResource = (GenericValue) context.get("dataResource");
String dataResourceTypeId = (String) dataResource.get("dataResourceTypeId");
String objectInfo = (String) dataResource.get("objectInfo");
byte [] imageData = (byte []) context.get("imageData");
String rootDir = (String)context.get("rootDir");
+ Locale locale = (Locale) context.get("locale");
File file = null;
if (Debug.infoOn()) {
Debug.logInfo("in updateBinaryFileMethod, dataResourceTypeId:" + dataResourceTypeId, module);
@@ -660,11 +690,16 @@ public class DataServices {
FileOutputStream out = new FileOutputStream(file);
){
out.write(imageData);
+ // Check if a webshell is not uploaded
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(), "All", delegator)) {
+ String errorMessage = UtilProperties.getMessage("SecurityUiLabels", "SupportedFileIncludingSvgFormats", locale);
+ return ServiceUtil.returnError(errorMessage);
+ }
+
if (Debug.infoOn()) {
Debug.logInfo("in updateBinaryFileMethod, length:" + file.length(), module);
}
- out.close();
- } catch (IOException e) {
+ } catch (IOException | ImageReadException e) {
Debug.logWarning(e, module);
throw new GenericServiceException(e.getMessage());
}
diff --git a/applications/datamodel/data/seed/ContentSeedData.xml b/applications/datamodel/data/seed/ContentSeedData.xml
index 67d73d3..12106d6 100644
--- a/applications/datamodel/data/seed/ContentSeedData.xml
+++ b/applications/datamodel/data/seed/ContentSeedData.xml
@@ -407,7 +407,8 @@ under the License.
<MimeType mimeTypeId="audio/basic" description="Basic Audio"/>
<MimeType mimeTypeId="audio/mpeg" description="MPEG Audio"/>
<MimeType mimeTypeId="audio/x-ms-wax" description="WAX Audio"/>
- <MimeType mimeTypeId="audio/x-ms-wma" description="WMA Audio"/>
+ <MimeType mimeTypeId="audio/ogg" description="OGG Audio"/>
+ <MimeType mimeTypeId="audio/wav" description="WAV Audio"/>


<MimeType mimeTypeId="image/jpeg" description="JPEG/JPG Image"/>
@@ -415,6 +416,8 @@ under the License.
<MimeType mimeTypeId="image/gif" description="GIF Image"/>
<MimeType mimeTypeId="image/tiff" description="TIFF Image"/>
<MimeType mimeTypeId="image/png" description="PNG Image"/>
+ <MimeType mimeTypeId="image/svg+xml" description="Image SVG"/>
+


<MimeType mimeTypeId="message/external-body" description="External Body Message"/>
@@ -473,6 +476,7 @@ under the License.
<FileExtension fileExtensionId="tif" mimeTypeId="image/tiff"/>
<FileExtension fileExtensionId="tiff" mimeTypeId="image/tiff"/>
<FileExtension fileExtensionId="gif" mimeTypeId="image/gif"/>
+ <FileExtension fileExtensionId="svg" mimeTypeId="image/svg+xml"/>
<FileExtension fileExtensionId="png" mimeTypeId="image/png"/>
<FileExtension fileExtensionId="mp4" mimeTypeId="video/mp4"/>
<FileExtension fileExtensionId="m4v" mimeTypeId="video/mp4"/>
@@ -497,6 +501,11 @@ under the License.
<FileExtension fileExtensionId="wmx" mimeTypeId="video/x-ms-wmx"/>
<FileExtension fileExtensionId="zip" mimeTypeId="application/zip"/>
<FileExtension fileExtensionId="vcf" mimeTypeId="text/x-vcard"/>
+ <FileExtension fileExtensionId="wav" mimeTypeId="audio/wav"/>
+ <FileExtension fileExtensionId="mpeg" mimeTypeId="audio/mpeg"/>
+ <FileExtension fileExtensionId="ogg" mimeTypeId="audio/ogg"/>
+ <FileExtension fileExtensionId="dflt" mimeTypeId="application/octet-stream"/>
+



- <#-- FreeMarker template for Fedex FDXShipRequest -->
+ <#-- FTL emplate for Fedex FDXShipRequest -->

<FDXShipRequest xmlns:api="http://www.fedex.com/fsmapi" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="FDXShipRequest.xsd">
<RequestHeader>
@@ -121,4 +121,4 @@

</FDXShipRequest>

-</#compress>
\ No newline at end of file
+</#compress>
diff --git a/applications/product/template/shipment/FedexSubscriptionRequestTemplate.xml.ftl b/applications/product/template/shipment/FedexSubscriptionRequestTemplate.xml.ftl
index e33f456..6f29e99 100644
--- a/applications/product/template/shipment/FedexSubscriptionRequestTemplate.xml.ftl
+++ b/applications/product/template/shipment/FedexSubscriptionRequestTemplate.xml.ftl
@@ -20,7 +20,7 @@
under the License.
-->

- <#-- FreeMarker template for Fedex FDXSubscriptionRequest -->
+ <#-- FTL template for Fedex FDXSubscriptionRequest -->

<FDXSubscriptionRequest xmlns:api="http://www.fedex.com/fsmapi" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="FDXSubscriptionRequest.xsd">
<RequestHeader>
@@ -34,7 +34,7 @@
<FaxNumber>${FaxNumber?xml}</FaxNumber>
</#if>
<#if EMailAddress??>
- <#-- Freemarker has a problem with the E-MailAddress tag name, so the opening and closing tags need to be wrapped in the noparse directive. -->
+ <#-- Free-marker has a problem with the E-MailAddress tag name, so the opening and closing tags need to be wrapped in the noparse directive. -->
<#noparse><E-MailAddress></#noparse>${EMailAddress?xml}<#noparse></E-MailAddress></#noparse>
</#if>
</Contact>
diff --git a/build.gradle b/build.gradle
index 24ced15..ed6d6a4 100644
--- a/build.gradle
+++ b/build.gradle
@@ -158,6 +158,10 @@ dependencies {
compile 'oro:oro:2.0.8'
compile 'wsdl4j:wsdl4j:1.6.3'
compile 'org.jsoup:jsoup:1.11.2'
+ compile 'net.lingala.zip4j:zip4j:2.6.4'
+ compile 'org.apache.commons:commons-imaging:1.0-alpha2' // Alpha but OK, "Imaging was working and was used by a number of projects in production even before reaching its initial release as an Apache Commons component."
+ compile 'org.apache.tika:tika-core:1.24.1'
+ compile 'org.apache.xmlgraphics:batik:1.13'

// ofbiz unit-test compile libs
testCompile 'org.mockito:mockito-core:2.+'
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
index 7ce7472..2b32e9e 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/FileUtil.java
@@ -41,6 +41,9 @@ import java.util.Set;
import org.apache.commons.io.FileUtils;
import org.apache.ofbiz.base.location.ComponentLocationResolver;

+import net.lingala.zip4j.ZipFile;
+import net.lingala.zip4j.exception.ZipException;
+
/**
* File Utilities
*
@@ -414,4 +417,26 @@ public final class FileUtil {
return new File(filePath).toPath().normalize().toFile();
}

+ /**
+ * Unzip file structure of the given zipFile to specified outputFolder The Zip slip vulnerabilty is handled since version 1.3.3 of
+ * net.lingala.zip4j.ZipFile; unzipFileToFolder is not as reliable and does not handle passwords
+ * @param source
+ * @param destination
+ * @param password optional
+ * @return true if OK
+ */
+ public static boolean unZip(String source, String destination, String password) {
+ try {
+ if (password.isEmpty()) {
+ new ZipFile(source).extractAll(destination);
+ } else {
+ new ZipFile(source, password.toCharArray()).extractAll(destination);
+ }
+ } catch (ZipException e) {
+ Debug.logError("Error extracting [" + source + "] file to dir destination: " + destination, e.toString(), module);
+ return false;
+ }
+ return true;
+ }
+
}
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java
index 95fc1c3..c359c95 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/HttpRequestFileUpload.java
@@ -30,6 +30,8 @@ import java.util.Map;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;

+import org.apache.commons.imaging.ImageReadException;
+import org.apache.ofbiz.entity.Delegator;

/**
* HttpRequestFileUpload - Receive a file upload through an HttpServletRequest
@@ -112,19 +114,19 @@ public class HttpRequestFileUpload {
}
}

- public void doUpload(HttpServletRequest request) throws IOException {
+ public boolean doUpload(HttpServletRequest request, String fileType) throws IOException {
+ Delegator delegator = (Delegator) request.getAttribute("delegator");
ServletInputStream in = request.getInputStream();

String reqLengthString = request.getHeader("content-length");
-
Debug.logInfo("expect " + reqLengthString + " bytes.", module);
int requestLength = 0;

try {
requestLength = Integer.parseInt(reqLengthString);
- } catch (Exception e2) {
- e2.printStackTrace();
- return;
+ } catch (NumberFormatException e) {
+ Debug.logError(e, e.getMessage(), module);
+ return false;
}
byte[] line = new byte[BUFFER_SIZE];

@@ -133,7 +135,8 @@ public class HttpRequestFileUpload {
i = waitingReadLine(in, line, 0, BUFFER_SIZE, requestLength);
requestLength -= i;
if (i < 3) {
- return;
+ Debug.logError("Possibly a waitingReadLine error", module);
+ return false;
}
int boundaryLength = i - 2;

@@ -152,7 +155,7 @@ public class HttpRequestFileUpload {
if (newLine.indexOf("filename=\"") != -1) {
setFilename(new String(line, 0, i - 2, UtilIO.getUtf8()));
if (filename == null) {
- return;
+ return false;
}
// this is the file content
i = waitingReadLine(in, line, 0, BUFFER_SIZE, requestLength);
@@ -190,8 +193,8 @@ public class HttpRequestFileUpload {
}

}
- try (
- FileOutputStream fos = new FileOutputStream(savePath + filenameToUse);) {
+ String fileTocheck = savePath + filenameToUse;
+ try (FileOutputStream fos = new FileOutputStream(fileTocheck);) {
boolean bail = (new String(line, 0, i, UtilIO.getUtf8()).startsWith(boundary));
boolean oneByteLine = (i == 1); // handle one-byte lines

@@ -231,11 +234,13 @@ public class HttpRequestFileUpload {
}
}
fos.flush();
- fos.close();
- } catch (RuntimeException e) {
- throw e;
- } catch (Exception e) {
+
+ if (!org.apache.ofbiz.security.SecuredUpload.isValidFile(fileTocheck, fileType, delegator)) {
+ return false;
+ }
+ } catch (ImageReadException e) {
Debug.logError(e, module);
+ return false;
}
} else {
// this is a field
@@ -259,7 +264,7 @@ public class HttpRequestFileUpload {
i = waitingReadLine(in, line, 0, BUFFER_SIZE, requestLength);
requestLength -= i;
if ((i == boundaryLength + 2 || i == boundaryLength + 4) // + 4 is eof
- && (new String(line, 0, i).startsWith(boundary))) {
+ && (new String(line, 0, i).startsWith(boundary))) {
fieldValue.append(newLine.substring(0, newLine.length() - 2));
} else {
fieldValue.append(newLine);
@@ -275,6 +280,7 @@ public class HttpRequestFileUpload {
}

} // end while
+ return true;
}

// reads a line, waiting if there is nothing available and reqLen > 0
@@ -295,7 +301,7 @@ public class HttpRequestFileUpload {
while (endMS > (new Date().getTime())) {
try {
wait(WAIT_INTERVAL);
- } catch (Exception e3) {
+ } catch (InterruptedException e) {
Debug.logInfo(".", module);
}
}
@@ -303,4 +309,5 @@ public class HttpRequestFileUpload {
}
return i;
}
+
}
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
index ffd16b8..f6b7222 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
@@ -116,8 +116,7 @@ public final class FreeMarkerWorker {
} catch (TemplateException e) {
Debug.logError("Unable to set date/time and number formats in FreeMarker: " + e, module);
}
- String templateClassResolver = UtilProperties.getPropertyValue("security", "templateClassResolver",
- "SAFER_RESOLVER");
+ String templateClassResolver = UtilProperties.getPropertyValue("security", "templateClassResolver", "SAFER_RESOLVER");
switch (templateClassResolver) {
case "UNRESTRICTED_RESOLVER":
newConfig.setNewBuiltinClassResolver(TemplateClassResolver.UNRESTRICTED_RESOLVER);
diff --git a/framework/common/config/SecurityUiLabels.xml b/framework/common/config/SecurityUiLabels.xml
index b4b6116..d80d292 100644
--- a/framework/common/config/SecurityUiLabels.xml
+++ b/framework/common/config/SecurityUiLabels.xml
@@ -774,6 +774,26 @@
<value xml:lang="zh">SecurityViewPermissionError 你没有权限浏览本页面。 (需要"SECURITY_VIEW" 或 "SECURITY_ADMIN")</value>
<value xml:lang="zh-TW">SecurityViewPermissionError 你沒有權限檢視本頁面. (需要"SECURITY_VIEW" 或 "SECURITY_ADMIN")</value>
</property>
+ <property key="SupportedFileIncludingSvgFormats">
+ <value xml:lang="en">For security reason only valid files of supported image formats (GIF, JPEG, PNG, TIFF), SVG, PDF, and ZIP or text files with safe contents are accepted.</value>
+ <value xml:lang="fr">Pour des raisons de sécurité, seuls les fichiers valides de formats d'image pris en charge (GIF, JPEG, PNG, TIFF), les fichiers SVG, PDF, et les fichiers ZIP ou texte aux contenus sûrs sont acceptés.</value>
+ </property>
+ <property key="SupportedFileFormats">
+ <value xml:lang="en">For security reason only valid files of supported image formats (GIF, JPEG, PNG, TIFF), PDF or text files with safe contents are accepted.</value>
+ <value xml:lang="fr">Pour des raisons de sécurité, seuls les fichiers valides de formats d'image pris en charge (GIF, JPEG, PNG, TIFF), les fichiers PDF ou les fichiers texte aux contenus sûrs sont acceptés.</value>
+ </property>
+ <property key="SupportedImageFormatseIncludingSvg">
+ <value xml:lang="en">For security reason only valid files of supported image formats (GIF, JPEG, PNG, TIFF), or SVG format are accepted.</value>
+ <value xml:lang="fr">Pour des raisons de sécurité, seuls les fichiers valides des formats d'image pris en charge (GIF, JPEG, PNG, TIFF) ou au format SVG sont acceptés.</value>
+ </property>
+ <property key="SupportedImageFormats">
+ <value xml:lang="en">For security reason only valid files of supported image formats (GIF, JPEG, PNG, TIFF) are accepted.</value>
+ <value xml:lang="fr">Pour des raisons de sécurité, seuls les fichiers valides des formats d'image pris en charge (GIF, JPEG, PNG, TIFF) sont acceptés.</value>
+ </property>
+ <property key="SupportedTextFileFormats">
+ <value xml:lang="en">For security reason only text files with safe contents are accepted.</value>
+ <value xml:lang="fr">Pour des raisons de sécurité, seuls les fichiers texte dont les contenus sont sûrs sont acceptés.</value>
+ </property>
<property key="UserLogin">
<value xml:lang="de">Benutzeranmeldung</value>
<value xml:lang="en">User Login</value>
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntityChecker.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntityChecker.java
index fb53f91..56e7412 100644
--- a/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntityChecker.java
+++ b/framework/entity/src/main/java/org/apache/ofbiz/entity/model/ModelEntityChecker.java
@@ -393,7 +393,7 @@ public class ModelEntityChecker {
"MESSAGESCROLL", "MFETCH", "MIDDLEINT", "MIN", "MIN_ROWS",
"MINRECLEN", "MINRETURNUNTIL", "MINUS", "MINUTE", "MINUTE_SECOND",
"MIRROREXIT", "MISLABEL", "MISSING", "MIXED", "MOD", "MODE",
- "MODIFIES", "MODIFY", "MODIFYREVOKEUPDATE", "MODULE", "MONEY",
+ "MODIFIES", "MODIFY", "MODIFYREVOKEUPDATE", "module", "MONEY",
"MONITOR", "MONTH", "MONTHNAME", "MOVE", "MULTI", "MYISAM",

"NAME", "NAMES", "NATIONAL", "NATURAL", "NATURALN", "NCHAR",
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index a5f80a5..5f4cc67 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -143,9 +143,38 @@ host-headers-allowed=localhost,127.0.0.1,demo-trunk.ofbiz.apache.org,demo-stable
SameSiteCookieAttribute=

# -- Freemarker TemplateClassResolver option, see OFBIZ-11709.
-# -- By default OFBiz uses the SAFER_RESOLVER because OOTB it does not use any of the Freemarker classes
-# -- that SAFER_RESOLVER prevents: ObjectConstructor, Execute and JythonRuntime.
+# -- By default OFBiz uses the SAFER_RESOLVER because OOTB it does not use any of the Freemarker classes
+# -- that SAFER_RESOLVER prevents: ObjectConstructor, Execute and JythonRuntime.
# -- If you need to use one to these classes you need to change the TemplateClassResolver
-# -- to UNRESTRICTED_RESOLVER and look at MemberAccessPolicy. In any cases better read
+# -- to UNRESTRICTED_RESOLVER and look at MemberAccessPolicy. In any cases better read
# -- https://freemarker.apache.org/docs/app_faq.html#faq_template_uploading_security
templateClassResolver=
+
+
+#-- UPLOAD: supported file formats are *safe* PNG, GIF, TIFF, JPEG, PDF and ZIP
+#--
+#-- No proprietary file formats (Excel, Word, etc.) are handled OOTB.
+#-- They can be handled by custom projects using https://github.com/righettod/document-upload-protection:
+#-- https://github.com/OWASP/www-projectchapter-example/blob/master/cheatsheets/Protect_FileUpload_Against_Malicious_File.md
+#-- Also Tika is an option, but you have to check Tika code, to be sure it's secure enough (ie don't use only metadata)
+#--
+#-- Apache Commons Imaging is used for images.
+#-- For supported formats see https://commons.apache.org/proper/commons-imaging/formatsupport.html
+#-- Notably https://commons.apache.org/proper/commons-imaging/formatsupport.html#Metadata_Format_Support
+#-- OOTB OFBiz only supports PNG, GIF, TIFF and JPEG, it's a breeze to extend using more:
+#-- commonsImagingSupportedFormats=BMP,GIF,JPEG/JFIF,ICNS,ICO/CUR,PCX/DCX,PNM/PGM/PBM/PPM/PAMPortablePixmap,PNG,PSD/Photoshop,RGBE/RadianceHDR,TIFF,WBMP,XBM,XPM
+#-- You should then modify SupportedImageFormats label.
+#--
+#-- If you want to get more image formats then use imageJ:
+#-- For imagejSupportedFormats see https://imagejdocu.tudor.lu/faq/general/which_file_formats_are_supported_by_imagej. NOTE: plugins support is important here
+#-- imagejSupportedFormats=TIFF(.tiff,.tif),JPEG(.jpeg,.jpg),BMP(.bmp),FITS(.fits),PGM(.pgm),PPM(.ppm),PBM(.pbm),GIF(.gif),AnimatedGIF(.gif),PNG(.png),DICOM(.dic,.dcm,.dicom),PICT(.pict,.pic,.pct),PSD(.psd),TGA(.tga),ICO(.ico),CUR(.cur),Sunraster(.sun),XBM(.xbm),XPM(.xpm),PCX(.pcx),ANALYZE,NIfTi,AHF(.ahf),SPE(.spe),PIC(.pic),LeicaTIFF(.tiff,.lei),Quicktime(.pic,.mov),AVI(.avi),PDS(.pds),LSM(.lsm),RAW,ISAC,FluoViewTIFF(.tiff),FluoviewFV1000OIB(.oib),FluoviewFV1000OIF(.oif,.tif,-ro.pty,.lu [...]
+#--
+#-- PDFBox and PDFReader are used for PDF files
+#--
+#-- For text files, the philosophy is we can't presume of all possible text contents used for attacks with payloads
+#-- At least there is an easy way to prevent them in SecuredUpload::isValidTextFile
+#--
+#-- The upload vulnerability is only a post-auth (needs a credential with suitable permissions),
+#-- people may like to allow more than what is allowed OOTB
+#-- As it name says, allowAllUploads opens all possibilities
+allowAllUploads=
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
new file mode 100644
index 0000000..3fdae3d
--- /dev/null
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -0,0 +1,548 @@
+
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *******************************************************************************/
+package org.apache.ofbiz.security;
+
+import java.awt.Graphics;
+import java.awt.Image;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.ByteBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.imageio.ImageIO;
+import javax.imageio.ImageReader;
+import javax.imageio.stream.ImageInputStream;
+
+import org.apache.batik.dom.svg.SAXSVGDocumentFactory;
+import org.apache.batik.util.XMLResourceDescriptor;
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.formats.gif.GifImageParser;
+import org.apache.commons.imaging.formats.jpeg.JpegImageParser;
+import org.apache.commons.imaging.formats.png.PngImageParser;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.io.FileUtils;
+import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.FileUtil;
+import org.apache.ofbiz.entity.Delegator;
+import org.apache.ofbiz.entity.util.EntityUtilProperties;
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.apache.pdfbox.pdmodel.PDDocumentNameDictionary;
+import org.apache.pdfbox.pdmodel.PDEmbeddedFilesNameTreeNode;
+import org.apache.tika.Tika;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TikaCoreProperties;
+import org.apache.tika.parser.AutoDetectParser;
+import org.apache.tika.parser.ParseContext;
+import org.apache.tika.parser.Parser;
+import org.apache.tika.parser.RecursiveParserWrapper;
+import org.apache.tika.sax.BasicContentHandlerFactory;
+import org.apache.tika.sax.ContentHandlerFactory;
+import org.apache.tika.sax.RecursiveParserWrapperHandler;
+import org.xml.sax.SAXException;
+
+import com.lowagie.text.pdf.PdfReader;
+
+public class SecuredUpload {
+
+ // This can be helpful:
+ // https://en.wikipedia.org/wiki/File_format
+ // https://en.wikipedia.org/wiki/List_of_file_signatures
+ // See also information in security.properties:
+ // Line #-- UPLOAD: supported file formats are *safe* PNG, GIF, TIFF, JPEG, PDF and ZIP
+
+ private static final String module = SecuredUpload.class.getName();
+
+ /**
+ * @param fileTocheck
+ * @param fileType
+ * @return true if the file is valid
+ * @throws IOException
+ * @throws ImageReadException
+ */
+ public static boolean isValidFile(String fileTocheck, String fileType, Delegator delegator) throws IOException, ImageReadException {
+
+ if (("true".equalsIgnoreCase(EntityUtilProperties.getPropertyValue("security", "allowAllUploads", delegator)))) {
+ return true;
+ }
+
+ if (isExecutable(fileTocheck)) {
+ return false;
+ }
+
+ switch (fileType) {
+ case "Image":
+ if (isValidImageFile(fileTocheck)) {
+ return true;
+ }
+ break;
+
+ case "ImageAndSvg":
+ if (isValidImageIncludingSvgFile(fileTocheck)) {
+ return true;
+ }
+ break;
+
+ case "PDF":
+ if (isValidPdfFile(fileTocheck)) {
+ return true;
+ }
+ break;
+
+ case "Compressed":
+ if (isValidCompressedFile(fileTocheck, delegator)) {
+ return true;
+ }
+ break;
+
+ case "AllButCompressed":
+ if (isValidTextFile(fileTocheck)
+ || isValidImageIncludingSvgFile(fileTocheck)
+ || isValidPdfFile(fileTocheck)) {
+ return true;
+ }
+ break;
+
+ case "Text":
+ // The philosophy for isValidTextFile() is that
+ // we can't presume of all possible text contents used for attacks with payloads
+ // At least there is an easy way to prevent them in isValidTextFile
+ if (isValidTextFile(fileTocheck)) {
+ return true;
+ }
+ break;
+
+ // case "Audio": TODO if needed
+ // break;
+ // case "Video": TODO if needed
+ // break;
+
+ default: // All
+ if (isValidTextFile(fileTocheck)
+ || isValidImageIncludingSvgFile(fileTocheck)
+ || isValidCompressedFile(fileTocheck, delegator)
+ || isValidPdfFile(fileTocheck)) {
+ return true;
+ }
+ break;
+ }
+ Debug.logError("File :" + fileTocheck + ", can't be uploaded for security reason", module);
+ File badFile = new File(fileTocheck);
+ if (!badFile.delete()) {
+ Debug.logError("File :" + fileTocheck + ", couldn't be deleted", module);
+ }
+ return false;
+ }
+
+ /**
+ * Is it a supported image format?
+ * @param fileName
+ * @return true if it's a valid image file
+ * @throws IOException ImageReadException
+ */
+ private static boolean isValidImageFile(String fileName) throws ImageReadException, IOException {
+ Path filePath = Paths.get(fileName);
+ byte[] bytesFromFile = Files.readAllBytes(filePath);
+ ImageFormat imageFormat = Imaging.guessFormat(bytesFromFile);
+ return imageFormat.equals(ImageFormats.PNG)
+ || imageFormat.equals(ImageFormats.GIF)
+ || imageFormat.equals(ImageFormats.TIFF)
+ || imageFormat.equals(ImageFormats.JPEG)
+ && imageMadeSafe(fileName);
+ }
+
+ /**
+ * Implementation based on https://github.com/righettod/document-upload-protection sanitizer for Image file. See
+ * https://github.com/righettod/document-upload-protection/blob/master/src/main/java/eu/righettod/poc/sanitizer/ImageDocumentSanitizerImpl.java
+ * Uses Java built-in API in complement of Apache Commons Imaging for format not supported by the built-in API. See
+ * http://commons.apache.org/proper/commons-imaging/ and http://commons.apache.org/proper/commons-imaging/formatsupport.html
+ */
+ private static boolean imageMadeSafe(String fileName) {
+ File file = new File(fileName);
+ boolean safeState = false;
+ boolean fallbackOnApacheCommonsImaging;
+ try {
+ if ((file != null) && file.exists() && file.canRead() && file.canWrite()) {
+ // Get the image format
+ String formatName;
+ ImageInputStream iis = ImageIO.createImageInputStream(file);
+ Iterator<ImageReader> imageReaderIterator = ImageIO.getImageReaders(iis);
+ // If there not ImageReader instance found so it's means that the current format is not supported by the Java built-in API
+ if (!imageReaderIterator.hasNext()) {
+ ImageInfo imageInfo = Imaging.getImageInfo(file);
+ if (imageInfo != null && imageInfo.getFormat() != null && imageInfo.getFormat().getName() != null) {
+ formatName = imageInfo.getFormat().getName();
+ fallbackOnApacheCommonsImaging = true;
+ } else {
+ throw new IOException("Format of the original image " + fileName + " is not supported for read operation !");
+ }
+ } else {
+ ImageReader reader = imageReaderIterator.next();
+ formatName = reader.getFormatName();
+ fallbackOnApacheCommonsImaging = false;
+ iis.close(); // This was not correctly handled in the document-upload-protection example, and I did not spot it :/
+ }
+
+ // Load the image
+ BufferedImage originalImage;
+ if (!fallbackOnApacheCommonsImaging) {
+ originalImage = ImageIO.read(file);
+ } else {
+ originalImage = Imaging.getBufferedImage(file);
+ }
+
+ // Check that image has been successfully loaded
+ if (originalImage == null) {
+ throw new IOException("Cannot load the original image " + fileName + "!");
+ }
+
+ // Get current Width and Height of the image
+ int originalWidth = originalImage.getWidth(null);
+ int originalHeight = originalImage.getHeight(null);
+
+ // Resize the image by removing 1px on Width and Height
+ Image resizedImage = originalImage.getScaledInstance(originalWidth - 1, originalHeight - 1, Image.SCALE_SMOOTH);
+
+ // Resize the resized image by adding 1px on Width and Height - In fact set image to is initial size
+ Image initialSizedImage = resizedImage.getScaledInstance(originalWidth, originalHeight, Image.SCALE_SMOOTH);
+
+ // Save image by overwriting the provided source file content
+ BufferedImage sanitizedImage = new BufferedImage(initialSizedImage.getWidth(null), initialSizedImage.getHeight(null),
+ BufferedImage.TYPE_INT_RGB);
+ Graphics bg = sanitizedImage.getGraphics();
+ bg.drawImage(initialSizedImage, 0, 0, null);
+ bg.dispose();
+ OutputStream fos = Files.newOutputStream(file.toPath(), StandardOpenOption.WRITE);
+ if (!fallbackOnApacheCommonsImaging) {
+ ImageIO.write(sanitizedImage, formatName, fos);
+ } else {
+ ImageParser imageParser;
+ // Handle only formats for which Apache Commons Imaging can successfully write (YES in Write column of the reference link)
+ // the image format. See reference link in the class header
+ switch (formatName) {
+ case "TIFF": {
+ imageParser = new TiffImageParser();
+ break;
+ }
+ case "GIF": {
+ imageParser = new GifImageParser();
+ break;
+ }
+ case "PNG": {
+ imageParser = new PngImageParser();
+ break;
+ }
+ case "JPEG": {
+ imageParser = new JpegImageParser();
+ break;
+ }
+ default: {
+ throw new IOException("Format of the original image " + fileName + " is not supported for write operation !");
+ }
+ }
+ imageParser.writeImage(sanitizedImage, fos, new HashMap<>());
+ }
+ // Set state flag
+ fos.close(); // This was not correctly handled in the document-upload-protection example, and I did not spot it :/
+ safeState = true;
+ }
+ } catch (IOException | ImageReadException | ImageWriteException e) {
+ safeState = false;
+ Debug.logWarning(e, "Error during Image file " + fileName + " processing !", module);
+ }
+ return safeState;
+ }
+
+ /**
+ * Is it a supported image format, including SVG?
+ * @param fileName
+ * @return true if it's a valid image file
+ * @throws IOException ImageReadException
+ */
+ private static boolean isValidImageIncludingSvgFile(String fileName) throws ImageReadException, IOException {
+ Path filePath = Paths.get(fileName);
+ byte[] bytesFromFile = Files.readAllBytes(filePath);
+ ImageFormat imageFormat = Imaging.guessFormat(bytesFromFile);
+ return imageFormat.equals(ImageFormats.PNG)
+ || imageFormat.equals(ImageFormats.GIF)
+ || imageFormat.equals(ImageFormats.TIFF)
+ || imageFormat.equals(ImageFormats.JPEG)
+ || isValidSvgFile(fileName);
+ }
+
+ /**
+ * Is it an SVG file?
+ * @param fileName
+ * @return true if it's a valid SVG file
+ * @throws IOException
+ */
+ private static boolean isValidSvgFile(String fileName) throws IOException {
+ Path filePath = Paths.get(fileName);
+ String parser = XMLResourceDescriptor.getXMLParserClassName();
+ SAXSVGDocumentFactory f = new SAXSVGDocumentFactory(parser);
+ try {
+ f.createDocument(filePath.toUri().toString());
+ } catch (IOException e) {
+ return false;
+ }
+ return isValidTextFile(fileName);
+ }
+
+ /**
+ * @param fileName
+ * @return true if it's a safe PDF file: is PDF and does not contains embedded files
+ * @throws IOException If there is an error parsing the document
+ */
+ private static boolean isValidPdfFile(String fileName) throws IOException {
+ File file = new File(fileName);
+ boolean safeState = false;
+ try {
+ if ((file != null) && file.exists()) {
+ // Load stream in PDF parser
+ // If the stream is not a PDF then exception will be thrown and safe state will be set to FALSE
+ PdfReader reader = new PdfReader(file.getAbsolutePath());
+ // Check 1: detect if the document contains any JavaScript code
+ String jsCode = reader.getJavaScript();
+ if (jsCode == null) {
+ // OK no JS code, pass to check 2: detect if the document has any embedded files
+ PDEmbeddedFilesNameTreeNode efTree = null;
+ try (PDDocument pdDocument = PDDocument.load(file)) {
+ PDDocumentNameDictionary names = new PDDocumentNameDictionary(pdDocument.getDocumentCatalog());
+ efTree = names.getEmbeddedFiles();
+ }
+ safeState = efTree == null;
+ }
+ }
+ } catch (Exception e) {
+ safeState = false;
+ Debug.logError(e, "for security reason the PDF file " + file.getAbsolutePath() + "can't be uploaded !", module);
+ }
+ return safeState;
+ }
+
+ private static boolean isExecutable(String fileName) throws IOException {
+ String mimeType = getMimeTypeFromFileName(fileName);
+ // Check for Windows executable. Neglect .bat and .ps1: https://s.apache.org/c8sim
+ if ("application/x-msdownload".equals(mimeType) || "application/x-ms-installer".equals(mimeType)) {
+ Debug.logError("The file" + fileName + " is a Windows executable, for security reason it's not accepted :", module);
+ return true;
+ }
+ // Check for ELF (Linux) and scripts
+ if ("application/x-elf".equals(mimeType)
+ || "application/x-sh".equals(mimeType)
+ || "application/text/x-perl".equals(mimeType)
+ || "application/text/x-python".equals(mimeType)) {
+ Debug.logError("The file" + fileName + " is a Linux executable, for security reason it's not accepted :", module);
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Check if the compressed file is valid Does not handle compressed files in sub folders of compressed files. Handles only ZIP files, if you need
+ * bzip, rar, tar or/and 7z file formats they can be handled by Apache commons-compress: Types based on
+ * https://developer.mozilla.org/fr/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types For code explanations see
+ * http://commons.apache.org/proper/commons-compress/examples.html
+ * @param fileName
+ * @return true if it's a valid compressed file
+ * @throws IOException ImageReadException
+ */
+ private static boolean isValidCompressedFile(String fileName, Delegator delegator) throws IOException, ImageReadException {
+ String mimeType = getMimeTypeFromFileName(fileName);
+ // I planned to handle more formats but did only ZIP
+ // The code can be extended based on that
+ // if ("application/octet-stream".equals(mimeType)
+ // || "application/x-bzip".equals(mimeType)
+ // || "application/x-bzip2".equals(mimeType)
+ // || "application/java-archive".equals(mimeType)
+ // || "application/x-rar-compressed".equals(mimeType)
+ // || "application/x-tar".equals(mimeType)
+ // || "application/zip".equals(mimeType)
+ // || "application/x-zip-compressed".equals(mimeType)
+ // || "multipart/x-zip".equals(mimeType)
+ // || "application/x-7z-compressed".equals(mimeType)) {
+
+ // Handles only Zip format OOTB
+ File fileToCheck = new File(fileName);
+ String folderName = fileToCheck.getParentFile().toString() + File.separator + UUID.randomUUID();
+ if ("application/octet-stream".equals(mimeType)
+ || "application/java-archive".equals(mimeType)
+ || "application/zip".equals(mimeType)
+ || "application/x-zip-compressed".equals(mimeType)
+ || "multipart/x-zip".equals(mimeType)) {
+ if (!FileUtil.unZip(fileName, folderName, "")) {
+ return false;
+ } else {
+ // Keep it like that to allow to spot other file types which could be included...
+ // try {
+ // recursiveParserWrapper(fileName);
+ // } catch (SAXException | TikaException e) {
+ // // TODO Auto-generated catch block
+ // e.printStackTrace();
+ // }
+ // Recursive method to check inside directories
+ return isValidDirectoryInCompressedFile(folderName, delegator);
+ }
+ }
+ return false;
+ }
+
+ /*
+ * According to http://tika.apache.org/1.24.1/detection.html#The_default_Tika_Detector The simplest way to detect is through the Tika Facade
+ * class, which provides methods to detect based on File, InputStream, InputStream and Filename, Filename or a few others. It works best with a
+ * File or TikaInputStream.
+ * @param fileName
+ * @return true if the file is valid
+ */
+ private static String getMimeTypeFromFileName(String fileName) throws IOException {
+ File file = new File(fileName);
+ Tika tika = new Tika();
+ return tika.detect(file);
+ }
+
+ private static boolean isValidDirectoryInCompressedFile(String folderName, Delegator delegator) throws IOException, ImageReadException {
+ File folder = new File(folderName);
+ Collection<File> files = FileUtils.listFiles(folder, null, true);
+ for (File f : files) {
+ if (f.isDirectory()) {
+ Collection<File> dirInside = FileUtils.listFiles(f, null, true);
+ for (File insideFile : dirInside) {
+ if (!isValidDirectoryInCompressedFile(insideFile.getAbsolutePath(), delegator)) {
+ FileUtils.deleteDirectory(folder);
+ return false;
+ }
+ }
+ } else if (!isValidFile(f.getAbsolutePath(), "AllButCompressed", delegator)) {
+ FileUtils.deleteDirectory(folder);
+ return false;
+ }
+ }
+ FileUtils.deleteDirectory(folder);
+ return true;
+ }
+
+ /**
+ * For documents that may contain embedded documents, it might be helpful to create list of metadata objects, one for the container document and
+ * one for each embedded document. This allows easy access to both the extracted content and the metadata of each embedded document. Note that
+ * many document formats can contain embedded documents, including traditional container formats -- zip, tar and others -- but also common office
+ * document formats including: MSWord, MSExcel, MSPowerPoint, RTF, PDF, MSG and several others.
+ * <p>
+ * The "content" format is determined by the ContentHandlerFactory, and the content is stored in
+ * {@link org.apache.tika.parser.RecursiveParserWrapper#TIKA_CONTENT}
+ * <p>
+ * The drawback to the RecursiveParserWrapper is that it caches metadata and contents in memory. This should not be used on files whose contents
+ * are too big to be handled in memory.
+ * @return a list of metadata object, one each for the container file and each embedded file
+ * @throws IOException
+ * @throws SAXException
+ * @throws TikaException
+ */
+ // This can turn to be useful, so I let it there...
+ // Inspired by https://cwiki.apache.org/confluence/display/tika/RecursiveMetadata
+ // And https://stackoverflow.com/questions/62132310/apache-tika-exctract-file-names-and-mime-types-from-archive
+ @SuppressWarnings("unused")
+ private static Set<String> recursiveParserWrapper(String fileName) throws IOException, SAXException, TikaException {
+ File file = new File(fileName);
+ Parser p = new AutoDetectParser();
+ ContentHandlerFactory factory = new BasicContentHandlerFactory(BasicContentHandlerFactory.HANDLER_TYPE.IGNORE, -1);
+ RecursiveParserWrapper wrapper = new RecursiveParserWrapper(p);
+ Metadata metadata = new Metadata();
+ metadata.set(TikaCoreProperties.ORIGINAL_RESOURCE_NAME, file.getName());
+ ParseContext context = new ParseContext();
+ RecursiveParserWrapperHandler handler = new RecursiveParserWrapperHandler(factory, -1);
+ try (InputStream stream = new FileInputStream(file)) {
+ wrapper.parse(stream, handler, metadata, context);
+ }
+ List<Metadata> metadatas = handler.getMetadataList();
+ Set<String> mimeTypes = new HashSet<>();
+ for (Metadata metadata1 : metadatas) {
+ mimeTypes.add(metadata1.get(Metadata.CONTENT_TYPE));
+ }
+ return mimeTypes;
+ }
+
+ /**
+ * Does this text file contains a Freemarker Server-Side Template Injection (SSTI) using freemarker.template.utility.Execute? Etc.
+ * @param fileName must be an UTF-8 encoded text file
+ * @return true if the text file does not contains a Freemarker SSTI
+ * @throws IOException
+ */
+ private static boolean isValidTextFile(String fileName) throws IOException {
+ Path filePath = Paths.get(fileName);
+ byte[] bytesFromFile = Files.readAllBytes(filePath);
+ try {
+ Charset.availableCharsets().get("UTF-8").newDecoder().decode(ByteBuffer.wrap(bytesFromFile));
+ } catch (CharacterCodingException e) {
+ return false;
+ }
+ String content = new String(bytesFromFile);
+ return !(content.toLowerCase().contains("freemarker") // Should be OK, should not be used in Freemarker templates, not part of the syntax.
+ // Else "template.utility.Execute" is a good replacement but not as much catching, who
+ // knows...
+ || content.toLowerCase().contains("import=\"java")
+ || content.toLowerCase().contains("Runtime.getRuntime().exec(")
+ || content.toLowerCase().contains("<%@ page")
+ || content.toLowerCase().contains("<script")
+ || content.toLowerCase().contains("<body>")
+ || content.toLowerCase().contains("<form")
+ || content.toLowerCase().contains("php")
+ || content.toLowerCase().contains("javascript")
+ || content.toLowerCase().contains("%eval")
+ || content.toLowerCase().contains("@eval")
+ || content.toLowerCase().contains("import os") // Python
+ || content.toLowerCase().contains("passthru")
+ || content.toLowerCase().contains("exec")
+ || content.toLowerCase().contains("shell_exec")
+ || content.toLowerCase().contains("assert")
+ || content.toLowerCase().contains("str_rot13")
+ || content.toLowerCase().contains("system")
+ || content.toLowerCase().contains("phpinfo")
+ || content.toLowerCase().contains("base64_decode")
+ || content.toLowerCase().contains("chmod")
+ || content.toLowerCase().contains("mkdir")
+ || content.toLowerCase().contains("fopen")
+ || content.toLowerCase().contains("fclose")
+ || content.toLowerCase().contains("readfile"));
+ // TODO.... to be continued with known webshell contents... a complete allow list is impossible anyway
+ // eg: https://www.acunetix.com/blog/articles/detection-prevention-introduction-web-shells-part-5/
+ }
+}
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/ServiceSynchronization.java b/framework/service/src/main/java/org/apache/ofbiz/service/ServiceSynchronization.java
index 484cb46..a3cb15c 100644
--- a/framework/service/src/main/java/org/apache/ofbiz/service/ServiceSynchronization.java
+++ b/framework/service/src/main/java/org/apache/ofbiz/service/ServiceSynchronization.java
@@ -45,7 +45,7 @@ import org.apache.ofbiz.entity.transaction.TransactionUtil;
*/
public class ServiceSynchronization implements Synchronization {

- public static final String MODULE = ServiceSynchronization.class.getName();
+ public static final String module = ServiceSynchronization.class.getName();

private static Map<Transaction, ServiceSynchronization> syncingleton = new WeakHashMap<>();
private List<ServiceExecution> services = new ArrayList<>();
@@ -140,18 +140,18 @@ public class ServiceSynchronization implements Synchronization {
// set the userLogin object
thisContext.put("userLogin", ServiceUtil.getUserLogin(dctx, thisContext, runAsUser));
if (async) {
- Debug.logInfo(msgPrefix + "Invoking [" + serviceName + "] via runAsync", MODULE);
+ Debug.logInfo(msgPrefix + "Invoking [" + serviceName + "] via runAsync", module);
dctx.getDispatcher().runAsync(serviceName, thisContext, persist);
} else {
- Debug.logInfo(msgPrefix + "Invoking [" + serviceName + "] via runSyncIgnore", MODULE);
+ Debug.logInfo(msgPrefix + "Invoking [" + serviceName + "] via runSyncIgnore", module);
dctx.getDispatcher().runSyncIgnore(serviceName, thisContext);
}
} catch (Throwable t) {
- Debug.logError(t, "Problem calling " + msgPrefix + "service : " + serviceName + " / " + context, MODULE);
+ Debug.logError(t, "Problem calling " + msgPrefix + "service : " + serviceName + " / " + context, module);
try {
TransactionUtil.rollback(beganTx, t.getMessage(), t);
} catch (GenericTransactionException e) {
- Debug.logError(e, MODULE);
+ Debug.logError(e, module);
}

} finally {
@@ -159,11 +159,11 @@ public class ServiceSynchronization implements Synchronization {
try {
TransactionUtil.commit(beganTx);
} catch (GenericTransactionException e) {
- Debug.logError(e, MODULE);
+ Debug.logError(e, module);
}
}
} catch (GenericTransactionException e) {
- Debug.logError(e, MODULE);
+ Debug.logError(e, module);
}

}