CVE-2021-25929: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: In OpenNMS Horizon, versions opennms-1-0-stable through opennms-27.1.0-1; OpenNMS Meridian, versions meridian-foundation-2015.1.0-1 through meridian-foundation-2019.1.18-1; meridian-foundation-2020.1.0-1 through meridian-foundation-2020.1.6-1 are vulnerable to Stored Cross-Site Scripting since there is no validation on the input being sent to the `name` parameter in `noticeWizard` endpoint. Due to this flaw an authenticated attacker could inject arbitrary script and trick other admin users into downloading malicious files.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 4.8
Published: 2021-05-20T15:15Z
Repository: https://github.com/OpenNMS/opennms
Language: Other
Files changed: opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/notification/noticeWizard/eventNotices.jsp, opennms-webapp/src/main/webapp/admin/notification/noticeWizard/choosePath.jsp, smoke-test/src/test/java/org/opennms/smoketest/NotificationsPageIT.java
Commit date: 2021-03-12 17:49:36 +0100
Commit message: NMS-13123: Prevent HTML markup in notification wizard


Diff: diff --git a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/notification/noticeWizard/eventNotices.jsp b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/notification/noticeWizard/eventNotices.jsp
index fdcdcc33869..472d1a1b239 100644
--- a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/notification/noticeWizard/eventNotices.jsp
+++ b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/notification/noticeWizard/eventNotices.jsp
@@ -34,10 +34,11 @@
 	session="true"
 	import="org.opennms.web.admin.notification.noticeWizard.*"
 %>
 <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c"%>
 <%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn"%>
+<%@ taglib uri="https://www.owasp.org/index.php/OWASP_Java_Encoder_Project" prefix="e"%>
 
 <jsp:include page="/includes/bootstrap.jsp" flush="false" >
   <jsp:param name="title" value="Event Notifications" />
   <jsp:param name="headTitle" value="Event Notifications" />
   <jsp:param name="headTitle" value="Admin" />
@@ -107,35 +108,35 @@
             </th>
           </tr>
           <c:forEach items="${notifications}" var="notification">
           <tr>
             <td>
-              <input type="button" class="btn btn-secondary" value="Edit" onclick="javascript:editNotice('${notification.escapedName}')"/>
+              <input type="button" class="btn btn-secondary" value="Edit" onclick="javascript:editNotice('${e:forJavaScript(notification.escapedName)}')"/>
             </td>
             <td>
-              <input type="button" class="btn btn-secondary" value="Delete"  onclick="javascript:deleteNotice('${notification.escapedName}')"/>
+              <input type="button" class="btn btn-secondary" value="Delete"  onclick="javascript:deleteNotice('${e:forJavaScript(notification.escapedName)}')"/>
             </td>
             <td>
             <c:choose>
               <c:when test="${notification.isOn}">
-                <input type="radio" value="Off" onclick="javascript:setStatus('${notification.escapedName}','off')"/>Off
-                <input type="radio" value="On" CHECKED onclick="javascript:setStatus('${notification.escapedName}','on')"/>On
+                <input type="radio" value="Off" onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','off')"/>Off
+                <input type="radio" value="On" CHECKED onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','on')"/>On
               </c:when>
               <c:otherwise>
-                <input type="radio" value="Off" CHECKED onclick="javascript:setStatus('${notification.escapedName}','off')"/>Off
-                <input type="radio" value="On" onclick="javascript:setStatus('${notification.escapedName}','on')"/>On
+                <input type="radio" value="Off" CHECKED onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','off')"/>Off
+                <input type="radio" value="On" onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','on')"/>On
               </c:otherwise>
             </c:choose>
             </td>
             <td>
-              ${notification.name}
+            ${fn:escapeXml(notification.name)}
             </td>
             <td>
-              ${notification.eventLabel}
+              ${fn:escapeXml(notification.eventLabel)}
             </td>
             <td>
-              ${notification.displayUei}
+              ${fn:escapeXml(notification.displayUei)}
             </td>
           </tr>
           </c:forEach>
         </table>
   </div>
diff --git a/opennms-webapp/src/main/webapp/admin/notification/noticeWizard/choosePath.jsp b/opennms-webapp/src/main/webapp/admin/notification/noticeWizard/choosePath.jsp
index 4c575c664cb..77068a986cb 100644
--- a/opennms-webapp/src/main/webapp/admin/notification/noticeWizard/choosePath.jsp
+++ b/opennms-webapp/src/main/webapp/admin/notification/noticeWizard/choosePath.jsp
@@ -37,10 +37,14 @@
 		org.opennms.netmgt.config.notifications.*,
         org.opennms.netmgt.config.destinationPaths.*,
 		org.opennms.netmgt.config.*
 	"
 %>
+<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
+
+<%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn"%>
+
 
 <%!
     public void init() throws ServletException {
         try {
             DestinationPathFactory.init();
@@ -114,11 +118,11 @@
         }
     }
   
 </script>
 
-<h2><%=(newNotice.getName()!=null ? "Editing notice: " + newNotice.getName() + "<br/>" : "")%></h2>
+<h2><%=(newNotice.getName()!=null ? "Editing notice: " + WebSecurityUtils.sanitizeString(newNotice.getName()) + "<br/>" : "")%></h2>
 
 <form method="post" name="info"
       action="admin/notification/noticeWizard/notificationWizard">
       <input type="hidden" name="userAction" value=""/>
       <input type="hidden" name="sourcePage" value="<%=NotificationWizardServlet.SOURCE_PAGE_PATH%>"/>
@@ -133,32 +137,32 @@
         <tr>
           <td width="10%" valign="top" align="left">
             <label>Name:</label>
           </td>
           <td valign="top" align="left">
-            <input type="text" class="form-control" name="name" value='<%=(newNotice.getName()!=null ? newNotice.getName() : "")%>'/>
+            <input type="text" class="form-control" name="name" value='<%=(newNotice.getName()!=null ? WebSecurityUtils.sanitizeString(newNotice.getName()) : "")%>'/>
           </td>
         </tr>
         <tr>
           <td width="10%" valign="top" align="left">
             <label>Description:</label>
           </td>
           <td valign="top" align="left">
-            <input type="text" class="form-control" name="description" value='<%=newNotice.getDescription().orElse("")%>'/>
+            <input type="text" class="form-control" name="description" value='<%=WebSecurityUtils.sanitizeString(newNotice.getDescription().orElse(""))%>'/>
           </td>
         </tr>
         <tr>
           <td width="10%" valign="top" align="left">
             <label>Parameter:</label>
           </td>
           <td valign="top" align="left">
             <div class="row">
               <div class="col-md-6">
-                <label>Name:</label> <input type="text" class="form-control" size="30" name="varbindName" value='<%=varbindName%>'/>
+                <label>Name:</label> <input type="text" class="form-control" size="30" name="varbindName" value='<%=WebSecurityUtils.sanitizeString(varbindName)%>'/>
               </div>
               <div class="col-md-6">
-                <label>Value:</label> <input class="form-control" type="text" size="30" name="varbindValue" value='<%=varbindValue%>'/>
+                <label>Value:</label> <input class="form-control" type="text" size="30" name="varbindValue" value='<%=WebSecurityUtils.sanitizeString(varbindValue)%>'/>
               </div>
             </div>
           </td>
         </tr>
         <tr>
@@ -172,27 +176,27 @@
          <tr>
           <td width="10%" valign="top" align="left">
             <label>Text Message:</label>
           </td>
           <td valign="top" align="left">
-            <textarea rows="3" class="form-control" name="textMsg"><%=(newNotice.getTextMessage()!=null ? newNotice.getTextMessage() : "")%></textarea>
+            <textarea rows="3" class="form-control" name="textMsg"><%=(newNotice.getTextMessage()!=null ? WebSecurityUtils.sanitizeString(newNotice.getTextMessage()) : "")%></textarea>
           </td>
          </tr>
          <tr>
           <td width="10%" valign="top" align="left">
             <label>Short Message:</label>
           </td>
           <td valign="top" align="left">
-            <textarea rows="1" class="form-control" name="numMsg"><%=newNotice.getNumericMessage().orElse("")%></textarea>
+            <textarea rows="1" class="form-control" name="numMsg"><%=WebSecurityUtils.sanitizeString(newNotice.getNumericMessage().orElse(""))%></textarea>
           </td>
          </tr>
          <tr>
           <td width="10%" valign="top" align="left">
             <label>Email Subject:</label>
           </td>
           <td valign="top" align="left">
-            <input type="text" class="form-control" name="subject" value='<%=newNotice.getSubject().orElse("")%>'/>
+            <input type="text" class="form-control" name="subject" value='<%=WebSecurityUtils.sanitizeString(newNotice.getSubject().orElse(""))%>'/>
           </td>
          </tr>
          <tr>
           <td width="10%" valign="top" align="left">
             <label>Special Values:</label>
diff --git a/smoke-test/src/test/java/org/opennms/smoketest/NotificationsPageIT.java b/smoke-test/src/test/java/org/opennms/smoketest/NotificationsPageIT.java
index 3c2e90e9b05..01a542ae96c 100644
--- a/smoke-test/src/test/java/org/opennms/smoketest/NotificationsPageIT.java
+++ b/smoke-test/src/test/java/org/opennms/smoketest/NotificationsPageIT.java
@@ -27,19 +27,30 @@
  *******************************************************************************/
 
 package org.opennms.smoketest;
 
 import static org.junit.Assert.assertEquals;
+import static org.openqa.selenium.support.ui.ExpectedConditions.visibilityOfElementLocated;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
 
 import org.junit.Before;
 import org.junit.FixMethodOrder;
 import org.junit.Test;
 import org.junit.runners.MethodSorters;
+import org.openqa.selenium.Alert;
 import org.openqa.selenium.By;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+import org.openqa.selenium.support.ui.Select;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
 public class NotificationsPageIT extends OpenNMSSeleniumIT {
+    private static final Logger LOG = LoggerFactory.getLogger(NotificationsPageIT.class);
+
     @Before
     public void setUp() throws Exception {
         notificationsPage();
     }
 
@@ -84,7 +95,6 @@ public class NotificationsPageIT extends OpenNMSSeleniumIT {
         findElementByXpath("//p//strong[text()='acknowledged']");
         findElementByLink("[Show outstanding]");
         findElementByLink("Respond Time");
         assertElementDoesNotHaveText(By.xpath("//span[@class='label label-default']"), "admin was notified [-]");
     }
-
 }

Vulnerable code:               <input type="button" class="btn btn-secondary" value="Edit" onclick="javascript:editNotice('${notification.escapedName}')"/>
              <input type="button" class="btn btn-secondary" value="Delete"  onclick="javascript:deleteNotice('${notification.escapedName}')"/>
                <input type="radio" value="Off" onclick="javascript:setStatus('${notification.escapedName}','off')"/>Off
                <input type="radio" value="On" CHECKED onclick="javascript:setStatus('${notification.escapedName}','on')"/>On
                <input type="radio" value="Off" CHECKED onclick="javascript:setStatus('${notification.escapedName}','off')"/>Off
                <input type="radio" value="On" onclick="javascript:setStatus('${notification.escapedName}','on')"/>On
              ${notification.name}
              ${notification.eventLabel}
              ${notification.displayUei}
<h2><%=(newNotice.getName()!=null ? "Editing notice: " + newNotice.getName() + "<br/>" : "")%></h2>
            <input type="text" class="form-control" name="name" value='<%=(newNotice.getName()!=null ? newNotice.getName() : "")%>'/>
            <input type="text" class="form-control" name="description" value='<%=newNotice.getDescription().orElse("")%>'/>
                <label>Name:</label> <input type="text" class="form-control" size="30" name="varbindName" value='<%=varbindName%>'/>
                <label>Value:</label> <input class="form-control" type="text" size="30" name="varbindValue" value='<%=varbindValue%>'/>
            <textarea rows="3" class="form-control" name="textMsg"><%=(newNotice.getTextMessage()!=null ? newNotice.getTextMessage() : "")%></textarea>
            <textarea rows="1" class="form-control" name="numMsg"><%=newNotice.getNumericMessage().orElse("")%></textarea>
            <input type="text" class="form-control" name="subject" value='<%=newNotice.getSubject().orElse("")%>'/>

Fixed code: <%@ taglib uri="https://www.owasp.org/index.php/OWASP_Java_Encoder_Project" prefix="e"%>
              <input type="button" class="btn btn-secondary" value="Edit" onclick="javascript:editNotice('${e:forJavaScript(notification.escapedName)}')"/>
              <input type="button" class="btn btn-secondary" value="Delete"  onclick="javascript:deleteNotice('${e:forJavaScript(notification.escapedName)}')"/>
                <input type="radio" value="Off" onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','off')"/>Off
                <input type="radio" value="On" CHECKED onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','on')"/>On
                <input type="radio" value="Off" CHECKED onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','off')"/>Off
                <input type="radio" value="On" onclick="javascript:setStatus('${e:forJavaScript(notification.escapedName)}','on')"/>On
            ${fn:escapeXml(notification.name)}
              ${fn:escapeXml(notification.eventLabel)}
              ${fn:escapeXml(notification.displayUei)}
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>

<%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn"%>

<h2><%=(newNotice.getName()!=null ? "Editing notice: " + WebSecurityUtils.sanitizeString(newNotice.getName()) + "<br/>" : "")%></h2>
            <input type="text" class="form-control" name="name" value='<%=(newNotice.getName()!=null ? WebSecurityUtils.sanitizeString(newNotice.getName()) : "")%>'/>
            <input type="text" class="form-control" name="description" value='<%=WebSecurityUtils.sanitizeString(newNotice.getDescription().orElse(""))%>'/>
                <label>Name:</label> <input type="text" class="form-control" size="30" name="varbindName" value='<%=WebSecurityUtils.sanitizeString(varbindName)%>'/>
                <label>Value:</label> <input class="form-control" type="text" size="30" name="varbindValue" value='<%=WebSecurityUtils.sanitizeString(varbindValue)%>'/>
            <textarea rows="3" class="form-control" name="textMsg"><%=(newNotice.getTextMessage()!=null ? WebSecurityUtils.sanitizeString(newNotice.getTextMessage()) : "")%></textarea>
            <textarea rows="1" class="form-control" name="numMsg"><%=WebSecurityUtils.sanitizeString(newNotice.getNumericMessage().orElse(""))%></textarea>
            <input type="text" class="form-control" name="subject" value='<%=WebSecurityUtils.sanitizeString(newNotice.getSubject().orElse(""))%>'/>
import static org.openqa.selenium.support.ui.ExpectedConditions.visibilityOfElementLocated;

import java.time.Duration;
import java.time.temporal.ChronoUnit;
import org.openqa.selenium.Alert;
import org.openqa.selenium.support.ui.ExpectedConditions;
import org.openqa.selenium.support.ui.Select;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
    private static final Logger LOG = LoggerFactory.getLogger(NotificationsPageIT.class);