[ofbiz-framework] branch release18.12 updated: Fixed: Sorting of lists generates undesired results (OFBIZ-8302)

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

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


The following commit(s) were added to refs/heads/release18.12 by this push:
     new 9cfd5a7  Fixed: Sorting of lists generates undesired results (OFBIZ-8302)
9cfd5a7 is described below

commit 9cfd5a73e1a1a8df1056a025b4180e991905b76a
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Sat Oct 31 18:47:08 2020 +0100

    Fixed: Sorting of lists generates undesired results (OFBIZ-8302)
   
    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!
   
    # Conflicts:
    # framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
    # framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java
---
 .../java/org/apache/ofbiz/base/util/UtilHttp.java  | 22 ++++++++++++++++++++--
 .../widget/renderer/macro/MacroFormRenderer.java   | 12 +++++++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

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
index d063719..666316a 100644
--- 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
@@ -29,9 +29,13 @@ import java.io.InputStream;
 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.util.ArrayList;
 import java.util.Arrays;
@@ -60,6 +64,8 @@ import org.apache.commons.fileupload.disk.DiskFileItemFactory;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
 import org.apache.commons.fileupload.servlet.ServletRequestContext;
 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;
@@ -179,6 +185,19 @@ public final class UtilHttp {
             Debug.logVerbose("Made Request Parameter Map with [" + paramMap.size() + "] Entries", module);
         }
 
+        // Handles encoded queryStrings
+        String requestURI = request.getRequestURI();
+        if (paramMap.isEmpty() && !requestURI.isEmpty()) {
+            try {
+                List<NameValuePair> nameValuePairs = URLEncodedUtils.parse(new URI(URLDecoder.decode(requestURI, "UTF-8")), Charset.forName("UTF-8"));
+                for (NameValuePair element : nameValuePairs) {
+                    paramMap.put(element.getName(), element.getValue());
+                }
+            } catch (UnsupportedEncodingException | URISyntaxException e1) {
+                Debug.logError("Can't handle encoded queryString " + requestURI, module);
+            }
+        }
+
         return canonicalizeParameterMap(paramMap);
     }
 
@@ -690,7 +709,7 @@ public final class UtilHttp {
         if (request.getContextPath().length() > 1) {
             appName = request.getContextPath().substring(1);
         }
-        // When you set a mountpoint which contains a slash inside its name (ie not only a slash as a trailer, which is possible),
+        // When you set a mountpoint which contains a slash inside its name (ie not only a slash as a trailer, which is possible),
         // as it's needed with OFBIZ-10765, OFBiz tries to create a cookie with a slash in its name and that's impossible.
         return appName.replaceAll("/","_");
     }
@@ -1016,7 +1035,6 @@ public final class UtilHttp {
 
     /**
      * Encodes a query parameter
-     *
      * @throws UnsupportedEncodingException
      */
     public static String getEncodedParameter(String parameter) throws UnsupportedEncodingException {
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
index 285c2c1..445bac9 100644
--- 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
@@ -22,8 +22,13 @@ import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.io.UnsupportedEncodingException;
+import java.net.URI;
 import java.rmi.server.UID;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
 import java.sql.Timestamp;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -2861,7 +2866,8 @@ public final class MacroFormRenderer implements FormStringRenderer {
         }
     }
 
-    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();
@@ -2921,7 +2927,7 @@ public final class MacroFormRenderer implements FormStringRenderer {
             }
             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 ");
@@ -2929,7 +2935,7 @@ public final class MacroFormRenderer implements FormStringRenderer {
         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));