svn commit: r752397 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/util: StringUtil.java UtilValidate.java

[jo...@apache.org]

Author: jonesde
Date: Wed Mar 11 07:20:48 2009
New Revision: 752397

URL: http://svn.apache.org/viewvc?rev=752397&view=rev
Log:
Changed validation code for when allow-html=none, which is the default, to soften up on % and & since those are caught in the canonicalize process, so now only restricting greater-than and less-than characters, which should make HTML injection impossible when combined with the canonicalization that has strict double encode checking and such on

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java
    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java?rev=752397&r1=752396&r2=752397&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/StringUtil.java Wed Mar 11 07:20:48 2009
@@ -489,27 +489,33 @@
             errorMessageList.add("In field [" + valueName + "] less-than (<) and greater-than (>) symbols are not allowed.");
         }
         
-        // check for & not followed by a space (can be used for escaping chars)
+        /* NOTE DEJ 20090311: After playing with this more this doesn't seem to be necessary; the canonicalize will convert all such characters into actual text before this check is done, including other illegal chars like &lt; which will canonicalize to < and then get caught
+        // check for & followed a semicolon within 7 characters, no spaces in-between (and perhaps other things sometime?)
         int curAmpIndex = value.indexOf("&");
-        while (curAmpIndex >= 0) {
-            if (' ' != value.charAt(curAmpIndex + 1)) {
-                errorMessageList.add("In field [" + valueName + "] the ampersand (&) symbol is only allowed if followed by a space.");
+        while (curAmpIndex > -1) {
+            int semicolonIndex = value.indexOf(";", curAmpIndex + 1);
+            int spaceIndex = value.indexOf(" ", curAmpIndex + 1);
+            if (semicolonIndex > -1 && (semicolonIndex - curAmpIndex <= 7) && (spaceIndex < 0 || (spaceIndex > curAmpIndex && spaceIndex < semicolonIndex))) {
+                errorMessageList.add("In field [" + valueName + "] the ampersand (&) symbol is only allowed if not used as an encoded character: no semicolon (;) within 7 spaces or there is a space between.");
                 // once we find one like this we have the message so no need to check for more
                 break;
             }
             curAmpIndex = value.indexOf("&", curAmpIndex + 1);
         }
+         */
         
-        // check for % not followed by a space (can be used for escaping chars)
+        /* NOTE DEJ 20090311: After playing with this more this doesn't seem to be necessary; the canonicalize will convert all such characters into actual text before this check is done, including other illegal chars like %3C which will canonicalize to < and then get caught
+        // check for % followed by 2 hex characters
         int curPercIndex = value.indexOf("%");
         while (curPercIndex >= 0) {
-            if (' ' != value.charAt(curPercIndex + 1)) {
+            if (value.length() > (curPercIndex + 3) && UtilValidate.isHexDigit(value.charAt(curPercIndex + 1)) && UtilValidate.isHexDigit(value.charAt(curPercIndex + 2))) {
                 errorMessageList.add("In field [" + valueName + "] the percent (%) symbol is only allowed if followed by a space.");
                 // once we find one like this we have the message so no need to check for more
                 break;
             }
             curPercIndex = value.indexOf("%", curPercIndex + 1);
         }
+         */
         
         // TODO: anything else to check for that can be used to get HTML or JavaScript going without these characters?
         

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java?rev=752397&r1=752396&r2=752397&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java Wed Mar 11 07:20:48 2009
@@ -66,6 +66,9 @@
     /** digit characters */
     public static final String digits = "0123456789";
 
+    /** hex digit characters */
+    public static final String hexDigits = digits + "abcdefABCDEF";
+
     /** lower-case letter characters */
     public static final String lowercaseLetters = "abcdefghijklmnopqrstuvwxyz";
 
@@ -303,6 +306,11 @@
         return Character.isLetterOrDigit(c);
     }
 
+    /** Returns true if character c is a letter or digit. */
+    public static boolean isHexDigit(char c) {
+        return hexDigits.indexOf(c) >= 0;
+    }
+
     /** Returns true if all characters in string s are numbers.
      *
      *  Accepts non-signed integers only. Does not accept floating