Skip to content

Commit

Permalink
Merge pull request #3023 from SCADA-LTS/feature/#2992_Prevent_XSS_for…
Browse files Browse the repository at this point in the history
…_body_request

#2992 Prevent XSS for body request
  • Loading branch information
Limraj authored Sep 27, 2024
2 parents 30b2799 + d5f0a2a commit fc84a87
Show file tree
Hide file tree
Showing 18 changed files with 507 additions and 12 deletions.
2 changes: 1 addition & 1 deletion WebContent/WEB-INF/jsp/systemSettings.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@
id="cssEditor"
class="hgl-editor"
spellcheck="false"
oninput="updateCodeTextEscaped(this.value, '#cssHighlightingContent');"
oninput="updateCodeText(this.value, '#cssHighlightingContent');"
onscroll="syncCodeScroll(this, '#cssHighlightingContent');">
</textarea>
<pre id="cssHighlighting" class="hgl-highlighting" aria-hidden="true">
Expand Down
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,9 @@ test {
includeTestsMatching "com.serotonin.mango.util.StartStopDataPointsUtilsTestsSuite"
includeTestsMatching "org.scada_lts.utils.BlockingQueuesUtilsTest"
includeTestsMatching "org.scada_lts.web.security.XssProtectHtmlEscapeUtilsTest"
includeTestsMatching "org.scada_lts.web.security.XssUtilsTest"
includeTestsMatching "org.scada_lts.web.security.XssUtilsTestsSuite"
includeTestsMatching "org.scada_lts.web.mvc.api.validation.css.CssValidatorTestsSuite"
includeTestsMatching "org.scada_lts.web.beans.validation.xss.XssValidatorTestsSuite"
}

failFast = true
Expand Down
32 changes: 32 additions & 0 deletions src/org/scada_lts/utils/SystemSettingsUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ private SystemSettingsUtils() {}

private static final String CUSTOM_CSS_CONTENT_KEY = "systemsettings.custom.css.content";

private static final String SECURITY_HTTP_BODY_ACCESS_DENIED_REGEX_KEY = "scadalts.security.http.body.access.denied.regex";
private static final String SECURITY_HTTP_BODY_ACCESS_GRANTED_REGEX_KEY = "scadalts.security.http.body.access.granted.regex";
private static final String SECURITY_HTTP_BODY_PROTECT_ENABLED_KEY = "scadalts.security.http.body.protect.enabled";

private static final org.apache.commons.logging.Log LOG = LogFactory.getLog(SystemSettingsUtils.class);

public static DataPointSyncMode getDataPointSynchronizedMode() {
Expand Down Expand Up @@ -605,4 +609,32 @@ public static String getCustomCssContent() {
return "";
}
}

public static String getSecurityHttpBodyAccessDeniedRegex() {
try {
return ScadaConfig.getInstance().getConf().getProperty(SECURITY_HTTP_BODY_ACCESS_DENIED_REGEX_KEY, "");
} catch (Exception e) {
LOG.error(e.getMessage());
return "";
}
}

public static String getSecurityHttpBodyAccessGrantedRegex() {
try {
return ScadaConfig.getInstance().getConf().getProperty(SECURITY_HTTP_BODY_ACCESS_GRANTED_REGEX_KEY, "");
} catch (Exception e) {
LOG.error(e.getMessage());
return "";
}
}

public static boolean isSecurityHttpBodyProtectEnabled() {
try {
String securityHttpQueryXssEnabled = ScadaConfig.getInstance().getConf().getProperty(SECURITY_HTTP_BODY_PROTECT_ENABLED_KEY, "false");
return Boolean.parseBoolean(securityHttpQueryXssEnabled);
} catch (Exception e) {
LOG.error(e.getMessage());
return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.scada_lts.web.beans.validation.xss;

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<XssProtect, 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 XssValidator();
validator.validate(value);
}
}
15 changes: 15 additions & 0 deletions src/org/scada_lts/web/beans/validation/xss/XssProtect.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.scada_lts.web.beans.validation.xss;

import javax.validation.Constraint;
import javax.validation.Payload;
import java.lang.annotation.*;

@Documented
@Constraint(validatedBy = XssConstraintValidator.class)
@Target({ElementType.METHOD, ElementType.FIELD, ElementType.ANNOTATION_TYPE, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
public @interface XssProtect {
String message() default "Potential XSS detected in the request body.";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}
15 changes: 15 additions & 0 deletions src/org/scada_lts/web/beans/validation/xss/XssValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.scada_lts.web.beans.validation.xss;

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

import static org.scada_lts.web.security.XssUtils.validateHttpBody;

public class XssValidator implements ScadaValidator<String> {

@Override
public void validate(String input) throws XssValidatorException {
if(!validateHttpBody(input)) {
throw new XssValidatorException("Potential XSS attack detected");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.scada_lts.web.beans.validation.xss;

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);
}
}
6 changes: 5 additions & 1 deletion src/org/scada_lts/web/mvc/api/UserCommentAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
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.beans.validation.xss.XssProtect;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;

import javax.servlet.http.HttpServletRequest;
import javax.validation.Valid;

import static org.scada_lts.utils.UserCommentApiUtils.validUserComment;
import static org.scada_lts.utils.UserCommentApiUtils.validUserCommentWithTs;
Expand Down Expand Up @@ -41,7 +43,7 @@ public UserCommentAPI() {
* @return Status
*/
@PostMapping(value = "/{typeId}/{refId}")
public ResponseEntity<?> createUserComment(HttpServletRequest request, @RequestBody CreateUserComment createUserComment, @PathVariable("typeId") Integer typeId, @PathVariable("refId") Integer refId) {
public ResponseEntity<?> createUserComment(HttpServletRequest request, @Valid @RequestBody CreateUserComment createUserComment, @PathVariable("typeId") Integer typeId, @PathVariable("refId") Integer refId) {
try {
User user = Common.getUser(request);
if(user != null) {
Expand Down Expand Up @@ -98,6 +100,8 @@ public ResponseEntity<String> deleteUserComment(HttpServletRequest request, @Pat
}

static class CreateUserComment {

@XssProtect
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.beans.validation.xss.XssProtect;

public class CssStyle {

@CssValid
@CssValid @XssProtect
private final String content;

@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
Expand Down
29 changes: 24 additions & 5 deletions src/org/scada_lts/web/security/XssUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.scada_lts.serorepl.utils.StringUtils;
import org.scada_lts.utils.SystemSettingsUtils;

import java.util.function.Predicate;
import java.util.regex.Pattern;

public final class XssUtils {
Expand All @@ -14,24 +15,42 @@ private XssUtils() {}
public static final int SECURITY_HTTP_ACCESS_GRANTED_QUERY_LIMIT = SystemSettingsUtils.getSecurityHttpQueryLimit();
public static final boolean SECURITY_HTTP_QUERY_PROTECT_ENABLED = SystemSettingsUtils.isSecurityHttpQueryProtectEnabled();

private static final Pattern SECURITY_HTTP_ACCESS_DENIED_BODY_REGEX = init(SystemSettingsUtils.getSecurityHttpBodyAccessDeniedRegex());
private static final Pattern SECURITY_HTTP_ACCESS_GRANTED_BODY_REGEX = init(SystemSettingsUtils.getSecurityHttpBodyAccessGrantedRegex());
public static final boolean SECURITY_HTTP_BODY_PROTECT_ENABLED = SystemSettingsUtils.isSecurityHttpBodyProtectEnabled();

public static boolean validateHttpQuery(String query) {
return validate(query, SECURITY_HTTP_QUERY_PROTECT_ENABLED, q -> q.length() > SECURITY_HTTP_ACCESS_GRANTED_QUERY_LIMIT,
SECURITY_HTTP_ACCESS_DENIED_QUERY_REGEX, SECURITY_HTTP_ACCESS_GRANTED_QUERY_REGEX);

}

public static boolean validateHttpBody(String body) {
return validate(body, SECURITY_HTTP_BODY_PROTECT_ENABLED, b -> false, SECURITY_HTTP_ACCESS_DENIED_BODY_REGEX,
SECURITY_HTTP_ACCESS_GRANTED_BODY_REGEX);
}

if(!SECURITY_HTTP_QUERY_PROTECT_ENABLED)
private static boolean validate(String input,
boolean protectEnabled,
Predicate<String> beforeCheckRegex,
Pattern accessDeniedRegex,
Pattern accessGrantedRegex) {
if(!protectEnabled)
return true;

if (query == null || query.isEmpty()) {
if (input == null || input.isEmpty()) {
return false;
}

if(query.length() > SECURITY_HTTP_ACCESS_GRANTED_QUERY_LIMIT) {
if(beforeCheckRegex.test(input)) {
return false;
}

if(SECURITY_HTTP_ACCESS_DENIED_QUERY_REGEX != null && SECURITY_HTTP_ACCESS_DENIED_QUERY_REGEX.matcher(query).matches()) {
if(accessDeniedRegex != null && accessDeniedRegex.matcher(input).matches()) {
return false;
}

return SECURITY_HTTP_ACCESS_GRANTED_QUERY_REGEX == null || SECURITY_HTTP_ACCESS_GRANTED_QUERY_REGEX.matcher(query).matches();
return accessGrantedRegex == null || accessGrantedRegex.matcher(input).matches();
}

private static Pattern init(String regex) {
Expand Down
10 changes: 8 additions & 2 deletions test/env.properties
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ http.protocol.timeout-ms=15000
event.assign.enabled=true

scadalts.security.http.query.protect.enabled=true
scadalts.security.http.query.limit=3900
scadalts.security.http.query.limit=137
scadalts.security.http.query.access.denied.regex=^(.*?(javascript:|onerror|onload|onmouseover|alert\\(){1}.*?)$
scadalts.security.http.query.access.granted.regex=^(([a-zA-Z0-9_\\-]{1,32}=[a-zA-Z0-9_\\-.,/+=\s!$*?@%]*&?)|([a-zA-Z0-9_\\-]{1,32}&?))+$
scadalts.security.http.query.access.granted.regex=^(([a-zA-Z0-9_\\-]{1,32}=[a-zA-Z0-9_\\-.,/+=\s!$*?@%]*&?)|([a-zA-Z0-9_\\-]{1,32}&?))+$

systemsettings.custom.css.content=#top-description-container {\n\r\t\t\t\tdisplay: flex;\n\r\t\t\t\talign-items: flex-end;\n\r\t\t\t\tjustify-content: center;\n\r} \n\r\n\r#top-description-prefix { \n\r\t\t\t\tcolor: black !important;\n\r\t\t\t\tfont-size: 2em !important;\n\r\t\t\t\tmargin-left: 0.5em !important;\n\r\t\t\t\tmargin-right: 0.5em !important;\n\r\t\t\t\tdisplay: inline-block !important;\n\r\t\t\t\tvertical-align: bottom !important;\n\r\t\t\t\tline-height: 1 !important;\n\r} \n\r\n\r#top-description {\n\r\t\t\t\tcolor: #39B54A !important;\n\r\t\t\t\tfont-size: 2em !important;\n\r\t\t\t\tdisplay: inline-block !important;\n\r\t\t\t\tline-height: 1 !important;\n\r}

scadalts.security.http.body.protect.enabled=true
scadalts.security.http.body.access.denied.regex=^(.*?((expression\\s*\\()|url\\s*\\(\\s*['\"]?javascript:|url\\s*\\(\\s*['\"]?data:|<script[^>]*>|</script>|<img[^>]+onerror=|@import\\s+url\\s*\\(\\s*['\"]?javascript:|<img[^>]*>|<script[^>]*>|<[^>]+>onerror\\s*=|onload\\s*=|eval\\s*\\(|alert\\s*\\(|onerror\\s*=|document.location){1}.*?)$
scadalts.security.http.body.access.granted.regex=^([\\s\\S]*)$
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.scada_lts.web.beans.validation.xss;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.scada_lts.web.beans.validation.ScadaValidator;
import org.scada_lts.web.beans.validation.ScadaValidatorException;

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

@RunWith(Parameterized.class)
public class XssValidatorExceptionTest {

@Parameterized.Parameters(name = "{index}: input: {0}")
public static Collection<Object[]> data() {
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')); }"},
{"\"><img src=x onerror=alert(document.location)>"},
{"<img src='x' onerror='alert(1)'>"},
{"<input type=\"text\" value=\"\" onfocus=\"alert(1)\">"},
{"<iframe src=\"javascript:alert(1)\"></iframe>"},
{"<form action=\"javascript:alert(1)\"><input type=\"submit\"></form>"},
{"<object data=\"javascript:alert(1)\"></object>"},
{"<embed src=\"javascript:alert(1)\">"},
{"<base href=\"javascript:alert(1)//\">"},
{"<svg onload=\"alert(1)\">"},
{"<svg><script>alert(1)</script></svg>"},
{"<math><a xlink:href=\"javascript:alert(1)\">XSS</a></math>"},
{"<img src=x onerror=\"alert(String.fromCharCode(88,83,83))\">"},
{"<b onmouseover=\"alert(1)\">XSS</b>"},
{"<video><source onerror=\"javascript:alert(1)\"></video>"},
{"<details open ontoggle=\"alert(1)\">"},
{"<input onfocus=\"alert('XSS')\">"},
{"<div style=\"width: expression(alert('XSS'))\">"},
{"<meta http-equiv=\"refresh\" content=\"0;url=javascript:alert(1)\">"},
{"<link rel=\"stylesheet\" href=\"javascript:alert(1)\">"},
{"<textarea onfocus=\"alert(1)\"></textarea>"},
{"<a href=\"//example.com\" onclick=\"alert(1)\">Link</a>"},
{"<button onclick=\"alert(1)\">Click me</button>"},
{"<div style=\"background-image: url(javascript:alert(1))\">XSS</div>"},
{"<audio src=\"javascript:alert(1)\"></audio>"},
{"<marquee onstart=\"alert(1)\">XSS</marquee>"},
{"<keygen autofocus onfocus=\"alert(1)\">"},
{"<command onclick=\"alert(1)\">Click me</command>"},
{"<img src=\"https://example.com/image.jpg\" alt=\"Example Image\" width=\"600\" height=\"400\" border=\"0\" />"},
{"<img src=\"http://example.com/image.jpg\" alt=\"Example Image\" width=\"600\" height=\"400\" border=\"0\" />"},
{"><img src=x onerror=alert(document.location)>"},
});
}

private final String input;
private final ScadaValidator<String> validator;

public XssValidatorExceptionTest(String input) {
this.input = input;
this.validator = new XssValidator();
}

@Test(expected = XssValidatorException.class)
public void when_isInvalidXss() throws ScadaValidatorException {
validator.validate(input);
}
}
55 changes: 55 additions & 0 deletions test/org/scada_lts/web/beans/validation/xss/XssValidatorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.scada_lts.web.beans.validation.xss;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.scada_lts.web.beans.validation.ScadaValidator;
import org.scada_lts.web.beans.validation.ScadaValidatorException;

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

@RunWith(Parameterized.class)
public class XssValidatorTest {

@Parameterized.Parameters(name = "{index}: input: {0}")
public static Collection<Object[]> data() {
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; }"},
{"a > 12"},
{"a>12"},
{"a> 12"},
{"a >12"},
{"a<12"},
{"a < 12"},
{"a< 12"},
{"a <12"},
{"'a'"},
{"\"a\""},
});
}

private final String input;
private final ScadaValidator<String> validator;

public XssValidatorTest(String input) {
this.input = input;
this.validator = new XssValidator();
}

@Test
public void when_isInvalidXss() throws ScadaValidatorException {
validator.validate(input);
}
}
Loading

0 comments on commit fc84a87

Please sign in to comment.