Author: jleroux
Date: Fri Apr  1 18:20:16 2016
New Revision: 1737415

URL: http://svn.apache.org/viewvc?rev=1737415&view=rev
Log:
Fixes a possible security issue reported by Pascal Proulx at 
https://issues.apache.org/jira/browse/OFBIZ-6973 : "Flaw in content wrapper 
cache handling with encoderType"

In ProductContentWrapper#getProductContentAsText and all similar content 
wrappers using a cache, the cacheKey does not include the new encoderType:
    String cacheKey = productContentTypeId + SEPARATOR + locale + SEPARATOR + 
mimeTypeId + SEPARATOR + product.get("productId");

This makes it possible for subsequent calls on the same wrapper using different 
encoderTypes to return content having the wrong encoding and create potential 
security flaws.

The key should include the encoderType:
    String cacheKey = productContentTypeId + SEPARATOR + locale + SEPARATOR + 
mimeTypeId + SEPARATOR + product.get("productId")  + SEPARATOR + encoderType;

jleroux: I fixed all possible such occurrences (ie when encoderType is used)

Modified:
    
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderContentWrapper.java
    
ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryContentWrapper.java
    
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
    
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWorker.java
    
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductContentWrapper.java
    
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductPromoContentWrapper.java

Modified: 
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderContentWrapper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderContentWrapper.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderContentWrapper.java
 (original)
+++ 
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderContentWrapper.java
 Fri Apr  1 18:20:16 2016
@@ -98,7 +98,7 @@ public class OrderContentWrapper impleme
 
         String orderItemSeqId = (order.getEntityName().equals("OrderItem")? 
order.getString("orderItemSeqId"): "_NA_");
 
-        String cacheKey = orderContentTypeId + SEPARATOR + locale + SEPARATOR 
+ mimeTypeId + SEPARATOR + order.get("orderId") + SEPARATOR + orderItemSeqId;
+        String cacheKey = orderContentTypeId + SEPARATOR + locale + SEPARATOR 
+ mimeTypeId + SEPARATOR + order.get("orderId") + SEPARATOR + orderItemSeqId + 
SEPARATOR + encoderType;
         try {
             String cachedValue = orderContentCache.get(cacheKey);
             if (cachedValue != null) {

Modified: 
ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryContentWrapper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryContentWrapper.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryContentWrapper.java
 (original)
+++ 
ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryContentWrapper.java
 Fri Apr  1 18:20:16 2016
@@ -97,7 +97,7 @@ public class CategoryContentWrapper impl
     public static String getProductCategoryContentAsText(GenericValue 
productCategory, String prodCatContentTypeId, Locale locale, String mimeTypeId, 
Delegator delegator, LocalDispatcher dispatcher, String encoderType) {
         String candidateFieldName = 
ModelUtil.dbNameToVarName(prodCatContentTypeId);
         UtilCodec.SimpleEncoder encoder = UtilCodec.getEncoder(encoderType);
-        String cacheKey = prodCatContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productCategory.get("productCategoryId");
+        String cacheKey = prodCatContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productCategory.get("productCategoryId") + 
SEPARATOR + encoderType;
         try {
             String cachedValue = categoryContentCache.get(cacheKey);
             if (cachedValue != null) {

Modified: 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
 (original)
+++ 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigItemContentWrapper.java
 Fri Apr  1 18:20:16 2016
@@ -120,7 +120,7 @@ public class ProductConfigItemContentWra
     public static String getProductConfigItemContentAsText(GenericValue 
productConfigItem, String confItemContentTypeId, Locale locale, String 
mimeTypeId, Delegator delegator, LocalDispatcher dispatcher, String 
encoderType) {
         UtilCodec.SimpleEncoder encoder = UtilCodec.getEncoder(encoderType);
         String candidateFieldName = 
ModelUtil.dbNameToVarName(confItemContentTypeId);
-        String cacheKey = confItemContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productConfigItem.get("configItemId");
+        String cacheKey = confItemContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productConfigItem.get("configItemId") + 
SEPARATOR + encoderType;
         try {
             String cachedValue = configItemContentCache.get(cacheKey);
             if (cachedValue != null) {

Modified: 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWorker.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWorker.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWorker.java
 (original)
+++ 
ofbiz/trunk/applications/product/src/org/ofbiz/product/config/ProductConfigWorker.java
 Fri Apr  1 18:20:16 2016
@@ -18,11 +18,11 @@
  
*******************************************************************************/
 package org.ofbiz.product.config;
 
+import java.util.Enumeration;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Enumeration;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -31,6 +31,7 @@ import org.ofbiz.base.util.UtilGenerics;
 import org.ofbiz.base.util.UtilHttp;
 import org.ofbiz.base.util.UtilProperties;
 import org.ofbiz.base.util.UtilValidate;
+import org.ofbiz.base.util.cache.UtilCache;
 import org.ofbiz.entity.Delegator;
 import org.ofbiz.entity.GenericEntityException;
 import org.ofbiz.entity.GenericValue;
@@ -42,7 +43,6 @@ import org.ofbiz.product.product.Product
 import org.ofbiz.product.store.ProductStoreWorker;
 import org.ofbiz.service.LocalDispatcher;
 import org.ofbiz.webapp.website.WebSiteWorker;
-import org.ofbiz.base.util.cache.UtilCache;
 
 /**
  * Product Config Worker class to reduce code in templates.
@@ -65,7 +65,7 @@ public class ProductConfigWorker {
             /* caching: there is one cache created, "product.config"  Each 
product's config wrapper is cached with a key of
              * productId::catalogId::webSiteId::currencyUomId, or whatever the 
SEPARATOR is defined above to be.
              */
-            String cacheKey = productId + SEPARATOR + productStoreId + 
SEPARATOR + catalogId + SEPARATOR + webSiteId + SEPARATOR + currencyUomId;
+            String cacheKey = productId + SEPARATOR + productStoreId + 
SEPARATOR + catalogId + SEPARATOR + webSiteId + SEPARATOR + currencyUomId + 
SEPARATOR + encoderType;
             configWrapper = productConfigCache.get(cacheKey);
             if (configWrapper == null) {
                 configWrapper = new 
ProductConfigWrapper((Delegator)request.getAttribute("delegator"),
@@ -345,7 +345,7 @@ public class ProductConfigWorker {
                         List<GenericValue> components = 
oneOption.getComponents();
                         for (GenericValue component: components) {
                             if (oneOption.isVirtualComponent(component) && 
UtilValidate.isNotEmpty(componentOptions)) {
-                               String  componentOption = 
componentOptions.get(component.getString("productId"));
+                                String  componentOption = 
componentOptions.get(component.getString("productId"));
                                 GenericValue configOptionProductOption = 
delegator.makeValue("ConfigOptionProductOption");
                                 configOptionProductOption.put("configId", 
configId);
                                 configOptionProductOption.put("configItemId", 
configItemId);

Modified: 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductContentWrapper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductContentWrapper.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductContentWrapper.java
 (original)
+++ 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductContentWrapper.java
 Fri Apr  1 18:20:16 2016
@@ -109,7 +109,7 @@ public class ProductContentWrapper imple
         /* caching: there is one cache created, "product.content"  Each 
product's content is cached with a key of
          * contentTypeId::locale::mimeType::productId, or whatever the 
SEPARATOR is defined above to be.
          */
-        String cacheKey = productContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + product.get("productId");
+        String cacheKey = productContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + product.get("productId") + SEPARATOR + 
encoderType;
         try {
             String cachedValue = productContentCache.get(cacheKey);
             if (cachedValue != null) {

Modified: 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductPromoContentWrapper.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductPromoContentWrapper.java?rev=1737415&r1=1737414&r2=1737415&view=diff
==============================================================================
--- 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductPromoContentWrapper.java
 (original)
+++ 
ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductPromoContentWrapper.java
 Fri Apr  1 18:20:16 2016
@@ -114,7 +114,7 @@ public class ProductPromoContentWrapper
         /* caching: there is one cache created, "product.promo.content"  Each 
productPromo's content is cached with a key of
          * contentTypeId::locale::mimeType::productPromoId, or whatever the 
SEPARATOR is defined above to be.
          */
-        String cacheKey = productPromoContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productPromo.get("productPromoId");
+        String cacheKey = productPromoContentTypeId + SEPARATOR + locale + 
SEPARATOR + mimeTypeId + SEPARATOR + productPromo.get("productPromoId") + 
SEPARATOR + encoderType;
         try {
             String cachedValue = productPromoContentCache.get(cacheKey);
             if (cachedValue != null) {


Reply via email to