CVE-2021-25933: 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 the function `validateFormInput()` performs improper validation checks on the input sent to the `groupName` and `groupComment` parameters. Due to this flaw, an authenticated attacker could inject arbitrary script and trick other admin users into downloading malicious files which can cause severe damage to the organization using opennms.
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-doc/guide-admin/src/asciidoc/text/user-management/users.adoc, opennms-webapp/src/main/java/org/opennms/web/admin/users/AddNewUserServlet.java, opennms-webapp/src/main/java/org/opennms/web/admin/users/RenameUserServlet.java, opennms-webapp/src/main/java/org/opennms/web/controller/admin/group/GroupController.java, opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp, opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/newGroup.jsp, opennms-webapp/src/main/webapp/admin/userGroupView/users/newUser.jsp
Commit date: 2021-03-16 17:02:15 +0100
Commit message: NMS-13125: user ID and group ID must not contain any HTML markup

Diff: diff --git a/opennms-doc/guide-admin/src/asciidoc/text/user-management/users.adoc b/opennms-doc/guide-admin/src/asciidoc/text/user-management/users.adoc
index 39b3851878fc..ca433535a822 100644
--- a/opennms-doc/guide-admin/src/asciidoc/text/user-management/users.adoc
+++ b/opennms-doc/guide-admin/src/asciidoc/text/user-management/users.adoc
@@ -55,10 +55,12 @@ IMPORTANT: Change the default _admin_ password to a secure password.
 . _Optional_: Assign _Security Roles_ to give or remove permissions in the system
 . _Optional_: Provide _Notification Information_ which are used in _Notification_ targets to send messages to the _User_
 . _Optional_: Set a schedule when a _User_ should receive _Notifications_
 . Click *Finish* to persist and apply the changes
 
+WARNING: Please note that angle brackets (<>), single (') and double quotation marks ("), and the ampersand symbol (&) are not allowed to be used in the user ID.
+
 NOTE: By default a new _User_ has the _Security Role_ similar to _ROLE_USER_ assigned.
      Acknowledgment and working with _Alarms_ and _Notifications_ is possible.
      The _Configure OpenNMS_ administration menu is not available.
 
 .How to delete existing user
diff --git a/opennms-webapp/src/main/java/org/opennms/web/admin/users/AddNewUserServlet.java b/opennms-webapp/src/main/java/org/opennms/web/admin/users/AddNewUserServlet.java
index 4867e09a3fcb..9d546038fcee 100644
--- a/opennms-webapp/src/main/java/org/opennms/web/admin/users/AddNewUserServlet.java
+++ b/opennms-webapp/src/main/java/org/opennms/web/admin/users/AddNewUserServlet.java
@@ -67,10 +67,15 @@ public class AddNewUserServlet extends HttpServlet {
             throw new ServletException("AddNewUserServlet: Error initialising user factory." + e);
         }
         UserManager userFactory = UserFactory.getInstance();
 
         String userID = request.getParameter("userID");
+
+        if (userID != null && userID.matches(".*[&<>\"`']+.*")) {
+            throw new ServletException("User ID must not contain any HTML markup.");
+        }
+
         String password = request.getParameter("pass1");
 
         boolean hasUser = false;
         try {
             hasUser = userFactory.hasUser(userID);
diff --git a/opennms-webapp/src/main/java/org/opennms/web/admin/users/RenameUserServlet.java b/opennms-webapp/src/main/java/org/opennms/web/admin/users/RenameUserServlet.java
index bf4e5efb59ac..22860222f339 100644
--- a/opennms-webapp/src/main/java/org/opennms/web/admin/users/RenameUserServlet.java
+++ b/opennms-webapp/src/main/java/org/opennms/web/admin/users/RenameUserServlet.java
@@ -58,10 +58,14 @@ public class RenameUserServlet extends HttpServlet {
     @Override
     public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
         String userID = request.getParameter("userID");
         String newID = request.getParameter("newID");
 
+        if (newID != null && newID.matches(".*[&<>\"`']+.*")) {
+            throw new ServletException("User ID must not contain any HTML markup.");
+        }
+
         // now save to the xml file
         try {
             UserManager userFactory = UserFactory.getInstance();
             userFactory.renameUser(userID, newID);
         } catch (Throwable e) {
diff --git a/opennms-webapp/src/main/java/org/opennms/web/controller/admin/group/GroupController.java b/opennms-webapp/src/main/java/org/opennms/web/controller/admin/group/GroupController.java
index fa7cceac5579..f589d026c2ef 100644
--- a/opennms-webapp/src/main/java/org/opennms/web/controller/admin/group/GroupController.java
+++ b/opennms-webapp/src/main/java/org/opennms/web/controller/admin/group/GroupController.java
@@ -149,11 +149,15 @@ public class GroupController extends AbstractController implements InitializingB
 
     private ModelAndView renameGroup(HttpServletRequest request, HttpServletResponse response) throws Exception {
         
         String oldName = request.getParameter("groupName");
         String newName = request.getParameter("newName");
-        
+
+        if (newName != null && newName.matches(".*[&<>\"`']+.*")) {
+            throw new ServletException("Group ID must not contain any HTML markup.");
+        }
+
         if (StringUtils.hasText(oldName) && StringUtils.hasText(newName)) {
             m_groupRepository.renameGroup(oldName, newName);
         }
         
         return listGroups(request, response);
@@ -310,10 +314,18 @@ public class GroupController extends AbstractController implements InitializingB
         String groupComment = request.getParameter("groupComment");
         if (groupComment == null) {
             groupComment = "";
         }
 
+        if (groupName != null && groupName.matches(".*[&<>\"`']+.*")) {
+            throw new ServletException("Group ID must not contain any HTML markup.");
+        }
+
+        if (groupComment != null && groupComment.matches(".*[&<>\"`']+.*")) {
+            throw new ServletException("Group comment must not contain any HTML markup.");
+        }
+
         boolean hasGroup = false;
         try {
             hasGroup = m_groupRepository.groupExists(groupName);
         } catch (Throwable e) {
             throw new ServletException("Can't determine if group " + groupName + " already exists in groups.xml.", e);
diff --git a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp
index 05082802a81e..caf9f8a218d0 100644
--- a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp
+++ b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp
@@ -75,12 +75,15 @@
 
     function renameGroup(groupName)
     {
         var newName = prompt("Enter new name for group.", groupName);
 
-        if (newName != null && newName != "")
-        {
+        if (newName != null && newName != "") {
+          if (/.*[&<>"`']+.*/.test(newName)) {
+            alert("The group ID must not contain any HTML markup.");
+            return;
+          }
           document.allGroups.newName.value = newName;
           document.allGroups.groupName.value=groupName;
           document.allGroups.operation.value="rename";
           document.allGroups.action="admin/userGroupView/groups/modifyGroup";
           document.allGroups.submit();
diff --git a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/newGroup.jsp b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/newGroup.jsp
index 90e078543d09..4acfccc449b5 100644
--- a/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/newGroup.jsp
+++ b/opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/newGroup.jsp
@@ -41,19 +41,30 @@
 	<jsp:param name="breadcrumb" value="<a href='admin/userGroupView/groups/list.htm'>Group List</a>" />
 	<jsp:param name="breadcrumb" value="New Group" />
 </jsp:include>
 
 <script type="text/javascript">
-  function validateFormInput() 
+  function validateFormInput()
   {
     var id = new String(document.newGroupForm.groupName.value);
     if (id.toLowerCase()=="admin")
     {
         alert("The group ID '" + document.newGroupForm.groupName.value + "' cannot be used. It may be confused with the administration group ID 'Admin'.");
         return false;
     }
-    
+
+    if (/.*[&<>"`']+.*/.test(id)) {
+        alert("The group ID must not contain any HTML markup.");
+        return false;
+    }
+
+    var comment = new String(document.newGroupForm.groupComment.value);
+    if (/.*[&<>"`']+.*/.test(comment)) {
+        alert("The group comment must not contain any HTML markup.");
+        return false;
+    }
+
     document.newGroupForm.action="admin/userGroupView/groups/modifyGroup";
     document.newGroupForm.operation.value="addGroup";
     return true;
   }    
   function cancelGroup()
diff --git a/opennms-webapp/src/main/webapp/admin/userGroupView/users/newUser.jsp b/opennms-webapp/src/main/webapp/admin/userGroupView/users/newUser.jsp
index 7b862f85bef2..1262f2ee1e09 100644
--- a/opennms-webapp/src/main/webapp/admin/userGroupView/users/newUser.jsp
+++ b/opennms-webapp/src/main/webapp/admin/userGroupView/users/newUser.jsp
@@ -49,11 +49,16 @@
     if (id.toLowerCase()=="admin")
     {
         alert("The user ID '" + document.newUserForm.userID.value + "' cannot be used. It may be confused with the administration user ID 'admin'.");
         return false;
     }
-    
+
+    if (/.*[&<>"`']+.*/.test(id)) {
+        alert("The user ID must not contain any HTML markup.");
+        return false;
+    }
+
     if (document.newUserForm.pass1.value == document.newUserForm.pass2.value) 
     {
       document.newUserForm.action="admin/userGroupView/users/addNewUser";
       return true;
     } 

Vulnerable code:         
        if (newName != null && newName != "")
        {
  function validateFormInput() 
    
    
Fixed code: WARNING: Please note that angle brackets (<>), single (') and double quotation marks ("), and the ampersand symbol (&) are not allowed to be used in the user ID.


        if (userID != null && userID.matches(".*[&<>\"`']+.*")) {
            throw new ServletException("User ID must not contain any HTML markup.");
        }

        if (newID != null && newID.matches(".*[&<>\"`']+.*")) {
            throw new ServletException("User ID must not contain any HTML markup.");
        }


        if (newName != null && newName.matches(".*[&<>\"`']+.*")) {
            throw new ServletException("Group ID must not contain any HTML markup.");
        }

        if (groupName != null && groupName.matches(".*[&<>\"`']+.*")) {
            throw new ServletException("Group ID must not contain any HTML markup.");
        }

        if (groupComment != null && groupComment.matches(".*[&<>\"`']+.*")) {
            throw new ServletException("Group comment must not contain any HTML markup.");
        }

        if (newName != null && newName != "") {
          if (/.*[&<>"`']+.*/.test(newName)) {
            alert("The group ID must not contain any HTML markup.");
            return;
          }
  function validateFormInput()

    if (/.*[&<>"`']+.*/.test(id)) {
        alert("The group ID must not contain any HTML markup.");
        return false;
    }

    var comment = new String(document.newGroupForm.groupComment.value);
    if (/.*[&<>"`']+.*/.test(comment)) {
        alert("The group comment must not contain any HTML markup.");
        return false;
    }


    if (/.*[&<>"`']+.*/.test(id)) {
        alert("The user ID must not contain any HTML markup.");
        return false;
    }