Merge remote-tracking branch 'origin/release/2.8.0' into fix/#2116_XS…

…S_Vulnerabilities_in_2_8
SCADA-LTS · Oct 3, 2024 · 8326249 · 8326249
2 parents b57044f + 684a394
commit 8326249
Show file tree

Hide file tree

Showing 24 changed files with 547 additions and 50 deletions.
diff --git 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');">
             </textarea>
             <pre id="cssHighlighting" class="hgl-highlighting" aria-hidden="true">

diff --git 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/com/serotonin/mango/rt/dataImage/DataPointRT.java b/src/com/serotonin/mango/rt/dataImage/DataPointRT.java
@@ -129,32 +129,12 @@ public PointValueTime getPointValueAt(long time) {
 	public List<PointValueTime> getPointValues(long since) {
 		List<PointValueTime> result = pointValueService.getPointValues(
 				vo.getId(), since);
-
-		for (PointValueTime pvt : valueCache.getCacheContents()) {
-			if (pvt.getTime() >= since) {
-				int index = Collections.binarySearch(result, pvt,
-						pvtTimeComparator);
-				if (index < 0)
-					result.add(-index - 1, pvt);
-			}
-		}
-
 		return result;
 	}
 
 	public List<PointValueTime> getPointValuesBetween(long from, long to) {
 		List<PointValueTime> result = pointValueService
 				.getPointValuesBetween(vo.getId(), from, to);
-
-		for (PointValueTime pvt : valueCache.getCacheContents()) {
-			if (pvt.getTime() >= from && pvt.getTime() < to) {
-				int index = Collections.binarySearch(result, pvt,
-						pvtTimeComparator);
-				if (index < 0)
-					result.add(-index - 1, pvt);
-			}
-		}
-
 		return result;
 	}
 

diff --git a/src/com/serotonin/mango/view/chart/StatisticsChartRenderer.java b/src/com/serotonin/mango/view/chart/StatisticsChartRenderer.java
@@ -85,25 +85,23 @@ public void addDataToModel(Map<String, Object> model, DataPointVO point) {
 
         // The start value is the value of the point at the start of the period for this renderer.
         PointValueTime startValue = null;
-        if (values.size() == 0 || values.get(0).getTime() > startTime) {
+        if (!values.isEmpty()) {
             // Get the value of the point at the start time
-            PointValueTime valueTime = pointValueFacade.getPointValueBefore(startTime);
-            if (valueTime != null)
-                startValue = new PointValueTime(valueTime.getValue(), startTime);
+            startValue = values.get(0);
         }
 
         if (startValue != null || values.size() > 0) {
             if (dataTypeId == DataTypes.BINARY || dataTypeId == DataTypes.MULTISTATE) {
                 // Runtime stats
                 StartsAndRuntimeList stats = new StartsAndRuntimeList(startValue, values, startTime, startTime
                         + getDuration());
-                model.put("start", stats.getRealStart());
+                model.put("start", stats.getStart());
                 model.put("end", stats.getEnd());
                 model.put("startsAndRuntimes", stats.getData());
             }
             else if (dataTypeId == DataTypes.NUMERIC) {
                 AnalogStatistics stats = new AnalogStatistics(startValue, values, startTime, startTime + getDuration());
-                model.put("start", stats.getRealStart());
+                model.put("start", stats.getStart());
                 model.put("end", stats.getEnd());
                 model.put("minimum", stats.getMinimum());
                 model.put("minTime", stats.getMinTime());

diff --git a/src/com/serotonin/mango/view/stats/AnalogStatistics.java b/src/com/serotonin/mango/view/stats/AnalogStatistics.java
@@ -38,6 +38,7 @@ public class AnalogStatistics implements StatisticsGenerator {
     private boolean noData = true;
     private long realStart;
     private final long end;
+    private final long start;
 
     // State values.
     private long lastTime = -1;
@@ -56,6 +57,7 @@ public AnalogStatistics(Double startValue, List<? extends IValueTime> values, lo
     }
 
     public AnalogStatistics(Double startValue, long start, long end) {
+        this.start = start;
         this.end = end;
 
         if (startValue != null) {
@@ -175,11 +177,28 @@ public String getHelp() {
         return toString();
     }
 
+    public long getStart() {
+        return start;
+    }
+
     @Override
     public String toString() {
-        return "{minimum: " + minimum + ", minTime=" + minTime + ", maximum: " + maximum + ", maxTime=" + maxTime
-                + ", average: " + average + ", sum: " + sum + ", count: " + count + ", noData: " + noData
-                + ", realStart: " + realStart + ", end: " + end + "}";
+        return "AnalogStatistics{" +
+                "minimum=" + minimum +
+                ", minTime=" + minTime +
+                ", maximum=" + maximum +
+                ", maxTime=" + maxTime +
+                ", average=" + average +
+                ", sum=" + sum +
+                ", count=" + count +
+                ", noData=" + noData +
+                ", realStart=" + realStart +
+                ", start=" + start +
+                ", end=" + end +
+                ", lastTime=" + lastTime +
+                ", realDuration=" + realDuration +
+                ", lastValue=" + lastValue +
+                '}';
     }
 
     public static void main(String[] args) {

diff --git a/src/com/serotonin/mango/view/stats/StartsAndRuntimeList.java b/src/com/serotonin/mango/view/stats/StartsAndRuntimeList.java
@@ -53,8 +53,10 @@ public StartsAndRuntimeList(MangoValue startValue, List<? extends IValueTime> va
     private long realStart = -1;
     private MangoValue lastValue;
     private StartsAndRuntime sar;
+    private final long start;
 
     public StartsAndRuntimeList(MangoValue startValue, long start, long end) {
+        this.start = start;
         this.end = end;
         if (startValue != null) {
             lastTime = start;
@@ -142,6 +144,10 @@ public String getHelp() {
         return toString();
     }
 
+    public long getStart() {
+        return start;
+    }
+
     @Override
     public String toString() {
         return "{realStart: " + realStart + ", end: " + end + ", data: " + data.toString() + "}";

diff --git a/src/org/scada_lts/dao/pointvalues/PointValueDAO.java b/src/org/scada_lts/dao/pointvalues/PointValueDAO.java
@@ -194,30 +194,30 @@ public class PointValueDAO implements GenericDaoCR<PointValue>, IPointValueDAO {
 
 	public static final String POINT_VALUE_FILTER_BASE_ON_DATA_POINT_ID_AND_TIME_STAMP = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? and "
-			+ "pv."+COLUMN_NAME_TIME_STAMP+" >= ? order by "+COLUMN_NAME_TIME_STAMP;
+			+ "pv."+COLUMN_NAME_TIME_STAMP+" >= ? order by pv."+COLUMN_NAME_TIME_STAMP+", pv." + COLUMN_NAME_ID;
 
 	public static final String POINT_VALUE_FILTER_BASE_ON_DATA_POINT_ID_AND_TIME_STAMP_FROM_TO = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? and "
-			+ "pv."+COLUMN_NAME_TIME_STAMP+">=? and pv."+COLUMN_NAME_TIME_STAMP+"<? order by "+COLUMN_NAME_TIME_STAMP;
+			+ "pv."+COLUMN_NAME_TIME_STAMP+">=? and pv."+COLUMN_NAME_TIME_STAMP+"<? order by pv."+COLUMN_NAME_TIME_STAMP+", pv." + COLUMN_NAME_ID;
 
 	public static final String POINT_VALUE_FILTER_LAST_BASE_ON_DATA_POINT_ID = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? "
-			+ "order by pv."+COLUMN_NAME_TIME_STAMP+" desc";
+			+ "order by pv."+COLUMN_NAME_TIME_STAMP+" desc, pv." + COLUMN_NAME_ID + " desc";
 
 	public static final String POINT_VALUE_FILTER_LATEST_BASE_ON_DATA_POINT_ID = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? and "
 			+ "pv."+COLUMN_NAME_TIME_STAMP+"<? "
-			+ "order by pv."+COLUMN_NAME_TIME_STAMP+" desc";
+			+ "order by pv."+COLUMN_NAME_TIME_STAMP+" desc, pv." + COLUMN_NAME_ID + " desc";
 
 	public static final String POINT_VALUE_FILTER_BEFORE_TIME_STAMP_BASE_ON_DATA_POINT_ID = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? and "
 			+ "pv."+COLUMN_NAME_TIME_STAMP+"<? "
-			+ "order by pv."+COLUMN_NAME_TIME_STAMP;
+			+ "order by pv."+COLUMN_NAME_TIME_STAMP+", pv." + COLUMN_NAME_ID;
 
 	public static final String POINT_VALUE_FILTER_AT_TIME_STAMP_BASE_ON_DATA_POINT_ID = " "
 			+ "pv."+COLUMN_NAME_DATA_POINT_ID+"=? and "
 			+ "pv."+COLUMN_NAME_TIME_STAMP+"=? "
-			+ "order by pv."+COLUMN_NAME_TIME_STAMP;
+			+ "order by pv."+COLUMN_NAME_TIME_STAMP+", pv." + COLUMN_NAME_ID;
 
 	public static final String POINT_VALUE_ID_OF_LAST_VALUE = ""
 			+ "select"
@@ -246,7 +246,7 @@ public class PointValueDAO implements GenericDaoCR<PointValue>, IPointValueDAO {
 			+ "select id "
 			+ "from pointValues "
 			+ "where dataPointId =? "
-			+ "order by id DESC "
+			+ "order by ts DESC, id DESC "
 			+ "limit 2 "
 			+ ") lastId ) and " + COLUMN_NAME_TIME_STAMP + "<? ";
 
@@ -279,7 +279,7 @@ public class PointValueDAO implements GenericDaoCR<PointValue>, IPointValueDAO {
 			"(select " + COLUMN_NAME_ID +
 			" from pointValues " +
 			"where " + COLUMN_NAME_DATA_POINT_ID + " =? " +
-			"order by " + COLUMN_NAME_ID + " desc " +
+			"order by " + COLUMN_NAME_TIME_STAMP + " desc, " + COLUMN_NAME_ID + " desc " +
 			"limit 1 offset ?) lastId)";
 
 	private static final String SELECT_MAX_TIME_WHERE_DATA_POINT_ID = ""

diff --git a/src/org/scada_lts/mango/service/PointValueService.java b/src/org/scada_lts/mango/service/PointValueService.java
@@ -312,7 +312,6 @@ private long savePointValueImpl(int pointId, int dataType, double dvalue, long t
     //TODO rewrite
     private List<PointValueTime> getLstPointValueTime(List<PointValue> lstIn) {
         List<PointValueTime> lst = new ArrayList<PointValueTime>();
-        lstIn.sort(Comparator.comparing(PointValue::getId).reversed());
         for (PointValue pv : lstIn) {
             lst.add(pv.getPointValue());
         }

diff --git 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
@@ -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);
+    }
+}
diff --git a/src/org/scada_lts/web/beans/validation/xss/XssProtect.java 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
@@ -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");
+        }
+    }
+}
diff --git a/src/org/scada_lts/web/beans/validation/xss/XssValidatorException.java 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
@@ -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<String> 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
@@ -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)