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 69828b3 Fixed: Sorting of lists generates undesired results
(OFBIZ-8302)
69828b3 is described below
commit 69828b37ad4180e343703f65940a4aada7db2f26
Author: Jacques Le Roux <[email protected]>
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 | 45 ++++++++++++++++------
.../widget/renderer/macro/MacroFormRenderer.java | 12 ++++--
2 files changed, 42 insertions(+), 15 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 108a428..d489375 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
@@ -27,9 +27,14 @@ import java.io.File;
import java.io.IOException;
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.nio.ByteBuffer;
+import java.nio.charset.Charset;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
@@ -56,8 +61,9 @@ import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.FileUploadException;
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;
@@ -137,7 +143,7 @@ public final class UtilHttp {
* @return The resulting Map
*/
public static Map<String, Object> getParameterMap(HttpServletRequest
request, Set<? extends String> nameSet, Boolean onlyIncludeOrSkip) {
- boolean onlyIncludeOrSkipPrim = onlyIncludeOrSkip == null ? true :
onlyIncludeOrSkip.booleanValue();
+ boolean onlyIncludeOrSkipPrim = onlyIncludeOrSkip == null ? true :
onlyIncludeOrSkip;
Map<String, Object> paramMap = new HashMap<>();
// add all the actual HTTP request parameters
@@ -177,6 +183,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);
}
@@ -688,7 +707,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("/","_");
}
@@ -1149,18 +1168,20 @@ public final class UtilHttp {
}
}
- /** The only x-content-type-options defined value, "nosniff", prevents
Internet Explorer from MIME-sniffing a response away from the declared
content-type.
- This also applies to Google Chrome, when downloading extensions. */
+ /**
+ * The only x-content-type-options defined value, "nosniff", prevents
Internet Explorer from MIME-sniffing a response away from the declared
+ * content-type. This also applies to Google Chrome, when downloading
extensions.
+ */
resp.addHeader("x-content-type-options", "nosniff");
- /** This header enables the Cross-site scripting (XSS) filter built
into most recent web browsers.
- It's usually enabled by default anyway, so the role of this header is
to re-enable the filter for this particular website if it was disabled by the
user.
- This header is supported in IE 8+, and in Chrome (not sure which
versions). The anti-XSS filter was added in Chrome 4. Its unknown if that
version honored this header.
- FireFox has still an open bug entry and "offers" only the noscript
plugin
- https://wiki.mozilla.org/Security/Features/XSS_Filter
- https://bugzilla.mozilla.org/show_bug.cgi?id=528661
+ /**
+ * This header enables the Cross-site scripting (XSS) filter built
into most recent web browsers. It's usually enabled by default anyway, so
+ * the role of this header is to re-enable the filter for this
particular website if it was disabled by the user. This header is supported in
+ * IE 8+, and in Chrome (not sure which versions). The anti-XSS filter
was added in Chrome 4. Its unknown if that version honored this header.
+ * FireFox has still an open bug entry and "offers" only the noscript
plugin https://wiki.mozilla.org/Security/Features/XSS_Filter
+ * https://bugzilla.mozilla.org/show_bug.cgi?id=528661
**/
- resp.addHeader("X-XSS-Protection","1; mode=block");
+ resp.addHeader("X-XSS-Protection", "1; mode=block");
resp.setHeader("Referrer-Policy", "no-referrer-when-downgrade"); //
This is the default (in Firefox at least)
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 cd0465f..cb498e2 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,9 +22,14 @@ 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.net.URLEncoder;
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;
@@ -2834,7 +2839,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();
@@ -2894,7 +2900,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 ");
@@ -2902,7 +2908,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));
