Skip to content

Commit 11ff67e

Browse files
refactor(calm-hub): use centralized strict sanitization policy to prevent XSS (finos#1860)
* refactor(calm-hub): use centralized strict sanitization policy to prevent XSS Refactors CoreSchemaResource to use ResourceValidationConstants.STRICT_SANITIZATION_POLICY instead of creating a new policy instance. This ensures consistent application of security policies. Adds a regression test TestSanitizationSecurityShould to verify immunity against OWASP HtmlPolicyBuilder XSS vulnerability involving noscript and style tags. * chore: Remove blank lines between imports in CoreSchemaResource.java * Update calm-hub/src/test/java/org/finos/calm/resources/TestSanitizationSecurityShould.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 74fbaa1 commit 11ff67e

File tree

2 files changed

+45
-4
lines changed

2 files changed

+45
-4
lines changed

calm-hub/src/main/java/org/finos/calm/resources/CoreSchemaResource.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
import org.finos.calm.security.CalmHubScopes;
1515
import org.finos.calm.security.PermittedScopes;
1616
import org.finos.calm.store.CoreSchemaStore;
17-
import org.owasp.html.HtmlPolicyBuilder;
1817
import org.owasp.html.PolicyFactory;
19-
2018
import java.net.URI;
2119
import java.net.URISyntaxException;
2220
import java.util.ArrayList;
@@ -25,7 +23,7 @@
2523
@Path("/calm/schemas")
2624
public class CoreSchemaResource {
2725

28-
private static final PolicyFactory STRICT_SANITIZATION_POLICY = new HtmlPolicyBuilder().toFactory();
26+
private static final PolicyFactory STRICT_SANITIZATION_POLICY = ResourceValidationConstants.STRICT_SANITIZATION_POLICY;
2927

3028
private final CoreSchemaStore coreSchemaStore;
3129

@@ -139,4 +137,4 @@ public void setSchemas(Map<String, Object> schemas) {
139137
this.schemas = schemas;
140138
}
141139
}
142-
}
140+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.finos.calm.resources;
2+
3+
import org.junit.jupiter.api.Assertions;
4+
import org.junit.jupiter.api.Test;
5+
import org.owasp.html.PolicyFactory;
6+
7+
public class TestSanitizationSecurityShould {
8+
9+
@Test
10+
void withstand_noscript_style_injection_attack() {
11+
// This test verifies the fix for the specific vulnerability where allowTextIn("style")
12+
// combined with noscript tags could lead to XSS.
13+
// The application uses a strict policy that should NOT be vulnerable.
14+
15+
PolicyFactory policy = ResourceValidationConstants.STRICT_SANITIZATION_POLICY;
16+
17+
String pocPayload1 = "<noscript><style></noscript><script>alert(1)</script>";
18+
String sanitized1 = policy.sanitize(pocPayload1);
19+
20+
// The strict policy (default factory) should strip everything not allowed.
21+
// Since nothing is allowed, it should ideally be empty or just text content if any remains valid text.
22+
// But definitively NO <script> tags should survive.
23+
Assertions.assertFalse(sanitized1.contains("<script>"), "Sanitized output should not contain script tags: " + sanitized1);
24+
Assertions.assertFalse(sanitized1.contains("alert(1)"), "Sanitized output should not contain executable code: " + sanitized1);
25+
26+
String pocPayload2 = "<p><style></p><script>alert(1)</script>";
27+
String sanitized2 = policy.sanitize(pocPayload2);
28+
Assertions.assertFalse(sanitized2.contains("<script>"), "Sanitized output should not contain script tags: " + sanitized2);
29+
Assertions.assertFalse(sanitized2.contains("alert(1)"), "Sanitized output should not contain executable code: " + sanitized2);
30+
}
31+
32+
@Test
33+
void act_as_strict_policy() {
34+
PolicyFactory policy = ResourceValidationConstants.STRICT_SANITIZATION_POLICY;
35+
36+
String input = "<div><b>bold</b><script>alert('xss')</script></div>";
37+
String output = policy.sanitize(input);
38+
39+
// Default builder().toFactory() allows NO elements.
40+
// So it should strip all tags.
41+
Assertions.assertEquals("bold", output, "Strict policy should strip all tags");
42+
}
43+
}

0 commit comments

Comments
 (0)