QPID-8675 - [Broker-J] XSS vulnerability in path

broker-core/src/main/java/org/apache/qpid/server/util/StringUtil.java

@@ -20,9 +20,15 @@
  */
 package org.apache.qpid.server.util;
 
+import java.io.IOException;
+import java.io.StringWriter;
+import java.io.Writer;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.BitSet;
+import java.util.Comparator;
+import java.util.Map;
 import java.util.Random;
 import java.security.SecureRandom;
 
@@ -33,9 +39,22 @@ public class StringUtil
     private static final String OTHERS = "_-";
     private static final char[] CHARACTERS = (NUMBERS + LETTERS + LETTERS.toUpperCase() + OTHERS).toCharArray();
     private static final char[] HEX = "0123456789ABCDEF".toCharArray();
+    private static final Map<CharSequence, CharSequence> HTML_ESCAPE = Map.of("\"", "&quot;",
+            "&", "&amp;",
+            "<", "&lt;",
+            ">", "&gt;",
+            "'", "&#x27;");
+    private static final BitSet HTML_ESCAPE_PREFIX_SET = new BitSet();
+    private static final int LONGEST_HTML_ESCAPE_ENTRY = HTML_ESCAPE.values().stream()
+            .max(Comparator.comparingInt(CharSequence::length))
+            .get().length();
+    private static final Random RANDOM = new SecureRandom();
 
-
-    private final Random _random = new SecureRandom();
+    static
+    {
+        HTML_ESCAPE.keySet().forEach(key -> HTML_ESCAPE_PREFIX_SET.set(key.charAt(0)));
+
+    }
 
     public static String elideDataUrl(final String path)
     {
@@ -57,7 +76,7 @@ public String randomAlphaNumericString(int maxLength)
         char[] result = new char[maxLength];
         for (int i = 0; i < maxLength; i++)
         {
-            result[i] = (char) CHARACTERS[_random.nextInt(CHARACTERS.length)];
+            result[i] = CHARACTERS[RANDOM.nextInt(CHARACTERS.length)];
         }
         return new String(result);
     }
@@ -69,9 +88,9 @@ public String randomAlphaNumericString(int maxLength)
      * @param managerName
      * @return unique java name
      */
-    public String createUniqueJavaName(String managerName)
+    public String createUniqueJavaName(final String managerName)
     {
-        StringBuilder builder = new StringBuilder();
+        final StringBuilder builder = new StringBuilder();
         boolean initialChar = true;
         for (int i = 0; i < managerName.length(); i++)
         {
@@ -89,7 +108,7 @@ public String createUniqueJavaName(String managerName)
         }
         try
         {
-            byte[] digest = MessageDigest.getInstance("MD5").digest(managerName.getBytes(StandardCharsets.UTF_8));
+            final byte[] digest = MessageDigest.getInstance("MD5").digest(managerName.getBytes(StandardCharsets.UTF_8));
             builder.append(toHex(digest).toLowerCase());
         }
         catch (NoSuchAlgorithmException e)
@@ -99,4 +118,78 @@ public String createUniqueJavaName(String managerName)
         return builder.toString();
     }
 
+    public static String escapeHtml4(final String input)
+    {
+        if (input == null)
+        {
+            return null;
+        }
+        try
+        {
+            final StringWriter writer = new StringWriter(input.length() * 2);
+            translate(input, writer);
+            return writer.toString();
+        }
+        catch (IOException e)
+        {
+            throw new ConnectionScopedRuntimeException(e);
+        }
+    }
+
+    private static void translate(final CharSequence input, final Writer writer) throws IOException
+    {
+        int pos = 0;
+        int len = input.length();
+
+        while (pos < len)
+        {
+            int consumed = translate(input, pos, writer);
+            if (consumed == 0)
+            {
+                char c1 = input.charAt(pos);
+                writer.write(c1);
+                ++pos;
+                if (Character.isHighSurrogate(c1) && pos < len)
+                {
+                    char c2 = input.charAt(pos);
+                    if (Character.isLowSurrogate(c2))
+                    {
+                        writer.write(c2);
+                        ++pos;
+                    }
+                }
+            }
+            else
+            {
+                for (int pt = 0; pt < consumed; ++pt)
+                {
+                    pos += Character.charCount(Character.codePointAt(input, pos));
+                }
+            }
+        }
+    }
+
+    private static int translate(final CharSequence input, final int index, final Writer writer) throws IOException
+    {
+        if (HTML_ESCAPE_PREFIX_SET.get(input.charAt(index)))
+        {
+            int max = LONGEST_HTML_ESCAPE_ENTRY;
+            if (index + LONGEST_HTML_ESCAPE_ENTRY > input.length())
+            {
+                max = input.length() - index;
+            }
+
+            for (int i = max; i >= 1; --i)
+            {
+                final CharSequence subSeq = input.subSequence(index, index + i);
+                final String result = (String) HTML_ESCAPE.get(subSeq.toString());
+                if (result != null)
+                {
+                    writer.write(result);
+                    return Character.codePointCount(subSeq, 0, subSeq.length());
+                }
+            }
+        }
+        return 0;
+    }
 }

broker-core/src/test/java/org/apache/qpid/server/util/StringUtilTest.java

@@ -21,6 +21,7 @@
 package org.apache.qpid.server.util;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -70,4 +71,19 @@ public void testCreateUniqueJavaName()
         assertEquals("97b247ba19ff869340d3797cc73ca065", _util.createUniqueJavaName("1++++----"));
         assertEquals("d41d8cd98f00b204e9800998ecf8427e", _util.createUniqueJavaName(""));
     }
+
+    @Test
+    public void escapeHtml4()
+    {
+        assertNull(StringUtil.escapeHtml4(null));
+        assertEquals("", StringUtil.escapeHtml4(""));
+        assertEquals("test", StringUtil.escapeHtml4("test"));
+        assertEquals("&lt;test&gt;", StringUtil.escapeHtml4("<test>"));
+        assertEquals("&quot;test&quot;", StringUtil.escapeHtml4("\"test\""));
+        assertEquals("&amp;", StringUtil.escapeHtml4("&"));
+        assertEquals("&amp; &amp; &lt; &quot; &gt; &amp; &amp;", StringUtil.escapeHtml4("& & < \" > & &"));
+        assertEquals("&gt;&gt;&gt;", StringUtil.escapeHtml4(">>>"));
+        assertEquals("&lt;&lt;&lt;", StringUtil.escapeHtml4("<<<"));
+        assertEquals("&#x27;&#x27;&#x27;", StringUtil.escapeHtml4("'''"));
+    }
 }

broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/RootServlet.java

@@ -27,6 +27,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 
 import org.apache.qpid.server.management.plugin.HttpManagementUtil;
+import org.apache.qpid.server.util.StringUtil;
 
 public class RootServlet extends HttpServlet
 {
@@ -37,7 +38,7 @@ public class RootServlet extends HttpServlet
     private final String _filename;
 
 
-    public RootServlet(String expectedPath, String apiDocsPath, String filename)
+    public RootServlet(final String expectedPath, final String apiDocsPath, final String filename)
     {
         _expectedPath = expectedPath;
         _apiDocsPath = apiDocsPath;
@@ -47,48 +48,45 @@ public RootServlet(String expectedPath, String apiDocsPath, String filename)
     @Override
     public void init() throws ServletException
     {
-
+        // currently no initialization logic needed
     }
 
     @Override
-    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
+    protected void doGet(final HttpServletRequest request, final HttpServletResponse response)
+            throws ServletException, IOException
     {
         final String path = request.getServletPath();
-        if(_expectedPath == null || _expectedPath.equals(path == null ? "" : path))
+        if (_expectedPath == null || _expectedPath.equals(path == null ? "" : path))
         {
-            try (OutputStream output = HttpManagementUtil.getOutputStream(request, response))
+            try (final OutputStream output = HttpManagementUtil.getOutputStream(request, response);
+                 final InputStream fileInput = getClass().getResourceAsStream("/resources/" + _filename))
             {
-                try (InputStream fileInput = getClass().getResourceAsStream("/resources/" + _filename))
+                if (fileInput == null)
                 {
-                    if (fileInput != null)
-                    {
-                        byte[] buffer = new byte[1024];
-                        response.setStatus(HttpServletResponse.SC_OK);
-                        int read = 0;
+                    final String fileName = StringUtil.escapeHtml4(_filename);
+                    response.sendError(HttpServletResponse.SC_NOT_FOUND, "unknown file: " + fileName);
+                    return;
+                }
+
+                final byte[] buffer = new byte[1024];
+                response.setStatus(HttpServletResponse.SC_OK);
+                int read;
 
-                        while ((read = fileInput.read(buffer)) > 0)
-                        {
-                            output.write(buffer, 0, read);
-                        }
-                    }
-                    else
-                    {
-                        response.sendError(HttpServletResponse.SC_NOT_FOUND, "unknown file: " + _filename);
-                    }
+                while ((read = fileInput.read(buffer)) > 0)
+                {
+                    output.write(buffer, 0, read);
                 }
             }
         }
         else
         {
-
             response.setStatus(HttpServletResponse.SC_NOT_FOUND);
-            try (OutputStream output = HttpManagementUtil.getOutputStream(request, response))
+            try (final OutputStream output = HttpManagementUtil.getOutputStream(request, response))
             {
-                final String notFoundMessage = "Unknown path '"
-                                 + request.getServletPath()
-                                 + "'. Please read the api docs at "
-                                 + request.getScheme()
-                                 + "://" + request.getServerName() + ":" + request.getServerPort() + _apiDocsPath + "\n";
+                final String servletPath = StringUtil.escapeHtml4(request.getServletPath());
+                final String notFoundMessage = "Unknown path \"" + servletPath +
+                        "\". Please read the api docs at " + request.getScheme() + "://" + request.getServerName() +
+                        ":" + request.getServerPort() + "/" + _apiDocsPath + "\n";
                 output.write(notFoundMessage.getBytes(StandardCharsets.UTF_8));
             }
         }