NMS-8722: XSS vulnerability in display of SNMP sysName and trap varbind values

core/lib/pom.xml

@@ -32,6 +32,12 @@
       <groupId>org.opennms.core</groupId>
       <artifactId>org.opennms.core.api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.opennms.dependencies</groupId>
+      <artifactId>owasp-encoder-dependencies</artifactId>
+      <scope>compile</scope>
+      <type>pom</type>
+    </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>

core/lib/src/main/java/org/opennms/core/utils/WebSecurityUtils.java

@@ -1,8 +1,8 @@
 /*******************************************************************************
  * This file is part of OpenNMS(R).
  *
- * Copyright (C) 2008-2014 The OpenNMS Group, Inc.
- * OpenNMS(R) is Copyright (C) 1999-2014 The OpenNMS Group, Inc.
+ * Copyright (C) 2008-2016 The OpenNMS Group, Inc.
+ * OpenNMS(R) is Copyright (C) 1999-2016 The OpenNMS Group, Inc.
  *
  * OpenNMS(R) is a registered trademark of The OpenNMS Group, Inc.
  *
@@ -31,6 +31,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.owasp.encoder.Encode;
+
 /**
  * <p>WebSecurityUtils class.</p>
  *
@@ -47,6 +49,8 @@ public abstract class WebSecurityUtils {
 
     private static final Pattern scriptPattern = Pattern.compile("script", Pattern.CASE_INSENSITIVE);
 
+    private static final Pattern imgOnErrorPattern = Pattern.compile("(img[^>]+)o(nerror=[^>]+>)", Pattern.CASE_INSENSITIVE);
+
     /**
      * <p>sanitizeString</p>
      *
@@ -82,11 +86,16 @@ public static String sanitizeString(String raw, boolean allowHTML)
         if (raw==null || raw.length()==0) {
             return raw;
         }
+        String next;
+
+        if (allowHTML) {
+            Matcher scriptMatcher = scriptPattern.matcher(raw);
+            next = scriptMatcher.replaceAll("&#x73;cript");
 
-        Matcher scriptMatcher = scriptPattern.matcher(raw);
-        String next = scriptMatcher.replaceAll("&#x73;cript");
-        if (!allowHTML) {
-            next = next.replaceAll("<", "&lt;").replaceAll(">", "&gt;").replaceAll("\"", "&quot;");
+            Matcher imgOnErrorMatcher = imgOnErrorPattern.matcher(next);
+            next = imgOnErrorMatcher.replaceAll("$1&#x6f;$2");
+        } else {
+            next = Encode.forHtmlContent(raw);
         }
         return next;
     }

core/lib/src/test/java/org/opennms/core/utils/WebSecurityUtilsTest.java

@@ -28,6 +28,7 @@
 
 package org.opennms.core.utils;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
@@ -46,24 +47,27 @@ public class WebSecurityUtilsTest {
 	@Test
 	public void testBasicSanitizeString() {
 		String script = "<script>foo</script>";
+		String imgXss = "<img src=/ onerror=\"alert('XSS');\"></img>";
 		String html = "<table>";
 		script = WebSecurityUtils.sanitizeString(script);
+		imgXss = WebSecurityUtils.sanitizeString(imgXss);
 		html = WebSecurityUtils.sanitizeString(html);
-		assertTrue("Script is sanitized",
-				script.equals("&lt;&#x73;cript&gt;foo&lt;/&#x73;cript&gt;"));
-		assertTrue("Html is sanitized", html.equals("&lt;table&gt;"));
+		assertEquals("Script is sanitized", "&lt;script&gt;foo&lt;/script&gt;", script);
+		assertEquals("IMG XSS is sanitized", "&lt;img src=/ onerror=\"alert('XSS');\"&gt;&lt;/img&gt;", imgXss);
+		assertEquals("Html is sanitized", "&lt;table&gt;", html);
 	}
 
 	@Test
 	public void testHTMLallowedSanitizeString() {
 		String script = "<script>foo</script>";
+		String imgXss = "<img src=/ onerror=\"alert('XSS');\"></img>";
 		String html = "<table>";
 		script = WebSecurityUtils.sanitizeString(script, true);
+		imgXss = WebSecurityUtils.sanitizeString(imgXss, true);
 		html = WebSecurityUtils.sanitizeString(html, true);
-		assertTrue("Script is sanitized with HTML allowed",
-				script.equals("<&#x73;cript>foo</&#x73;cript>"));
-		assertTrue("HtmlTable is sanitized with HTML allowed, so unchanged",
-				html.equals("<table>"));
+		assertEquals("Script is sanitized with HTML allowed", "<&#x73;cript>foo</&#x73;cript>", script);
+                assertEquals("IMG XSS is sanitized with HTML allowed", "<img src=/ &#x6f;nerror=\"alert('XSS');\"></img>", imgXss);
+		assertEquals("HtmlTable is sanitized with HTML allowed, so unchanged", "<table>", html);
 	}
 
 }

dependencies/owasp-encoder/pom.xml

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <parent>
+    <groupId>org.opennms</groupId>
+    <artifactId>dependencies</artifactId>
+    <version>17.0.1-SNAPSHOT</version>
+  </parent>
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.opennms.dependencies</groupId>
+  <artifactId>owasp-encoder-dependencies</artifactId>
+  <packaging>pom</packaging>
+  <name>OpenNMS Dependencies OWASP Java Encoder</name>
+  <description>
+    This module is used to provide a single artifact that the opennms project
+    can depend on to use the OWASP Java Encoder
+  </description>
+  <dependencies>
+    <dependency>
+      <groupId>org.owasp.encoder</groupId>
+      <artifactId>encoder</artifactId>
+      <version>${owaspEncoderVersion}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>junit</groupId>
+          <artifactId>junit</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+    <dependency>
+      <groupId>org.owasp.encoder</groupId>
+      <artifactId>encoder-jsp</artifactId>
+      <version>${owaspEncoderVersion}</version>
+      <exclusions>
+        <exclusion>
+          <groupId>junit</groupId>
+          <artifactId>junit</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+  </dependencies>
+</project>

dependencies/pom.xml

@@ -42,6 +42,7 @@
     <module>mina</module>
     <module>netty</module>
     <module>newts</module>
+    <module>owasp-encoder</module>
     <module>pax-exam</module>
     <module>quartz</module>
     <module>rancid</module>

opennms-webapp/src/main/webapp/WEB-INF/jsp/notification/list.jsp

@@ -2,8 +2,8 @@
 /*******************************************************************************
  * This file is part of OpenNMS(R).
  *
- * Copyright (C) 2002-2014 The OpenNMS Group, Inc.
- * OpenNMS(R) is Copyright (C) 1999-2014 The OpenNMS Group, Inc.
+ * Copyright (C) 2002-2016 The OpenNMS Group, Inc.
+ * OpenNMS(R) is Copyright (C) 1999-2016 The OpenNMS Group, Inc.
  *
  * OpenNMS(R) is a registered trademark of The OpenNMS Group, Inc.
  *
@@ -296,7 +296,7 @@
           </td>
         </tr>
         <tr class="severity-<%=eventSeverity%>">
-          <td colspan="6"><%=notification.getTextMessage()%></td> 
+          <td colspan="6"><%=WebSecurityUtils.sanitizeString(notification.getTextMessage())%></td> 
         </tr>
       <% } /*end for*/%>
       </table>

opennms-webapp/src/main/webapp/includes/eventlist.jsp

@@ -2,8 +2,8 @@
 /*******************************************************************************
  * This file is part of OpenNMS(R).
  *
- * Copyright (C) 2002-2014 The OpenNMS Group, Inc.
- * OpenNMS(R) is Copyright (C) 1999-2014 The OpenNMS Group, Inc.
+ * Copyright (C) 2002-2016 The OpenNMS Group, Inc.
+ * OpenNMS(R) is Copyright (C) 1999-2016 The OpenNMS Group, Inc.
  *
  * OpenNMS(R) is a registered trademark of The OpenNMS Group, Inc.
  *
@@ -191,7 +191,7 @@
        <% } %>
        <td class="divider"><fmt:formatDate value="${event.time}" type="date" dateStyle="short"/>&nbsp;<fmt:formatDate value="${event.time}" type="time" pattern="HH:mm:ss"/></td>
        <td class="divider bright"><%= event.getSeverity().getLabel() %></td>
-       <td class="divider"><%=event.getLogMessage()%></td>
+       <td class="divider"><%=WebSecurityUtils.sanitizeString(event.getLogMessage())%></td>
      </tr>
 <% } %>
 

pom.xml

@@ -1299,6 +1299,7 @@
     <oroVersion>2.0.8</oroVersion>
     <osgiVersion>5.0.0</osgiVersion>
     <osgiEnterpriseVersion>5.0.0</osgiEnterpriseVersion>
+    <owaspEncoderVersion>1.2</owaspEncoderVersion>
     <quartzVersion>1.6.5</quartzVersion>
     <slf4jVersion>1.7.21</slf4jVersion>
     <snmp4jVersion>2.4.3</snmp4jVersion>
@@ -2683,6 +2684,12 @@
         <version>${project.version}</version>
         <type>pom</type>
       </dependency>
+      <dependency>
+        <groupId>org.opennms.dependencies</groupId>
+        <artifactId>owasp-encoder-dependencies</artifactId>
+        <version>${project.version}</version>
+        <type>pom</type>
+      </dependency>
 
       <!--  external dependencies -->
       <!--  PLEASE KEEP THESE IN ALPHABETICAL ORDER -->