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 0a49e0a Fixed: CVE-2021-37608 vulnerability bypass (OFBIZ-12307)
0a49e0a is described below
commit 0a49e0adddd3577f344781ddec664027d8de799d
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Fri Sep 17 11:29:43 2021 +0200
Fixed: CVE-2021-37608 vulnerability bypass (OFBIZ-12307)
Fixes a number of issues I spotted while working on OFBIZ-12305 in relation
with
OFBIZ-12055
The last change I made for OFBIZ-12305 was incomplete, the files could not
be
checked by SecuredUpload because they did not exist! This concerns
ImageManagementServices, DataServices, FrameImage and ProductServices
classes.
I used Files::createTempFile to fix that.
Also fixes a bug in SecuredUpload where I reversed the check on
fileToCheck.length in Windows case. I also added a comment, Windows 10 now
allows more length (need an OS parameter change though)
Finally, creates public SecuredUpload::isValidText to be used in OFBIZ-12305
Conflicts handled by hand
DataServices.java
---
.../apache/ofbiz/content/data/DataServices.java | 192 +++++++++++----------
.../ofbiz/product/imagemanagement/FrameImage.java | 8 +-
.../imagemanagement/ImageManagementServices.java | 21 ++-
.../ofbiz/product/product/ProductServices.java | 8 +-
.../org/apache/ofbiz/security/SecuredUpload.java | 6 +-
5 files changed, 140 insertions(+), 95 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 b6b17df..47e5a01 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
@@ -27,6 +27,10 @@ import java.io.RandomAccessFile;
import java.io.StringWriter;
import java.io.Writer;
import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
import java.sql.Timestamp;
import java.util.Arrays;
import java.util.HashMap;
@@ -38,7 +42,6 @@ import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.GeneralException;
import org.apache.ofbiz.base.util.UtilDateTime;
import org.apache.ofbiz.base.util.UtilGenerics;
-import org.apache.ofbiz.base.util.UtilIO;
import org.apache.ofbiz.base.util.UtilMisc;
import org.apache.ofbiz.base.util.UtilProperties;
import org.apache.ofbiz.base.util.UtilValidate;
@@ -57,8 +60,8 @@ import org.apache.ofbiz.service.ServiceUtil;
*/
public class DataServices {
- public static final String module = DataServices.class.getName();
- public static final String resource = "ContentUiLabels";
+ private static final String MODULE = DataServices.class.getName();
+ private static final String RESOURCE = "ContentUiLabels";
public static Map<String, Object>
clearAssociatedRenderCache(DispatchContext dctx, Map<String, Object> context) {
Delegator delegator = dctx.getDelegator();
@@ -67,8 +70,9 @@ public class DataServices {
try {
DataResourceWorker.clearAssociatedRenderCache(delegator,
dataResourceId);
} catch (GeneralException e) {
- Debug.logError(e, "Unable to clear associated render cache with
dataResourceId=" + dataResourceId, module);
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentClearAssociatedRenderCacheError", UtilMisc.toMap("dataResourceId",
dataResourceId), locale));
+ Debug.logError(e, "Unable to clear associated render cache with
dataResourceId=" + dataResourceId, MODULE);
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentClearAssociatedRenderCacheError",
+ UtilMisc.toMap("dataResourceId", dataResourceId), locale));
}
return ServiceUtil.returnSuccess();
}
@@ -129,7 +133,7 @@ public class DataServices {
dataResourceId = delegator.getNextSeqId("DataResource");
}
if (Debug.infoOn()) {
- Debug.logInfo("in createDataResourceMethod, dataResourceId:" +
dataResourceId, module);
+ Debug.logInfo("in createDataResourceMethod, dataResourceId:" +
dataResourceId, MODULE);
}
GenericValue dataResource = delegator.makeValue("DataResource",
UtilMisc.toMap("dataResourceId", dataResourceId));
dataResource.setNonPKFields(context);
@@ -140,9 +144,10 @@ public class DataServices {
// get first statusId for content out of the statusItem table if not
provided
if (UtilValidate.isEmpty(dataResource.get("statusId"))) {
try {
- GenericValue statusItem =
EntityQuery.use(delegator).from("StatusItem").where("statusTypeId",
"CONTENT_STATUS").orderBy("sequenceId").queryFirst();
+ GenericValue statusItem =
EntityQuery.use(delegator).from("StatusItem").where("statusTypeId",
"CONTENT_STATUS")
+ .orderBy("sequenceId").queryFirst();
if (statusItem != null) {
- dataResource.put("statusId", statusItem.get("statusId"));
+ dataResource.put("statusId", statusItem.get("statusId"));
}
} catch (GenericEntityException e) {
return ServiceUtil.returnError(e.getMessage());
@@ -173,7 +178,8 @@ public class DataServices {
String dataResourceId = (String) context.get("dataResourceId");
String textData = (String) context.get("textData");
if (UtilValidate.isNotEmpty(textData)) {
- GenericValue electronicText =
delegator.makeValue("ElectronicText", UtilMisc.toMap("dataResourceId",
dataResourceId, "textData", textData));
+ GenericValue electronicText = delegator.makeValue("ElectronicText",
+ UtilMisc.toMap("dataResourceId", dataResourceId,
"textData", textData));
try {
electronicText.create();
} catch (GenericEntityException e) {
@@ -210,18 +216,19 @@ public class DataServices {
// extended validation for binary/character data
if (UtilValidate.isNotEmpty(textData) && binData != null) {
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentCannotProcessBothCharacterAndBinaryFile", locale));
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentCannotProcessBothCharacterAndBinaryFile", locale));
}
// obtain a reference to the file
File file = null;
if (UtilValidate.isEmpty(objectInfo)) {
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableObtainReferenceToFile", UtilMisc.toMap("objectInfo", ""),
locale));
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableObtainReferenceToFile",
+ UtilMisc.toMap("objectInfo", ""), locale));
}
if (UtilValidate.isEmpty(dataResourceTypeId) ||
"LOCAL_FILE".equals(dataResourceTypeId) ||
"LOCAL_FILE_BIN".equals(dataResourceTypeId)) {
file = new File(objectInfo);
if (!file.isAbsolute()) {
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentLocalFileDoesNotPointToAbsoluteLocation", locale));
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentLocalFileDoesNotPointToAbsoluteLocation", locale));
}
} else if ("OFBIZ_FILE".equals(dataResourceTypeId) ||
"OFBIZ_FILE_BIN".equals(dataResourceTypeId)) {
prefix = System.getProperty("ofbiz.home");
@@ -232,7 +239,7 @@ public class DataServices {
} else if ("CONTEXT_FILE".equals(dataResourceTypeId) ||
"CONTEXT_FILE_BIN".equals(dataResourceTypeId)) {
prefix = (String) context.get("rootDir");
if (UtilValidate.isEmpty(prefix)) {
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentCannotFindContextFileWithEmptyContextRoot", locale));
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentCannotFindContextFileWithEmptyContextRoot", locale));
}
if (objectInfo.indexOf('/') != 0 && prefix.lastIndexOf('/') !=
(prefix.length() - 1)) {
sep = "/";
@@ -240,43 +247,49 @@ public class DataServices {
file = new File(prefix + sep + objectInfo);
}
if (file == null) {
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableObtainReferenceToFile", UtilMisc.toMap("objectInfo", objectInfo),
locale));
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableObtainReferenceToFile",
+ UtilMisc.toMap("objectInfo", objectInfo), locale));
}
// write the data to the file
if (UtilValidate.isNotEmpty(textData)) {
- try (
- OutputStreamWriter out = new OutputStreamWriter(new
FileOutputStream(file), UtilIO.getUtf8());
- ) {
+ try (OutputStreamWriter out = new OutputStreamWriter(new
FileOutputStream(file), StandardCharsets.UTF_8);) {
out.write(textData);
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));
+ Debug.logWarning(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableWriteCharacterDataToFile",
+ UtilMisc.toMap("fileName", file.getAbsolutePath()),
locale));
}
} else if (binData != null) {
try {
// Check if a webshell is not uploaded
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(),
"All", delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, binData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"All", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels",
"SupportedFileFormatsIncludingSvg", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.write(binData.array());
out.close();
} catch (FileNotFoundException | ImageReadException e) {
- Debug.logError(e, module);
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ Debug.logError(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableToOpenFileForWriting",
+ UtilMisc.toMap("fileName", file.getAbsolutePath()),
locale));
} catch (IOException e) {
- Debug.logError(e, module);
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableWriteBinaryDataToFile", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ Debug.logError(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableWriteBinaryDataToFile",
+ UtilMisc.toMap("fileName", file.getAbsolutePath()),
locale));
}
} else {
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentNoContentFilePassed", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentNoContentFilePassed",
+ UtilMisc.toMap("fileName", file.getAbsolutePath()),
locale));
}
Map<String, Object> result = ServiceUtil.returnSuccess();
@@ -324,8 +337,9 @@ public class DataServices {
try {
dataResource =
EntityQuery.use(delegator).from("DataResource").where("dataResourceId",
dataResourceId).queryOne();
} catch (GenericEntityException e) {
- Debug.logWarning(e, module);
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentDataResourceNotFound", UtilMisc.toMap("parameters.dataResourceId",
dataResourceId), locale));
+ Debug.logWarning(e, MODULE);
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentDataResourceNotFound",
+ UtilMisc.toMap("parameters.dataResourceId",
dataResourceId), locale));
}
dataResource.setNonPKFields(context);
@@ -335,7 +349,7 @@ public class DataServices {
try {
dataResource.store();
} catch (GenericEntityException e) {
- Debug.logError(e, module);
+ Debug.logError(e, MODULE);
return ServiceUtil.returnError(e.getMessage());
}
@@ -352,8 +366,8 @@ public class DataServices {
}
/**
- * Because sometimes a DataResource will exist, but no ElectronicText has
been created,
- * this method will create an ElectronicText if it does not exist.
+ * Because sometimes a DataResource will exist, but no ElectronicText has
been created, this method will create an
+ * ElectronicText if it does not exist.
* @param dctx the dispatch context
* @param context the context
* @return update the ElectronicText
@@ -368,12 +382,12 @@ public class DataServices {
String contentId = (String) context.get("contentId");
result.put("contentId", contentId);
if (UtilValidate.isEmpty(dataResourceId)) {
- Debug.logError("dataResourceId is null.", module);
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentDataResourceIsNull", locale));
+ Debug.logError("dataResourceId is null.", MODULE);
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentDataResourceIsNull", locale));
}
String textData = (String) context.get("textData");
if (Debug.verboseOn()) {
- Debug.logVerbose("in updateElectronicText, textData:" + textData,
module);
+ Debug.logVerbose("in updateElectronicText, textData:" + textData,
MODULE);
}
try {
electronicText =
EntityQuery.use(delegator).from("ElectronicText").where("dataResourceId",
dataResourceId).queryOne();
@@ -387,8 +401,8 @@ public class DataServices {
electronicText.create();
}
} catch (GenericEntityException e) {
- Debug.logWarning(e, module);
- return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentElectronicTextNotFound", locale) + " " + e.getMessage());
+ Debug.logWarning(e, MODULE);
+ return ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentElectronicTextNotFound", locale) + " " + e.getMessage());
}
return result;
@@ -445,53 +459,59 @@ public class DataServices {
// write the data to the file
if (UtilValidate.isNotEmpty(textData)) {
- try (
- OutputStreamWriter out = new OutputStreamWriter(new
FileOutputStream(file),UtilIO.getUtf8());
- ) {
+ try (OutputStreamWriter out = new OutputStreamWriter(new
FileOutputStream(file), StandardCharsets.UTF_8);) {
out.write(textData);
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));
+ Debug.logWarning(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableWriteCharacterDataToFile",
+ UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
}
} else if (binData != null) {
try {
// Check if a webshell is not uploaded
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(),
"All", delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, binData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels",
"SupportedFileFormatsIncludingSvg", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.setLength(binData.array().length);
out.write(binData.array());
out.close();
} catch (FileNotFoundException | ImageReadException e) {
- Debug.logError(e, module);
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ Debug.logError(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableToOpenFileForWriting",
+ UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
} catch (IOException e) {
- Debug.logError(e, module);
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentUnableWriteBinaryDataToFile", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ Debug.logError(e, MODULE);
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentUnableWriteBinaryDataToFile",
+ UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
}
} else {
- return
ServiceUtil.returnError(UtilProperties.getMessage(resource,
"ContentNoContentFilePassed", UtilMisc.toMap("fileName",
file.getAbsolutePath()), locale));
+ return
ServiceUtil.returnError(UtilProperties.getMessage(RESOURCE,
"ContentNoContentFilePassed",
+ UtilMisc.toMap("fileName", file.getAbsolutePath()),
locale));
}
} catch (IOException e) {
- Debug.logWarning(e, module);
+ Debug.logWarning(e, MODULE);
throw new GenericServiceException(e.getMessage());
}
return result;
}
- public static Map<String, Object> renderDataResourceAsText(DispatchContext
dctx, Map<String, ? extends Object> context) throws GeneralException,
IOException {
+ public static Map<String, Object> renderDataResourceAsText(DispatchContext
dctx, Map<String, ? extends Object> context)
+ throws GeneralException, IOException {
Map<String, Object> results = new HashMap<>();
//LocalDispatcher dispatcher = dctx.getDispatcher();
Writer out = (Writer) context.get("outWriter");
- Map<String, Object> templateContext =
UtilGenerics.checkMap(context.get("templateContext"));
+ Map<String, Object> templateContext =
UtilGenerics.cast(context.get("templateContext"));
//GenericValue userLogin = (GenericValue) context.get("userLogin");
String dataResourceId = (String) context.get("dataResourceId");
if (templateContext != null && UtilValidate.isEmpty(dataResourceId)) {
@@ -514,7 +534,7 @@ public class DataServices {
out.write(outWriter.toString());
results.put("textData", outWriter.toString());
} catch (IOException e) {
- Debug.logError(e, "Error rendering sub-content text", module);
+ Debug.logError(e, "Error rendering sub-content text", MODULE);
return ServiceUtil.returnError(e.toString());
}
return results;
@@ -533,14 +553,15 @@ public class DataServices {
Delegator delegator = dctx.getDelegator();
//Locale locale = (Locale) context.get("locale");
String dataResourceId = (String) context.get("dataResourceId");
- ByteBuffer byteBuffer = (ByteBuffer)context.get("imageData");
+ ByteBuffer byteBuffer = (ByteBuffer) context.get("imageData");
if (byteBuffer != null) {
byte[] imageBytes = byteBuffer.array();
try {
- GenericValue imageDataResource =
EntityQuery.use(delegator).from("ImageDataResource").where("dataResourceId",
dataResourceId).queryOne();
+ GenericValue imageDataResource =
EntityQuery.use(delegator).from("ImageDataResource")
+ .where("dataResourceId", dataResourceId).queryOne();
if (Debug.infoOn()) {
- Debug.logInfo("imageDataResource(U):" + imageDataResource,
module);
- Debug.logInfo("imageBytes(U):" +
Arrays.toString(imageBytes), module);
+ Debug.logInfo("imageDataResource(U):" + imageDataResource,
MODULE);
+ Debug.logInfo("imageBytes(U):" +
Arrays.toString(imageBytes), MODULE);
}
if (imageDataResource == null) {
return createImageMethod(dctx, context);
@@ -566,14 +587,14 @@ public class DataServices {
Map<String, Object> result = new HashMap<>();
Delegator delegator = dctx.getDelegator();
String dataResourceId = (String) context.get("dataResourceId");
- ByteBuffer byteBuffer = (ByteBuffer)context.get("imageData");
+ ByteBuffer byteBuffer = (ByteBuffer) context.get("imageData");
if (byteBuffer != null) {
byte[] imageBytes = byteBuffer.array();
try {
GenericValue imageDataResource =
delegator.makeValue("ImageDataResource", UtilMisc.toMap("dataResourceId",
dataResourceId));
imageDataResource.setBytes("imageData", imageBytes);
if (Debug.infoOn()) {
- Debug.logInfo("imageDataResource(C):" + imageDataResource,
module);
+ Debug.logInfo("imageDataResource(C):" + imageDataResource,
MODULE);
}
imageDataResource.create();
} catch (GenericEntityException e) {
@@ -597,35 +618,34 @@ public class DataServices {
return result;
}
- public static Map<String, Object> createBinaryFileMethod(DispatchContext
dctx, Map<String, ? extends Object> context) throws GenericServiceException {
+ 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");
+ 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);
- Debug.logInfo("in createBinaryFileMethod, objectInfo:" +
objectInfo, module);
- Debug.logInfo("in createBinaryFileMethod, rootDir:" + rootDir,
module);
+ Debug.logInfo("in createBinaryFileMethod, dataResourceTypeId:" +
dataResourceTypeId, MODULE);
+ Debug.logInfo("in createBinaryFileMethod, objectInfo:" +
objectInfo, MODULE);
+ Debug.logInfo("in createBinaryFileMethod, rootDir:" + rootDir,
MODULE);
}
try {
file = DataResourceWorker.getContentFile(dataResourceTypeId,
objectInfo, rootDir);
} catch (FileNotFoundException | GeneralException e) {
- Debug.logWarning(e, module);
+ Debug.logWarning(e, MODULE);
throw new GenericServiceException(e.getMessage());
}
if (Debug.infoOn()) {
- Debug.logInfo("in createBinaryFileMethod, file:" + file, module);
- Debug.logInfo("in createBinaryFileMethod, imageData:" +
imageData.length, module);
+ Debug.logInfo("in createBinaryFileMethod, file:" + file, MODULE);
+ Debug.logInfo("in createBinaryFileMethod, imageData:" +
imageData.length, MODULE);
}
if (imageData != null && imageData.length > 0) {
- try (
- FileOutputStream out = new FileOutputStream(file);
- ) {
+ try (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)) {
@@ -633,10 +653,10 @@ public class DataServices {
return ServiceUtil.returnError(errorMessage);
}
if (Debug.infoOn()) {
- Debug.logInfo("in createBinaryFileMethod, length:" +
file.length(), module);
+ Debug.logInfo("in createBinaryFileMethod, length:" +
file.length(), MODULE);
}
} catch (IOException | ImageReadException e) {
- Debug.logWarning(e, module);
+ Debug.logWarning(e, MODULE);
throw new GenericServiceException(e.getMessage());
}
}
@@ -657,38 +677,34 @@ public class DataServices {
return result;
}
- public static Map<String, Object> updateBinaryFileMethod(DispatchContext
dctx, Map<String, ? extends Object> context) throws GenericServiceException {
+ 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");
+ 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);
- Debug.logInfo("in updateBinaryFileMethod, objectInfo:" +
objectInfo, module);
- Debug.logInfo("in updateBinaryFileMethod, rootDir:" + rootDir,
module);
+ Debug.logInfo("in updateBinaryFileMethod, dataResourceTypeId:" +
dataResourceTypeId, MODULE);
+ Debug.logInfo("in updateBinaryFileMethod, objectInfo:" +
objectInfo, MODULE);
+ Debug.logInfo("in updateBinaryFileMethod, rootDir:" + rootDir,
MODULE);
}
try {
file = DataResourceWorker.getContentFile(dataResourceTypeId,
objectInfo, rootDir);
- } catch (FileNotFoundException e) {
- Debug.logWarning(e, module);
+ } catch (FileNotFoundException | GeneralException e) {
+ Debug.logWarning(e, MODULE);
throw new GenericServiceException(e.getMessage());
- } catch (GeneralException e2) {
- Debug.logWarning(e2, module);
- throw new GenericServiceException(e2.getMessage());
}
if (Debug.infoOn()) {
- Debug.logInfo("in updateBinaryFileMethod, file:" + file, module);
- Debug.logInfo("in updateBinaryFileMethod, imageData:" +
Arrays.toString(imageData), module);
+ Debug.logInfo("in updateBinaryFileMethod, file:" + file, MODULE);
+ Debug.logInfo("in updateBinaryFileMethod, imageData:" +
Arrays.toString(imageData), MODULE);
}
if (imageData != null && imageData.length > 0) {
- try (
- FileOutputStream out = new FileOutputStream(file);
- ){
+ try (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)) {
@@ -697,10 +713,10 @@ public class DataServices {
}
if (Debug.infoOn()) {
- Debug.logInfo("in updateBinaryFileMethod, length:" +
file.length(), module);
+ Debug.logInfo("in updateBinaryFileMethod, length:" +
file.length(), MODULE);
}
} catch (IOException | ImageReadException e) {
- Debug.logWarning(e, module);
+ Debug.logWarning(e, MODULE);
throw new GenericServiceException(e.getMessage());
}
}
diff --git
a/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java
b/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java
index e942586..776ddab 100644
---
a/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java
+++
b/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/FrameImage.java
@@ -31,6 +31,9 @@ import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
@@ -292,11 +295,14 @@ public class FrameImage {
request.setAttribute("_ERROR_MESSAGE_", "There is an existing
frame, please select from the existing frame.");
return "error";
}
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.getAbsolutePath(),
"Image", delegator)) {
+ Path tmpFile = Files.createTempFile(null, null);
+ Files.write(tmpFile, imageData.array(), StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tmpFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels",
"SupportedFileFormatsIncludingSvg", locale);
request.setAttribute("_ERROR_MESSAGE_", errorMessage);
return "error";
}
+ Files.delete(tmpFile);
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.write(imageData.array());
out.close();
diff --git
a/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/ImageManagementServices.java
b/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/ImageManagementServices.java
index 54eb24d..d339678 100644
---
a/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/ImageManagementServices.java
+++
b/applications/product/src/main/java/org/apache/ofbiz/product/imagemanagement/ImageManagementServices.java
@@ -27,6 +27,9 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
@@ -149,12 +152,16 @@ public class ImageManagementServices {
}
if (UtilValidate.isEmpty(imageResize)) {
- // Create image file original to folder product id.
+ // check file content
try {
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(file.toString(), "Image",
delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, imageData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels", "SupportedImageFormats", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
+ // Create image file original to folder product id.
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.write(imageData.array());
out.close();
@@ -175,10 +182,13 @@ public class ImageManagementServices {
fileOriginal = checkExistsImage(fileOriginal);
try {
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(fileOriginal.toString(),
"Image", delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, imageData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels", "SupportedImageFormats", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
RandomAccessFile outFile = new
RandomAccessFile(fileOriginal, "rw");
outFile.write(imageData.array());
outFile.close();
@@ -528,10 +538,13 @@ public class ImageManagementServices {
String fileToCheck = imageServerPath + "/" + productId + "/" +
filenameToUseThumb;
File fileOriginalThumb = new File(fileToCheck);
try {
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(fileToCheck, "Image",
delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, imageData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels", "SupportedImageFormats", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
RandomAccessFile outFileThumb = new
RandomAccessFile(fileOriginalThumb, "rw");
outFileThumb.write(imageData.array());
outFileThumb.close();
diff --git
a/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductServices.java
b/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductServices.java
index d8f9924..3fe1a66 100644
---
a/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductServices.java
+++
b/applications/product/src/main/java/org/apache/ofbiz/product/product/ProductServices.java
@@ -24,6 +24,9 @@ import java.io.IOException;
import java.io.RandomAccessFile;
import java.math.BigDecimal;
import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
import java.sql.Timestamp;
import java.util.Arrays;
import java.util.HashMap;
@@ -1323,10 +1326,13 @@ public class ProductServices {
File file = new File(fileToCheck);
try {
- if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(fileToCheck, "Image",
delegator)) {
+ Path tempFile = Files.createTempFile(null, null);
+ Files.write(tempFile, imageData.array(),
StandardOpenOption.APPEND);
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(tempFile.toString(),
"Image", delegator)) {
String errorMessage =
UtilProperties.getMessage("SecurityUiLabels", "SupportedImageFormats", locale);
return ServiceUtil.returnError(errorMessage);
}
+ Files.delete(tempFile);
RandomAccessFile out = new RandomAccessFile(file, "rw");
out.write(imageData.array());
out.close();
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 bfffcec..e3fb163 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
@@ -123,7 +123,7 @@ public class SecuredUpload {
}
if (org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS) {
- if (fileToCheck.length() < 259) {
+ 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
@@ -615,6 +615,10 @@ public class SecuredUpload {
return false;
}
String content = new String(bytesFromFile);
+ return isValidText(content);
+ }
+
+ public static boolean isValidText(String content) throws IOException {
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...
