Skip to content

Commit

Permalink
Merge pull request #3042 from SCADA-LTS/fix/#2669_Fixed_XSS_vulnerabi…
Browse files Browse the repository at this point in the history
…lities_in_graphical_view_components

#2669 Fixed XSS vulnerabilities in graphical view components:
  • Loading branch information
Limraj authored Nov 6, 2024
2 parents 5a74464 + 0f52419 commit 5e8686d
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 22 deletions.
2 changes: 1 addition & 1 deletion WebContent/WEB-INF/snippet/alarmList.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<c:if test="${!hideTimestampColumn}"><td><spring:message code="common.time"/></td></c:if>
<td><spring:message code="events.msg"/></td>
<c:if test="${!hideInactivityColumn}"><td><spring:message code="common.inactiveTime"/></td></c:if>
<c:if test="${isEventAssignEnabled and !hideAssigneeColumn}"><td><sst:i18n key="common.assignee"/></td></c:if>
<c:if test="${isEventAssignEnabled and !hideAssigneeColumn}"><td><spring:message code="common.assignee"/></td></c:if>
<c:if test="${!hideAckColumn}"> <td>&nbsp;</td></c:if>
</tr>
<c:if test="${empty events}"><tr><td colspan="6"><b><spring:message code="events.emptyList"/></b></td></tr></c:if>
Expand Down
10 changes: 5 additions & 5 deletions WebContent/WEB-INF/snippet/basicContent.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,22 @@
<%@ include file="/WEB-INF/snippet/common.jsp" %>
<c:set var="content"><%--
--%><c:choose><%--
--%><c:when test="${displayPointName}">${pointComponent.name}:&nbsp;<b>${mango:htmlText(point, pointValue)}</b></c:when><%--
--%><c:when test="${displayPointName}"><c:out value="${pointComponent.name}"/>:&nbsp;<b>${mango:htmlText(point, pointValue)}</b></c:when><%--
--%><c:otherwise>${mango:htmlText(point, pointValue)}</c:otherwise><%--
--%></c:choose><%--
--%></c:set>
<c:if test="${!empty styleAttribute}"><div style="${styleAttribute}"></c:if>
<c:if test="${!empty styleAttribute}"><div style="<c:out value="${styleAttribute}"/>"></c:if>
<c:choose>
<c:when test='${!empty viewComponent}'>
<c:choose>
<c:when test='${empty viewComponent.bkgdColorOverride}'>
<span class="simpleRenderer"/>${content}</span>
</c:when>
<c:when test='${viewComponent.bkgdColorOverride == "transparent"}'>
<span class="simpleRenderer" style="background:transparent;border:0;"/>${content}</span>
<span class="simpleRenderer" style="background:transparent;border:0;"/>${content}/></span>
</c:when>
<c:otherwise>
<span class="simpleRenderer" style="background-color:${viewComponent.bkgdColorOverride};"/>${content}</span>
<span class="simpleRenderer" style="background-color:<c:out value="${viewComponent.bkgdColorOverride}"/>;"/>${content}</span>
</c:otherwise>
</c:choose>
</c:when>
Expand All @@ -47,7 +47,7 @@
<span class="simpleRenderer" style="background:transparent;border:0;"/>${content}</span>
</c:when>
<c:otherwise>
<span class="simpleRenderer" style="background-color:${pointComponent.bkgdColorOverride};"/>${content}</span>
<span class="simpleRenderer" style="background-color:<c:out value="${pointComponent.bkgdColorOverride}"/>;"/>${content}</span>
</c:otherwise>
</c:choose>
</c:otherwise>
Expand Down
4 changes: 2 additions & 2 deletions WebContent/WEB-INF/snippet/compoundInfoContent.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
This snippet supports all data types.
--%>
<%@ include file="/WEB-INF/jsp/include/tech.jsp" %>
<b>${compoundComponent.name}</b><br/>
<b><c:out value="${compoundComponent.name}"/></b><br/>
<c:forEach items="${childData}" var="child">
<c:if test="${!empty child.point}">
&nbsp;&nbsp;&nbsp;
<c:if test="${!empty sessionUser}">
<tag:img png="icon_comp" title="watchlist.pointDetails" style="display:inline"
onclick="window.location='data_point_details.shtm?dpid=${child.point.id}'"/>
</c:if>
${child.name}: <span class="infoData">${mango:htmlText(child.point, child.pointValue)}</span><br/>
<c:out value="${child.name}"/>: <span class="infoData">${mango:htmlText(child.point, child.pointValue)}</span><br/>
</c:if>
</c:forEach>
2 changes: 1 addition & 1 deletion WebContent/WEB-INF/snippet/infoContent.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<tag:img png="icon_comp" title="watchlist.pointDetails" style="display:inline"
onclick="window.location='data_point_details.shtm?dpid=${point.id}'"/>
</c:if>
<b>${pointComponent.name}</b><br/>
<b><c:out value="${pointComponent.name}"/></b><br/>

<c:if test="${!empty point}">
&nbsp;&nbsp;&nbsp;<spring:message code="common.value"/>:
Expand Down
8 changes: 4 additions & 4 deletions WebContent/WEB-INF/tags/displayView.tag
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<img id="viewBackground" src="images/spacer.gif" alt="" width="${view.width}" height="${view.height}"/>
</c:when>
<c:otherwise>
<img id="viewBackground" src="${view.backgroundFilename}" alt=""/>
<img id="viewBackground" src="<c:out value="${view.backgroundFilename}"/>" alt=""/>
</c:otherwise>
</c:choose>

Expand All @@ -50,10 +50,10 @@
<tag:pointComponent vc="${vc.leadComponent}"/>
<c:choose>
<c:when test="${empty vc.backgroundColour}"><c:set var="bkgd"></c:set></c:when>
<c:otherwise><c:set var="bkgd">background:${vc.backgroundColour};</c:set></c:otherwise>
<c:otherwise><c:set var="bkgd">background:<c:out value="${vc.backgroundColour}"/>;</c:set></c:otherwise>
</c:choose>
<div id="c${vc.id}Controls" class="controlContent" style="left:5px;top:5px;${bkgd}">
<b>${vc.name}</b><br/>
<b><c:out value="${vc.name}"/></b><br/>
<c:forEach items="${vc.childComponents}" var="child">
<c:if test="${child.viewComponent.visible && child.viewComponent.id != vc.leadComponent.id}">
<tag:pointComponent vc="${child.viewComponent}"/>
Expand Down Expand Up @@ -98,7 +98,7 @@
pointProps.push({
pointName: "<c:out value="${childComponent.viewComponent.extendedName}"/>",
alias: "<c:out value="${childComponent.viewComponent.alias}"/>",
color: "${childComponent.viewComponent.color}",
color: "<c:out value="${childComponent.viewComponent.color}"/>",
strokeWidth: ${childComponent.viewComponent.strokeWidth},
lineType: "${childComponent.viewComponent.lineType}",
showPoints: ${childComponent.viewComponent.showPoints}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.serotonin.mango.DataTypes;
import com.serotonin.util.StringUtils;
import org.scada_lts.utils.SystemSettingsUtils;

/**
* @author Matthew Lohbihler
Expand All @@ -28,7 +29,7 @@ public class AlphanumericValue extends MangoValue implements Comparable<Alphanum
private final String value;

public AlphanumericValue(String value) {
this.value = StringUtils.escapeLT(value);
this.value = SystemSettingsUtils.isDataTypeAlphanumericSaveValueAsEscaped() ? StringUtils.escapeLT(value) : value;
}

@Override
Expand Down
7 changes: 3 additions & 4 deletions src/com/serotonin/mango/web/taglib/Functions.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,17 @@ private static String getHtml(String colour, String text, boolean detectOverflow
String result;

if (text != null && detectOverflow && text.length() > 30) {
text = encodeDQuot(text);
if (StringUtils.isEmpty(colour))
result = "<input type='text' readonly='readonly' class='ovrflw' value=\"" + escape(text) + "\"/>";
else
result = "<input type='text' readonly='readonly' class='ovrflw' style='color:" + colour + ";' value=\""
+ text + "\"/>";
result = "<input type='text' readonly='readonly' class='ovrflw' style='color:" + escape(colour) + ";' value=\""
+ escape(text) + "\"/>";
}
else {
if (StringUtils.isEmpty(colour))
result = "<span>" + escape(text) + "</span>";
else
result = "<span style='color:" + colour + ";'>" + escape(text) + "</span>";
result = "<span style='color:" + escape(colour) + ";'>" + escape(text) + "</span>";
}

return result;
Expand Down
16 changes: 14 additions & 2 deletions src/org/scada_lts/utils/SystemSettingsUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ private SystemSettingsUtils() {}
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 String DATA_POINT_EXTENDED_NAME_LENGTH_IN_REPORTS_LIMIT = "systemsettings.reports.data-point-extended-name-length.limit";
private static final String DATA_POINT_EXTENDED_NAME_LENGTH_IN_REPORTS_LIMIT_KEY = "systemsettings.reports.data-point-extended-name-length.limit";

private static final String DATA_TYPE_ALPHANUMERIC_SAVE_VALUE_AS_ESCAPED_KEY = "data-type.alphanumeric.save-value-as-escaped";

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

Expand Down Expand Up @@ -641,11 +643,21 @@ public static boolean isSecurityHttpBodyProtectEnabled() {

public static int getDataPointExtendedNameLengthInReportsLimit() {
try {
String config = ScadaConfig.getInstance().getConf().getProperty(DATA_POINT_EXTENDED_NAME_LENGTH_IN_REPORTS_LIMIT, "65");
String config = ScadaConfig.getInstance().getConf().getProperty(DATA_POINT_EXTENDED_NAME_LENGTH_IN_REPORTS_LIMIT_KEY, "65");
return Integer.parseInt(config);
} catch (Exception e) {
LOG.error(e.getMessage());
return 66;
}
}

public static boolean isDataTypeAlphanumericSaveValueAsEscaped() {
try {
String dataPointTypeAlphanumericEscaped = ScadaConfig.getInstance().getConf().getProperty(DATA_TYPE_ALPHANUMERIC_SAVE_VALUE_AS_ESCAPED_KEY, "true");
return Boolean.parseBoolean(dataPointTypeAlphanumericEscaped);
} catch (Exception e) {
LOG.error(e.getMessage());
return true;
}
}
}
3 changes: 2 additions & 1 deletion test/env.properties
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,5 @@ 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]*)$

systemsettings.reports.data-point-extended-name-length.limit=64
systemsettings.reports.data-point-extended-name-length.limit=64
data-type.alphanumeric.save-value-as-escaped=false
4 changes: 3 additions & 1 deletion webapp-resources/env.properties
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,6 @@ 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]*)$

systemsettings.reports.data-point-extended-name-length.limit=64
systemsettings.reports.data-point-extended-name-length.limit=64
#save-value-as-escaped configuration for Scada-LTS version <= v2.7.8 is true
data-type.alphanumeric.save-value-as-escaped=false

0 comments on commit 5e8686d

Please sign in to comment.