Skip to content

Commit

Permalink
#2992 Prevent XSS for body request
Browse files Browse the repository at this point in the history
Added:
- OWASP validator for requests bodies
- Annotation for XssValid.java (used for custom CSS and user comments in alarms section)
- Tests for validator class
  • Loading branch information
Patrykb0802 committed Sep 25, 2024
1 parent 30b2799 commit 85554b8
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 3 deletions.
Binary file added WebContent/WEB-INF/lib/java10-shim-20240325.1.jar
Binary file not shown.
Binary file added WebContent/WEB-INF/lib/java8-shim-20240325.1.jar
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ test {
includeTestsMatching "org.scada_lts.web.security.XssProtectHtmlEscapeUtilsTest"
includeTestsMatching "org.scada_lts.web.security.XssUtilsTest"
includeTestsMatching "org.scada_lts.web.mvc.api.validation.css.CssValidatorTestsSuite"
includeTestsMatching "org.scada_lts.web.security.XssUtilsTestsSuite"
}

failFast = true
Expand Down
3 changes: 3 additions & 0 deletions src/org/scada_lts/web/mvc/api/UserCommentAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.scada_lts.mango.service.UserCommentService;
import org.scada_lts.web.beans.ApplicationBeans;
import org.scada_lts.web.mvc.api.json.JsonUserComment;
import org.scada_lts.web.security.XssValid;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
Expand Down Expand Up @@ -98,6 +99,8 @@ public ResponseEntity<String> deleteUserComment(HttpServletRequest request, @Pat
}

static class CreateUserComment {

@XssValid
private String commentText;

public CreateUserComment() {
Expand Down
3 changes: 2 additions & 1 deletion src/org/scada_lts/web/mvc/api/css/CssStyle.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.scada_lts.web.beans.validation.css.CssValid;
import org.scada_lts.web.security.XssValid;

public class CssStyle {

@CssValid
@CssValid @XssValid
private final String content;

@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
Expand Down
36 changes: 36 additions & 0 deletions src/org/scada_lts/web/security/OwaspXssValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.scada_lts.web.security;

import org.scada_lts.web.beans.validation.ScadaValidator;
import org.owasp.html.HtmlPolicyBuilder;
import org.owasp.html.PolicyFactory;

public class OwaspXssValidator implements ScadaValidator<String> {

private final PolicyFactory policyFactory;

public OwaspXssValidator() {
this.policyFactory = createPolicyFactory();
}

@Override
public void validate(String input) throws XssValidatorException {
try {
validateXss(input);
} catch (Exception e) {
throw new XssValidatorException("XSS validation failed: " + e.getMessage(), e);
}
}

void validateXss(String input) throws XssValidatorException {
String sanitized = policyFactory.sanitize(input);
if (!sanitized.equals(input)) {
throw new XssValidatorException("Potential XSS attack detected");
}
}

private PolicyFactory createPolicyFactory() {
return new HtmlPolicyBuilder()
.allowElements("b", "i", "u", "strong", "em", "p", "ul", "ol", "li")
.toFactory();
}
}
21 changes: 21 additions & 0 deletions src/org/scada_lts/web/security/XssConstraintValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.scada_lts.web.security;

import org.scada_lts.serorepl.utils.StringUtils;
import org.scada_lts.web.beans.validation.AbstractConstraintValidator;
import org.scada_lts.web.beans.validation.ScadaValidator;

public class XssConstraintValidator extends AbstractConstraintValidator<XssValid, String> {

@Override
public void beforeValidate(String value) throws Exception {
if (StringUtils.isEmpty(value)) {
throw new XssValidatorException("Input is empty");
}
}

@Override
public void validate(String value) throws Exception {
ScadaValidator<String> validator = new OwaspXssValidator();
validator.validate(value);
}
}
19 changes: 19 additions & 0 deletions src/org/scada_lts/web/security/XssValid.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.scada_lts.web.security;

import javax.validation.Constraint;
import javax.validation.Payload;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Constraint(validatedBy = XssConstraintValidator.class)
@Target({ ElementType.FIELD, ElementType.PARAMETER })
@Retention(RetentionPolicy.RUNTIME)
public @interface XssValid {
String message() default "Potential XSS detected in the request body.";

Class<?>[] groups() default {};

Class<? extends Payload>[] payload() default {};
}
25 changes: 25 additions & 0 deletions src/org/scada_lts/web/security/XssValidatorException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.scada_lts.web.security;

import org.scada_lts.web.beans.validation.ScadaValidatorException;

public class XssValidatorException extends ScadaValidatorException {

public XssValidatorException() {
}

public XssValidatorException(String message) {
super(message);
}

public XssValidatorException(String message, Throwable cause) {
super(message, cause);
}

public XssValidatorException(Throwable cause) {
super(cause);
}

public XssValidatorException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
}
}
13 changes: 13 additions & 0 deletions test/org/scada_lts/web/security/XssUtilsTestsSuite.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.scada_lts.web.security;

import org.junit.runner.RunWith;
import org.junit.runners.Suite;

@RunWith(Suite.class)
@Suite.SuiteClasses({
XssUtilsValidateHttpQueryTest.class,
XssUtilsValidateHttpBodyTest.class,
XssUtilsValidateHttpBodyExceptionTest.class
})
public class XssUtilsTestsSuite {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.scada_lts.web.security;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.util.Arrays;
import java.util.Collection;

@RunWith(Parameterized.class)
public class XssUtilsValidateHttpBodyExceptionTest {

@Parameterized.Parameters
public static Collection<Object[]> testData() {
return Arrays.asList(new Object[][]{
{null},
{"<script>alert(1)</script>"},
{"<a href=\"javascript:alert(1)\">Link</a>"},
{"<div onclick=\"alert(1)\">Click me</div>"},
{"body { background-image: url('\"><img src=x onerror=alert(document.location)>'); }"},
{"div { content: \"<script>alert('XSS')</script>\"; }"},
{"h1 { font-family: \"<img src=x onerror=alert('XSS')>\"; }"},
{"@import url(\"javascript:alert('XSS')\");"},
{"div { /* comment: <img src=x onerror=alert('XSS')> */ }"},
{"span { content: '\"><script>alert(1)</script>'; }"},
{"h2 { color: expression(alert('XSS')); }"}
});
}

private final String input;
private final OwaspXssValidator owaspXssValidator = new OwaspXssValidator();

public XssUtilsValidateHttpBodyExceptionTest(String input) {
this.input = input;
}

@Test(expected = XssValidatorException.class)
public void testValidateHttpBodyException() throws XssValidatorException {
owaspXssValidator.validate(input);
}
}
43 changes: 43 additions & 0 deletions test/org/scada_lts/web/security/XssUtilsValidateHttpBodyTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.scada_lts.web.security;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Arrays;
import java.util.Collection;

@RunWith(Parameterized.class)
public class XssUtilsValidateHttpBodyTest {

@Parameterized.Parameters
public static Collection<Object[]> testData() {
return Arrays.asList(new Object[][]{
{"<b>Bold</b>"},
{"Hello, World!"},
{""},
{"<i>Italic text</i>"},
{"<u>Underlined</u>"},
{"<strong>Strong text</strong>"},
{"<em>Emphasized text</em>"},
{"<p>Paragraph with <b>bold</b> and <i>italic</i></p>"},
{"<ul><li>Item 1</li><li>Item 2</li></ul>"},
{"<ol><li>First</li><li>Second</li></ol>"},
{"body { font-size: 14px; }"},
{"h1 { font-weight: bold; }"},
{"p { margin: 0; padding: 0; }"},
});
}

private final String input;
private final OwaspXssValidator owaspXssValidator = new OwaspXssValidator();

public XssUtilsValidateHttpBodyTest(String input) {
this.input = input;
}

@Test
public void testValidateHttpBody() throws XssValidatorException {
owaspXssValidator.validate(input);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
import java.util.Collection;

@RunWith(Parameterized.class)
public class XssUtilsTest {
public class XssUtilsValidateHttpQueryTest {

private final String input;
private final boolean expectedResult;

public XssUtilsTest(String input, boolean expectedResult) {
public XssUtilsValidateHttpQueryTest(String input, boolean expectedResult) {
this.input = input;
this.expectedResult = expectedResult;
}
Expand Down

0 comments on commit 85554b8

Please sign in to comment.