This is an automated email from the ASF dual-hosted git repository. jleroux pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push: new 56c3fa8807 Fixed: Extend HTML Sanitizer - style attribute (OFBIZ-12691) 56c3fa8807 is described below commit 56c3fa8807fb73b31068c781baeac7a3fa9f1184 Author: Jacques Le Roux <jacques.le.r...@les7arts.com> AuthorDate: Tue Sep 13 11:12:40 2022 +0200 Fixed: Extend HTML Sanitizer - style attribute (OFBIZ-12691) While backporting previous trunk commit (to a0d829f770) a test error showed in 22.01 (not in trunk, the same was just in log). Then if today you try to put a quote (single or double) at https://demo-trunk.ofbiz.apache.org/content/control/WebSiteCms?webSiteId=CmsSite you won't be able to, because of: <<The Following Errors Occurred: In field [textData] by our input policy, your input has not been accepted for security reason. Please check and modify accordingly, thanks.>> This is due to the use of HtmlSanitizer.Policy() on value in checkStringForHtmlSafe The solution is to put back quotes (single or double) before comparing. While at it, I also modified checkStringForHtmlSafe to return safe HTML entities for ' and " This also adds comments about why we have <<new Locale("test")>> in several places: labels are not available in testClasses Gradle task. --- .../main/java/org/apache/ofbiz/base/util/UtilCodec.java | 15 ++++++++------- .../java/org/apache/ofbiz/base/util/UtilCodecTests.java | 6 +++--- 2 files changed, 11 insertions(+), 10 deletions(-) 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 cd505a0e9d..b24d5d9372 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 @@ -426,7 +426,7 @@ public class UtilCodec { // check for "<", ">" if (value.indexOf("<") >= 0 || value.indexOf(">") >= 0) { String issueMsg = null; - if (locale.equals(new Locale("test"))) { + if (locale.equals(new Locale("test"))) { // labels are not available in testClasses Gradle task issueMsg = "In field [" + valueName + "] less-than (<) and greater-than (>) symbols are not allowed."; } else { issueMsg = UtilProperties.getMessage("SecurityUiLabels", "PolicyNoneLess-thanGreater-than", @@ -440,7 +440,7 @@ public class UtilCodec { if (JS_EVENT_LIST.stream().anyMatch(str -> StringUtils.containsIgnoreCase(str, onEvent)) || value.contains("seekSegmentTime")) { String issueMsg = null; - if (locale.equals(new Locale("test"))) { + if (locale.equals(new Locale("test"))) { // labels are not available in testClasses Gradle task issueMsg = "In field [" + valueName + "] Javascript events are not allowed."; } else { issueMsg = UtilProperties.getMessage("SecurityUiLabels", "PolicyNoneJsEvents", @@ -478,7 +478,7 @@ public class UtilCodec { PolicyFactory policy = null; try { Class<?> customPolicyClass = null; - if (locale.equals(new Locale("test"))) { + if (locale.equals(new Locale("test"))) { // labels are not available in testClasses Gradle task customPolicyClass = Class.forName("org.apache.ofbiz.base.html.CustomSafePolicy"); } else { customPolicyClass = Class.forName(UtilProperties.getPropertyValue("owasp", "sanitizer.custom.safe.policy.class")); @@ -523,8 +523,9 @@ public class UtilCodec { } }); - // Remove space within and semicolons on end of style attributes whn using allowStyling() - value = htmlOutput.toString(); + // Remove space within and semicolons on end of style attributes when using allowStyling() + // Replace quotes to avoid issue with testCreateCustRequestItemNote and allow saving when using them in fields + value = htmlOutput.toString().replace("'", "'").replace(""", "\""); String regex = "(style\\s*=\\s*\\\"([^\\\"]*)\\\")"; Pattern p = Pattern.compile(regex); Matcher m = p.matcher(value); @@ -540,7 +541,7 @@ public class UtilCodec { String unescapeEcmaScriptAndHtml4 = StringEscapeUtils.unescapeEcmaScript(unescapeHtml4); if (filtered != null && !value.equals(unescapeEcmaScriptAndHtml4)) { String issueMsg = null; - if (locale.equals(new Locale("test"))) { + if (locale.equals(new Locale("test"))) { // labels are not available in testClasses Gradle task issueMsg = "In field [" + valueName + "] by our input policy, your input has not been accepted " + "for security reason. Please check and modify accordingly, thanks."; } else { @@ -551,7 +552,7 @@ public class UtilCodec { } } - return value; + return value.replace("'", "'").replace("\"", """); // Quotes to HTML entity to be safe } /** diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilCodecTests.java b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilCodecTests.java index 0dcaa158e8..2603bd191e 100644 --- a/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilCodecTests.java +++ b/framework/base/src/test/java/org/apache/ofbiz/base/util/UtilCodecTests.java @@ -42,7 +42,7 @@ public class UtilCodecTests { String xssVector = "<script>alert(\"XSS vector\");</script>"; List<String> errorList = new ArrayList<>(); String canonicalizedXssVector = UtilCodec.checkStringForHtmlStrictNone("fieldName", xssVector, errorList, - new Locale("test")); + new Locale("test")); // labels are not available in testClasses Gradle task assertEquals("<script>alert(\"XSS vector\");</script>", canonicalizedXssVector); assertEquals(1, errorList.size()); assertEquals("In field [fieldName] less-than (<) and greater-than (>) symbols are not allowed.", @@ -91,7 +91,7 @@ public class UtilCodecTests { String... wantedMessages) { List<String> gottenMessages = new ArrayList<>(); assertEquals(label, fixed, UtilCodec.checkStringForHtmlStrictNone(label, input, gottenMessages, - new Locale("test"))); + new Locale("test"))); // labels are not available in testClasses Gradle task assertEquals(label, Arrays.asList(wantedMessages), gottenMessages); } @@ -100,7 +100,7 @@ public class UtilCodecTests { String xssVector = "<script>alert('XSS vector');</script>"; List<String> errorList = new ArrayList<>(); String canonicalizedXssVector = UtilCodec.checkStringForHtmlSafe("fieldName", xssVector, errorList, - new Locale("test"), true); + new Locale("test"), true); // labels are not available in testClasses Gradle task assertEquals("<script>alert('XSS vector');</script>", canonicalizedXssVector); assertEquals(1, errorList.size()); assertEquals("In field [fieldName] by our input policy, your input has not been accepted for security reason. "