svn commit: r1759558 - in /ofbiz/branches: release13.07/framework/widget/src/org/ofbiz/widget/form/ release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/ release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1759558 - in /ofbiz/branches: release13.07/framework/widget/src/org/ofbiz/widget/form/ release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/ release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/

jleroux@apache.org
Author: jleroux
Date: Wed Sep  7 08:02:46 2016
New Revision: 1759558

URL: http://svn.apache.org/viewvc?rev=1759558&view=rev
Log:
Fixes a vulnerability in the form widget sort-order element.

By manipulating the UL parameter externalLoginKey it is possible to pass valid Freemarker directives to the Template Engine that are reflected on webpages.
With Freemarker it is possible to create and use Java classes that implement the TemplateModel, including the freemarker.template.utility.Execute class.
An attacker can pass arbitrary commands via this class, which are executed on the server.

This fixes it using 2 redundant mechanisms (better safe than sorry):
1) UTF-8 encodes the linkUrl, by  encoding the dangerous characters this prevents the attack (thanks to Gregory)
2) Prepends the linkUrl String with r, which is a Freemarker way to prevent its own interpretation (thanks to Scott)

It's not bad to have the redundant mechanisms, and either both or one of them can be used in other places if necessary.
So far only the form widget sort-order element is concerned.

Modified:
    ofbiz/branches/release13.07/framework/widget/src/org/ofbiz/widget/form/MacroFormRenderer.java
    ofbiz/branches/release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java
    ofbiz/branches/release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java

Modified: ofbiz/branches/release13.07/framework/widget/src/org/ofbiz/widget/form/MacroFormRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release13.07/framework/widget/src/org/ofbiz/widget/form/MacroFormRenderer.java?rev=1759558&r1=1759557&r2=1759558&view=diff
==============================================================================
--- ofbiz/branches/release13.07/framework/widget/src/org/ofbiz/widget/form/MacroFormRenderer.java (original)
+++ ofbiz/branches/release13.07/framework/widget/src/org/ofbiz/widget/form/MacroFormRenderer.java Wed Sep  7 08:02:46 2016
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.net.URLEncoder;
 import java.rmi.server.UID;
 import java.sql.Timestamp;
 import java.util.HashSet;
@@ -2762,6 +2763,7 @@ public class MacroFormRenderer implement
             String newQueryString = sb.toString();
             String urlPath = UtilHttp.removeQueryStringFromTarget(paginateTarget);
             linkUrl = rh.makeLink(this.request, this.response, urlPath.concat(newQueryString));
+            linkUrl = URLEncoder.encode(linkUrl, "UTF-8");
         }
         StringWriter sr = new StringWriter();
         sr.append("<@renderSortField ");
@@ -2769,7 +2771,7 @@ public class MacroFormRenderer implement
         sr.append(sortFieldStyle);
         sr.append("\" title=\"");
         sr.append(titleText);
-        sr.append("\" linkUrl=\"");
+        sr.append("\" linkUrl=r\"");
         sr.append(linkUrl);
         sr.append("\" ajaxEnabled=");
         sr.append(Boolean.toString(ajaxEnabled));

Modified: ofbiz/branches/release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java?rev=1759558&r1=1759557&r2=1759558&view=diff
==============================================================================
--- ofbiz/branches/release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java (original)
+++ ofbiz/branches/release14.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java Wed Sep  7 08:02:46 2016
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.net.URLEncoder;
 import java.rmi.server.UID;
 import java.sql.Timestamp;
 import java.util.HashSet;
@@ -2804,6 +2805,7 @@ public final class MacroFormRenderer imp
             String newQueryString = sb.toString();
             String urlPath = UtilHttp.removeQueryStringFromTarget(paginateTarget);
             linkUrl = rh.makeLink(this.request, this.response, urlPath.concat(newQueryString));
+            linkUrl = URLEncoder.encode(linkUrl, "UTF-8");
         }
         StringWriter sr = new StringWriter();
         sr.append("<@renderSortField ");
@@ -2811,7 +2813,7 @@ public final class MacroFormRenderer imp
         sr.append(sortFieldStyle);
         sr.append("\" title=\"");
         sr.append(titleText);
-        sr.append("\" linkUrl=\"");
+        sr.append("\" linkUrl=r\"");
         sr.append(linkUrl);
         sr.append("\" ajaxEnabled=");
         sr.append(Boolean.toString(ajaxEnabled));

Modified: ofbiz/branches/release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java?rev=1759558&r1=1759557&r2=1759558&view=diff
==============================================================================
--- ofbiz/branches/release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java (original)
+++ ofbiz/branches/release15.12/framework/widget/src/org/ofbiz/widget/renderer/macro/MacroFormRenderer.java Wed Sep  7 08:02:46 2016
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.net.URLEncoder;
 import java.rmi.server.UID;
 import java.sql.Timestamp;
 import java.util.HashSet;
@@ -2876,6 +2877,7 @@ public final class MacroFormRenderer imp
             String newQueryString = sb.toString();
             String urlPath = UtilHttp.removeQueryStringFromTarget(paginateTarget);
             linkUrl = rh.makeLink(this.request, this.response, urlPath.concat(newQueryString));
+            linkUrl = URLEncoder.encode(linkUrl, "UTF-8");
         }
         StringWriter sr = new StringWriter();
         sr.append("<@renderSortField ");
@@ -2883,7 +2885,7 @@ public final class MacroFormRenderer imp
         sr.append(sortFieldStyle);
         sr.append("\" title=\"");
         sr.append(titleText);
-        sr.append("\" linkUrl=\"");
+        sr.append("\" linkUrl=r\"");
         sr.append(linkUrl);
         sr.append("\" ajaxEnabled=");
         sr.append(Boolean.toString(ajaxEnabled));