diff --git a/WebContent/WEB-INF/jsp/systemSettings.jsp b/WebContent/WEB-INF/jsp/systemSettings.jsp index c8e12ff94..e672a1e3e 100644 --- a/WebContent/WEB-INF/jsp/systemSettings.jsp +++ b/WebContent/WEB-INF/jsp/systemSettings.jsp @@ -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');">
diff --git a/build.gradle b/build.gradle index 352a67a2a..711ec0d32 100644 --- a/build.gradle +++ b/build.gradle @@ -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 diff --git a/src/org/scada_lts/utils/SystemSettingsUtils.java b/src/org/scada_lts/utils/SystemSettingsUtils.java index 39c630d47..5c9533a6d 100644 --- a/src/org/scada_lts/utils/SystemSettingsUtils.java +++ b/src/org/scada_lts/utils/SystemSettingsUtils.java @@ -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() { @@ -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; + } + } } diff --git a/src/org/scada_lts/web/beans/validation/xss/XssConstraintValidator.java b/src/org/scada_lts/web/beans/validation/xss/XssConstraintValidator.java new file mode 100644 index 000000000..ce1069391 --- /dev/null +++ b/src/org/scada_lts/web/beans/validation/xss/XssConstraintValidator.java @@ -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{ + + @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 validator = new XssValidator(); + validator.validate(value); + } +} diff --git a/src/org/scada_lts/web/beans/validation/xss/XssProtect.java b/src/org/scada_lts/web/beans/validation/xss/XssProtect.java new file mode 100644 index 000000000..c56362c59 --- /dev/null +++ b/src/org/scada_lts/web/beans/validation/xss/XssProtect.java @@ -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 {}; +} diff --git a/src/org/scada_lts/web/beans/validation/xss/XssValidator.java b/src/org/scada_lts/web/beans/validation/xss/XssValidator.java new file mode 100644 index 000000000..9ab196479 --- /dev/null +++ b/src/org/scada_lts/web/beans/validation/xss/XssValidator.java @@ -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 { + + @Override + public void validate(String input) throws XssValidatorException { + if(!validateHttpBody(input)) { + throw new XssValidatorException("Potential XSS attack detected"); + } + } +} diff --git a/src/org/scada_lts/web/beans/validation/xss/XssValidatorException.java b/src/org/scada_lts/web/beans/validation/xss/XssValidatorException.java new file mode 100644 index 000000000..e7cf8a61d --- /dev/null +++ b/src/org/scada_lts/web/beans/validation/xss/XssValidatorException.java @@ -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); + } +} diff --git a/src/org/scada_lts/web/mvc/api/UserCommentAPI.java b/src/org/scada_lts/web/mvc/api/UserCommentAPI.java index a8260fe59..5992f4992 100644 --- a/src/org/scada_lts/web/mvc/api/UserCommentAPI.java +++ b/src/org/scada_lts/web/mvc/api/UserCommentAPI.java @@ -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; @@ -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) { @@ -98,6 +100,8 @@ public ResponseEntity deleteUserComment(HttpServletRequest request, @Pat } static class CreateUserComment { + + @XssProtect private String commentText; public CreateUserComment() { diff --git a/src/org/scada_lts/web/mvc/api/css/CssStyle.java b/src/org/scada_lts/web/mvc/api/css/CssStyle.java index 0021b3f7f..cfe3b78f4 100644 --- a/src/org/scada_lts/web/mvc/api/css/CssStyle.java +++ b/src/org/scada_lts/web/mvc/api/css/CssStyle.java @@ -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) diff --git a/src/org/scada_lts/web/security/XssUtils.java b/src/org/scada_lts/web/security/XssUtils.java index c92f42392..71b1339ac 100644 --- a/src/org/scada_lts/web/security/XssUtils.java +++ b/src/org/scada_lts/web/security/XssUtils.java @@ -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 { @@ -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 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) { diff --git a/test/env.properties b/test/env.properties index be695d027..babfe1ba3 100644 --- a/test/env.properties +++ b/test/env.properties @@ -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}&?))+$ \ No newline at end of file +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:||]+onerror=|@import\\s+url\\s*\\(\\s*['\"]?javascript:|]*>|"}, + {"Link"}, + {" Click me"}, + {"body { background-image: url('\">'); }"}, + {"div { content: \"\"; }"}, + {"h1 { font-family: \"\"; }"}, + {"@import url(\"javascript:alert('XSS')\");"}, + {"div { /* comment: */ }"}, + {"span { content: '\">'; }"}, + {"h2 { color: expression(alert('XSS')); }"}, + {"\">"}, + {""}, + {""}, + {""}, + {""}, + {""}, + {"