[ofbiz-framework] 01/03: Improved: OWASP sanitizer breaks proper rendering of HTML code (OFBIZ-12653)

[jleroux]

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

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

commit 546e313f6d3ec9f9b20d4abb1eb7ca8c46b0e47a
Author: Jacques Le Roux <jacques.le.r...@les7arts.com>
AuthorDate: Tue Jun 21 20:48:36 2022 +0200

    Improved: OWASP sanitizer breaks proper rendering of HTML code  
(OFBIZ-12653)
    
    Allows both <br> and <br /> to pass in UtilCodec::checkStringForHtmlSafe, 
both
    are correct.
    
    Clarifies owasp.properties documentation about how to create own sanitizer
    policies
---
 framework/base/config/owasp.properties                            | 8 ++++----
 .../base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java  | 5 ++++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/framework/base/config/owasp.properties 
b/framework/base/config/owasp.properties
index 4bb7fdc716..d9520adf84 100644
--- a/framework/base/config/owasp.properties
+++ b/framework/base/config/owasp.properties
@@ -22,9 +22,9 @@
 ####
 
 # By default we use a permissive sanitizer policy
-# This has a slight impact on the code rendered, see last comments in 
OFBIZ-6669. 
-# Given as an example based on rendering cmssite, as it was before using the 
sanitizer.
-# You might want to adapt the PERMISSIVE_POLICY to your needs. 
+# This has a slight impact on the HTML code rendered, look for "cmssite" in 
OFBIZ-6669 description.
+# You will an example based on rendering cmssite, as it was before using the 
sanitizer.
+# You might want to adapt the PERMISSIVE_POLICY to your needs.
 # Be sure to check 
https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet before...
 sanitizer.enable=true
 
@@ -34,4 +34,4 @@ 
sanitizer.custom.permissive.policy.class=org.apache.ofbiz.base.html.CustomPermis
 
 # Use sanitizer.safe.policy=CUSTOM to use your custom safe PolicyFactory (see 
OFBIZ-5254)
 sanitizer.safe.policy=DEFAULT
-sanitizer.custom.safe.policy.class=org.apache.ofbiz.base.html.CustomSafePolicy
\ No newline at end of file
+sanitizer.custom.safe.policy.class=org.apache.ofbiz.base.html.CustomSafePolicy
diff --git 
a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java 
b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
index 12420d26e2..495befd3c5 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilCodec.java
@@ -490,8 +490,11 @@ public class UtilCodec {
         }
 
         if (value != null) {
+            value = value.replaceAll("<br>", "<br />"); // Both are OK, so 
<br> is accepted, see OFBIZ-12653
             String filtered = policy.sanitize(value);
-            if (filtered != null && 
!value.equals(StringEscapeUtils.unescapeEcmaScript(StringEscapeUtils.unescapeHtml4(filtered))))
 {
+            String unescapeHtml4 = StringEscapeUtils.unescapeHtml4(filtered);
+            String unescapeEcmaScriptAndHtml4 = 
StringEscapeUtils.unescapeEcmaScript(unescapeHtml4);
+            if (filtered != null && !value.equals(unescapeEcmaScriptAndHtml4)) 
{
                 String issueMsg = null;
                 if (locale.equals(new Locale("test"))) {
                     issueMsg = "In field [" + valueName + "] by our input 
policy, your input has not been accepted "