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 9d3631e Fixed: Secure the uploads (OFBIZ-12080)
9d3631e is described below
commit 9d3631e278773b9a92ebc9211bb24871ce1f836b
Author: Jacques Le Roux <[email protected]>
AuthorDate: Wed Dec 2 09:53:23 2020 +0100
Fixed: Secure the uploads (OFBIZ-12080)
validateUploadedFile() fixes another case reported in August by Harshit
Conflicts handled by hand
applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
---
.../ofbiz/content/ContentManagementServices.java | 71 ++++++++++++++++------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git
a/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
b/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
index 1ff7829..67713ca 100644
---
a/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
+++
b/applications/content/src/main/java/org/apache/ofbiz/content/ContentManagementServices.java
@@ -18,6 +18,8 @@
*******************************************************************************/
package org.apache.ofbiz.content;
+import java.io.File;
+import java.io.IOException;
import java.math.BigDecimal;
import java.nio.ByteBuffer;
import java.sql.Timestamp;
@@ -29,6 +31,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Set;
+import org.apache.commons.imaging.ImageReadException;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.StringUtil;
import org.apache.ofbiz.base.util.UtilDateTime;
@@ -260,7 +263,7 @@ public class ContentManagementServices {
}
// Do update and create permission checks on Content if warranted.
- context.put("skipPermissionCheck", null); // Force check here
+ context.put("skipPermissionCheck", null); // Force check here
boolean contentExists = true;
if (Debug.infoOn()) {
Debug.logInfo("in persist... contentTypeId:" + contentTypeId + "
dataResourceTypeId:" + dataResourceTypeId + " contentId:" + contentId + "
dataResourceId:" + dataResourceId, module);
@@ -271,7 +274,9 @@ public class ContentManagementServices {
} else {
try {
GenericValue val =
EntityQuery.use(delegator).from("Content").where("contentId",
contentId).queryOne();
- if (val == null) contentExists = false;
+ if (val == null) {
+ dataResourceExists = false;
+ }
} catch (GenericEntityException e) {
return ServiceUtil.returnError(e.toString());
}
@@ -403,9 +408,8 @@ public class ContentManagementServices {
}
/**
- Service for update publish sites with a ContentRole that will tie them to
the passed
- in party.
- */
+ * Service for update publish sites with a ContentRole that will tie them
to the passed in party.
+ */
public static Map<String, Object> updateSiteRoles(DispatchContext dctx,
Map<String, ? extends Object> context) {
LocalDispatcher dispatcher = dctx.getDispatcher();
Delegator delegator = dctx.getDelegator();
@@ -529,17 +533,24 @@ public class ContentManagementServices {
return result;
}
- public static Map<String, Object>
persistDataResourceAndDataMethod(DispatchContext dctx, Map<String, ? extends
Object> rcontext) throws GenericServiceException, GenericEntityException,
Exception {
- Delegator delegator = dctx.getDelegator();
- LocalDispatcher dispatcher = dctx.getDispatcher();
- Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
- Map<String, Object> result = new HashMap<String, Object>();
- Map<String, Object> newDrContext = new HashMap<String, Object>();
- GenericValue dataResource = delegator.makeValue("DataResource");
- dataResource.setPKFields(context);
- dataResource.setNonPKFields(context);
- dataResource.setAllFields(context, false, "dr", null);
- context.putAll(dataResource);
+ public static Map<String, Object>
persistDataResourceAndDataMethod(DispatchContext dctx, Map<String, ? extends
Object> rcontext)
+ throws GenericServiceException, GenericEntityException {
+ Delegator delegator = dctx.getDelegator();
+ LocalDispatcher dispatcher = dctx.getDispatcher();
+ Map<String, Object> context = UtilMisc.makeMapWritable(rcontext);
+
+ String errorMessage = validateUploadedFile(dctx, context);
+ if (errorMessage != null) {
+ return ServiceUtil.returnError(errorMessage);
+ }
+
+ Map<String, Object> result = new HashMap<>();
+ Map<String, Object> newDrContext = new HashMap<>();
+ GenericValue dataResource = delegator.makeValue("DataResource");
+ dataResource.setPKFields(context);
+ dataResource.setNonPKFields(context);
+ dataResource.setAllFields(context, false, "dr", null);
+ context.putAll(dataResource);
GenericValue electronicText = delegator.makeValue("ElectronicText");
electronicText.setPKFields(context);
@@ -737,7 +748,7 @@ public class ContentManagementServices {
} else {
if (fromDate != null) {
// for now, will assume that any error is due to
non-existence - ignore
- //return ServiceUtil.returnError(e.toString());
+ // return ServiceUtil.returnError(e.toString());
try {
Debug.logInfo("updateSiteRoles, serviceContext(2):" +
serviceContext, module);
Map<String, Object> newContext = new HashMap<String,
Object>();
@@ -962,7 +973,6 @@ public class ContentManagementServices {
return result;
}
-
/**
* This service changes the contentTypeId of the current content and its
children depending on the pageMode.
* if pageMode == "outline" then if the contentTypeId of children is not
"OUTLINE_NODE" or "PAGE_NODE"
@@ -1492,7 +1502,9 @@ public class ContentManagementServices {
}
return result;
}
- public static Map<String, Object> followNodeChildrenMethod(GenericValue
content, LocalDispatcher dispatcher, String serviceName, Map<String, Object>
context) throws GenericEntityException, GenericServiceException {
+
+ public static Map<String, Object> followNodeChildrenMethod(GenericValue
content, LocalDispatcher dispatcher, String serviceName,
+ Map<String, Object> context) throws GenericEntityException,
GenericServiceException {
Map<String, Object> result = null;
String contentId = content.getString("contentId");
List<String> contentAssocTypeIdList =
UtilGenerics.checkList(context.get("contentAssocTypeIdList"));
@@ -1586,4 +1598,25 @@ public class ContentManagementServices {
return result;
}
+
+ private static String validateUploadedFile(DispatchContext dctx,
Map<String, ? extends Object> context) {
+ Delegator delegator = dctx.getDelegator();
+ Locale locale = (Locale) context.get("locale");
+ String objectInfo = (String) context.get("objectInfo");
+ String errorMessage = null;
+ if (!UtilValidate.isEmpty(objectInfo)) {
+ File file = new File(objectInfo);
+ if (file.isFile()) {
+ try {
+ if
(!org.apache.ofbiz.security.SecuredUpload.isValidFile(objectInfo, "All",
delegator)) {
+ errorMessage =
UtilProperties.getMessage("SecurityUiLabels",
"SupportedFileFormatsIncludingSvg", locale);
+ }
+ } catch (ImageReadException | IOException e) {
+ errorMessage = UtilProperties.getMessage(RESOURCE,
"ContentUnableToOpenFileForWriting", UtilMisc.toMap("fileName",
+ objectInfo), locale);
+ }
+ }
+ }
+ return errorMessage;
+ }
}
