Fixed: Sorting of lists generates undesired results (OFBIZ-8302)

JacquesLeRoux · JacquesLeRoux · commit cd242ea34ce3 · 2020-10-31T20:21:32.000+01:00
For this issue (OFBIZ-8302) I reverted the point 1 of http://svn.apache.org/viewvc?view=revision&revision=1759555 As reported by Alvaro Munoz from GH security team it's not sufficient: <<the second part of the fix was not effective, since the attacker can close the raw string context with a double quote and write a new attribute or even close the macro tag and write arbitrary FreeMarker code.>> So this removes the 2nd part and add better solution to fix the OFBIZ-8302 issue The solution is to encode only the QueryString and to handle it correctly in UtilHttp::getParameterMap. I must say it was not a sinecure!
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
@@ -33,9 +33,13 @@
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
 import java.net.FileNameMap;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URLConnection;
+import java.net.URLDecoder;
 import java.net.URLEncoder;
 import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.sql.Timestamp;
 import java.time.LocalDateTime;
 import java.util.Arrays;
@@ -69,6 +73,8 @@
 import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
 import org.apache.commons.lang.RandomStringUtils;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.utils.URLEncodedUtils;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.conn.ssl.TrustSelfSignedStrategy;
@@ -138,7 +144,7 @@ public static Map<String, Object> getParameterMap(HttpServletRequest request) {
      * Creates a canonicalized parameter map from a HTTP request.
      * <p>
      * If parameters are empty, the multi-part parameter map will be used.
-     * @param req  the HTTP request containing the parameters
+     * @param req the HTTP request containing the parameters
      * @param pred the predicate filtering the parameter names
      * @return a canonicalized parameter map.
      */
@@ -160,6 +166,20 @@ public static Map<String, Object> getParameterMap(HttpServletRequest req, Predic
         if (Debug.verboseOn()) {
             Debug.logVerbose("Made Request Parameter Map with [" + params.size() + "] Entries", MODULE);
         }
+
+        // Handles encoded queryStrings
+        String requestURI = req.getRequestURI();
+        if (params.isEmpty() && !requestURI.isEmpty()) {
+            try {
+                List<NameValuePair> nameValuePairs = URLEncodedUtils.parse(new URI(URLDecoder.decode(requestURI, "UTF-8")),
+                        Charset.forName("UTF-8"));
+                for (NameValuePair element : nameValuePairs) {
+                    params.put(element.getName(), element.getValue());
+                }
+            } catch (UnsupportedEncodingException | URISyntaxException e) {
+                Debug.logError("Can't handle encoded queryString " + requestURI, MODULE);
+            }
+        }
         return canonicalizeParameterMap(params);
     }
 
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
@@ -20,7 +20,9 @@
 
 import java.io.IOException;
 import java.io.StringWriter;
+import java.io.UnsupportedEncodingException;
 import java.net.URI;
+import java.net.URLEncoder;
 import java.sql.Timestamp;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -2978,7 +2980,8 @@ public void renderEndingBoundaryComment(Appendable writer, String widgetType, Mo
         }
     }
 
-    public void renderSortField(Appendable writer, Map<String, Object> context, ModelFormField modelFormField, String titleText) {
+    public void renderSortField(Appendable writer, Map<String, Object> context, ModelFormField modelFormField, String titleText)
+            throws UnsupportedEncodingException {
         boolean ajaxEnabled = false;
         ModelForm modelForm = modelFormField.getModelForm();
         List<ModelForm.UpdateArea> updateAreas = modelForm.getOnSortColumnUpdateAreas();
@@ -3038,15 +3041,15 @@ public void renderSortField(Appendable writer, Map<String, Object> context, Mode
             }
             String newQueryString = sb.toString();
             String urlPath = UtilHttp.removeQueryStringFromTarget(paginateTarget);
-            linkUrl = rh.makeLink(this.request, this.response, urlPath.concat(newQueryString));
+            linkUrl = rh.makeLink(this.request, this.response, urlPath.concat(URLEncoder.encode(newQueryString, "UTF-8")));
         }
         StringWriter sr = new StringWriter();
         sr.append("<@renderSortField ");
         sr.append(" style=\"");
         sr.append(sortFieldStyle);
         sr.append("\" title=\"");
         sr.append(titleText);
-        sr.append("\" linkUrl=r\"");
+        sr.append("\" linkUrl=\"");
         sr.append(linkUrl);
         sr.append("\" ajaxEnabled=");
         sr.append(Boolean.toString(ajaxEnabled));
