merging 915269,915384 - https://issues.apache.org/activemq/browse/AMQ…

…-2613 - XSS and CSRF vunerabilities git-svn-id: https://svn.apache.org/repos/asf/activemq/branches/activemq-5.3@915387 13f79535-47bb-0310-9956-ffa450edef68
apache · Feb 23, 2010 · 1f464b9 · 1f464b9
1 parent e440e90
commit 1f464b9
Show file tree

Hide file tree

Showing 18 changed files with 82 additions and 34 deletions.
diff --git a/activemq-web-console/src/main/java/org/apache/activemq/web/controller/CreateDestination.java b/activemq-web-console/src/main/java/org/apache/activemq/web/controller/CreateDestination.java
@@ -39,4 +39,10 @@ public ModelAndView handleRequest(HttpServletRequest request, HttpServletRespons
         return redirectToBrowseView();
     }
 
+	public String[] getSupportedHttpMethods() {
+		return new String[]{"POST"};
+	}
+
+
+
 }
diff --git a/activemq-web-console/src/main/java/org/apache/activemq/web/controller/CreateSubscriber.java b/activemq-web-console/src/main/java/org/apache/activemq/web/controller/CreateSubscriber.java
@@ -50,5 +50,9 @@ public ModelAndView handleRequest(HttpServletRequest request, HttpServletRespons
         getBrokerAdmin().createDurableSubscriber(getClientId(), getSubscriberName(), getValidDestination(), selector);
         return new ModelAndView("redirect:subscribers.jsp");
     }
+
+	public String[] getSupportedHttpMethods() {
+		return new String[]{"POST"};
+	}
 
 }
diff --git a/activemq-web-console/src/main/java/org/apache/activemq/web/controller/SendMessage.java b/activemq-web-console/src/main/java/org/apache/activemq/web/controller/SendMessage.java
@@ -209,4 +209,8 @@ protected boolean isValidPropertyName(String name) {
         // allow JMSX extensions or non JMS properties
         return name.startsWith("JMSX") || !name.startsWith("JMS");
     }
+
+	public String[] getSupportedHttpMethods() {
+		return new String[]{"POST"};
+	}
 }
diff --git a/...nsole/src/main/java/org/apache/activemq/web/handler/BindingBeanNameUrlHandlerMapping.java b/...nsole/src/main/java/org/apache/activemq/web/handler/BindingBeanNameUrlHandlerMapping.java
@@ -16,8 +16,12 @@
  */
 package org.apache.activemq.web.handler;
 
+import java.util.Arrays;
+import java.util.UUID;
+
 import javax.servlet.http.HttpServletRequest;
 
+import org.apache.activemq.web.DestinationFacade;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.web.bind.ServletRequestDataBinder;
@@ -41,8 +45,21 @@ protected Object getHandlerInternal(HttpServletRequest request) throws Exception
             HandlerExecutionChain handlerExecutionChain = (HandlerExecutionChain) object;
             object = handlerExecutionChain.getHandler();
         }
-
+        
         if (object != null) {
+        	// prevent CSRF attacks
+        	if (object instanceof DestinationFacade) {
+        		// check supported methods
+        		if (!Arrays.asList(((DestinationFacade)object).getSupportedHttpMethods()).contains(request.getMethod())) {
+        			throw new UnsupportedOperationException("Unsupported method " + request.getMethod() + " for path " + request.getRequestURI());
+        		}
+        		// check the 'secret'
+        		if (!request.getSession().getAttribute("secret").equals(request.getParameter("secret"))) {
+        			throw new UnsupportedOperationException("Possible CSRF attack");
+        		}
+        	}
+
+
             ServletRequestDataBinder binder = new ServletRequestDataBinder(object, "request");
             try {
                 binder.bind(request);
@@ -56,6 +73,7 @@ protected Object getHandlerInternal(HttpServletRequest request) throws Exception
                 throw e;
             }
         }
+
         return object;
     }
 }
diff --git a/activemq-web-console/src/main/webapp/WEB-INF/tags/form/short.tag b/activemq-web-console/src/main/webapp/WEB-INF/tags/form/short.tag
@@ -17,6 +17,8 @@
 <%@ attribute name="text" type="java.lang.String" required="true"  %>
 <%@ attribute name="length" type="java.lang.Integer" required="false" %>
 <%
+ text = org.apache.commons.lang.StringEscapeUtils.escapeHtml(text);
+ text = org.apache.commons.lang.StringEscapeUtils.escapeJavaScript(text);
  if (length == null)
     length = 20;
  if (text.length() <= 20) {

diff --git a/activemq-web-console/src/main/webapp/WEB-INF/tags/form/text.tag b/activemq-web-console/src/main/webapp/WEB-INF/tags/form/text.tag
@@ -19,10 +19,12 @@
 <%
     String value = request.getParameter(name);
     if (value == null || value.trim().length() == 0) {
-    		value = defaultValue;
-		}
-		if (value == null) {
-			value = "";
-		}
+    	value = defaultValue;
+	}
+	if (value == null) {
+		value = "";
+	}
+	value = org.apache.commons.lang.StringEscapeUtils.escapeHtml(value);
+
 %>
 <input type="text" name="${name}" value="<%= value %>"/>
diff --git a/activemq-web-console/src/main/webapp/WEB-INF/web.xml b/activemq-web-console/src/main/webapp/WEB-INF/web.xml
@@ -50,7 +50,7 @@
 
   <filter-mapping>
     <filter-name>spring</filter-name>
-    <url-pattern>/*</url-pattern>
+    <url-pattern>*.jsp</url-pattern>
   </filter-mapping>
 
   <!-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -->
@@ -101,7 +101,7 @@
 
   <filter-mapping>
     <filter-name>session</filter-name>
-    <url-pattern>/*</url-pattern>
+    <url-pattern>*.jsp</url-pattern>
   </filter-mapping>
   <filter-mapping>
     <filter-name>spring-rq</filter-name>

diff --git a/activemq-web-console/src/main/webapp/browse.jsp b/activemq-web-console/src/main/webapp/browse.jsp
@@ -39,7 +39,7 @@
 <tbody>
 <jms:forEachMessage queueBrowser="${requestContext.queueBrowser.browser}" var="row">
 <tr>
-<td><a href="message.jsp?id=${row.JMSMessageID}&JMSDestination=${requestContext.queueBrowser.JMSDestination}" 
+<td><a href="message.jsp?id=${row.JMSMessageID}&JMSDestination=<c:out value="${requestContext.queueBrowser.JMSDestination}" />" 
     title="${row.properties}">${row.JMSMessageID}</a></td>
 <td>${row.JMSCorrelationID}</td>
 <td><jms:persistent message="${row}"/></td>
@@ -49,15 +49,15 @@
 <td><jms:formatTimestamp timestamp="${row.JMSTimestamp}"/></td>
 <td>${row.JMSType}</td>
 <td>
-    <a href="deleteMessage.action?JMSDestination=${row.JMSDestination}&messageId=${row.JMSMessageID}">Delete</a>
+    <a href="deleteMessage.action?JMSDestination=<c:out value="${row.JMSDestination}"/>&messageId=${row.JMSMessageID}&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a>
 </td>
 </tr>
 </jms:forEachMessage>
 </tbody>
 </table>
 
 <div>
-<a href="queueConsumers.jsp?JMSDestination=${requestContext.queueBrowser.JMSDestination}">View Consumers</a>
+<a href="queueConsumers.jsp?JMSDestination=<c:out value="${requestContext.queueBrowser.JMSDestination}"/>">View Consumers</a>
 </div>
 </body>
 </html>

diff --git a/activemq-web-console/src/main/webapp/graph.jsp b/activemq-web-console/src/main/webapp/graph.jsp
@@ -51,7 +51,7 @@
 <td>${row.JMSTimestamp}</td>
 <td>${row.JMSType}</td>
 <td>
-    <a href="deleteDestination.action?destination=${row.JMSMessageID}">Delete</a>
+    <a href="deleteDestination.action?destination=${row.JMSMessageID}&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a>
 </td>
 </tr>
 </jms:forEachMessage>

diff --git a/activemq-web-console/src/main/webapp/message.jsp b/activemq-web-console/src/main/webapp/message.jsp
@@ -130,24 +130,24 @@ No message could be found for ID ${requestContext.messageQuery.id}
                 </thead>
                 <tbody>
                     <tr>
-                        <td colspan="2"><a href="deleteMessage.action?JMSDestination=${row.JMSDestination}&messageId=${row.JMSMessageID}">Delete</a></td>
+                        <td colspan="2"><a href="deleteMessage.action?JMSDestination=<c:out value="${row.JMSDestination}" />&messageId=${row.JMSMessageID}&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a></td>
                     </tr>
                     <tr class="odd">
-                    <td><a href="javascript:confirmAction('queue', 'copyMessage.action?destination=%target%&JMSDestination=${row.JMSDestination}&messageId=${row.JMSMessageID}&JMSDestinationType=queue')">Copy</a></td>
+                    <td><a href="javascript:confirmAction('queue', 'copyMessage.action?destination=%target%&JMSDestination=<c:out value="${row.JMSDestination}" />&messageId=${row.JMSMessageID}&JMSDestinationType=queue&secret=<c:out value='${sessionScope["secret"]}'/>')">Copy</a></td>
                         <td rowspan="2">
                             <select id="queue">
                                 <option value=""> -- Please select --</option>
                                 <c:forEach items="${requestContext.brokerQuery.queues}" var="queues">
                                     <c:if test="${queues.name != requestContext.messageQuery.JMSDestination}">
-                                    <option value="${queues.name}"><form:short text="${queues.name}"/></option>
+                                    <option value="<c:out value="${queues.name}" />"><form:short text="${queues.name}"/></option>
                                     </c:if>
                                 </c:forEach>
                             </select>
                         </td>
 
                     </tr>
                     <tr class="odd">
-                        <td><a href="javascript:confirmAction('queue', 'moveMessage.action?destination=%target%&JMSDestination=${row.JMSDestination}&messageId=${row.JMSMessageID}&JMSDestinationType=queue')">Move</a></td>
+                        <td><a href="javascript:confirmAction('queue', 'moveMessage.action?destination=%target%&JMSDestination=<c:out value="${row.JMSDestination}" />&messageId=${row.JMSMessageID}&JMSDestinationType=queue&secret=<c:out value='${sessionScope["secret"]}'/>')">Move</a></td>
                     </tr>
                 </tbody>
             </table>

diff --git a/activemq-web-console/src/main/webapp/queueConsumers.jsp b/activemq-web-console/src/main/webapp/queueConsumers.jsp
@@ -16,11 +16,11 @@
 --%>
 <html>
 <head>
-<title>Consumers for ${requestContext.queueConsumerQuery.JMSDestination}</title>
+<title>Consumers for <c:out value="${requestContext.queueConsumerQuery.JMSDestination}" /></title>
 </head>
 <body>
 
-<h2>Active Consumers for ${requestContext.queueConsumerQuery.JMSDestination}</h2>
+<h2>Active Consumers for <c:out value="${requestContext.queueConsumerQuery.JMSDestination}" /></h2>
 
 <table id="messages" class="sortable autostripe">
 <thead>

diff --git a/activemq-web-console/src/main/webapp/queues.jsp b/activemq-web-console/src/main/webapp/queues.jsp
@@ -21,8 +21,9 @@
 <body>
 
 <div>
-<form action="createDestination.action" method="get">
+<form action="createDestination.action" method="post">
     <input type="hidden" name="JMSDestinationType" value="queue"/>
+    <input type="hidden" name="secret" value="<c:out value='${sessionScope["secret"]}'/>"/>
 
     <label name="destination">Queue Name</label>
     <input type="text" name="JMSDestination" value=""/>
@@ -48,22 +49,23 @@
 </thead>
 <tbody>
 <c:forEach items="${requestContext.brokerQuery.queues}" var="row">
+
 <tr>
-<td><a href="browse.jsp?JMSDestination=${row.name}"><form:tooltip text="${row.name}" length="50"/></a></td>
+<td><a href="browse.jsp?JMSDestination=<c:out value="${row.name}" />"><form:tooltip text="${row.name}" length="50"/></a></td>
 <td>${row.queueSize}</td>
 <td>${row.consumerCount}</td>
 <td>${row.enqueueCount}</td>
 <td>${row.dequeueCount}</td>
 <td>
-    <a href="browse.jsp?JMSDestination=${row.name}">Browse</a>
-	<a href="queueConsumers.jsp?JMSDestination=${row.name}">Active Consumers</a><br/>
-    <a href="queueBrowse/${row.name}?view=rss&feedType=atom_1.0" title="Atom 1.0"><img src="images/feed_atom.png"/></a>
-    <a href="queueBrowse/${row.name}?view=rss&feedType=rss_2.0" title="RSS 2.0"><img src="images/feed_rss.png"/></a>
+    <a href="browse.jsp?JMSDestination=<c:out value="${row.name}" />">Browse</a>
+	<a href="queueConsumers.jsp?JMSDestination=<c:out value="${row.name}" />">Active Consumers</a><br/>
+    <a href="queueBrowse/<c:out value="${row.name}" />?view=rss&feedType=atom_1.0" title="Atom 1.0"><img src="images/feed_atom.png"/></a>
+    <a href="queueBrowse/<c:out value="${row.name}" />?view=rss&feedType=rss_2.0" title="RSS 2.0"><img src="images/feed_rss.png"/></a>
 </td>
 <td>
-    <a href="send.jsp?JMSDestination=${row.name}&JMSDestinationType=queue">Send To</a>
-    <a href="purgeDestination.action?JMSDestination=${row.name}&JMSDestinationType=queue">Purge</a>
-    <a href="deleteDestination.action?JMSDestination=${row.name}&JMSDestinationType=queue">Delete</a>
+    <a href="send.jsp?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=queue">Send To</a>
+    <a href="purgeDestination.action?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=queue&secret=<c:out value='${sessionScope["secret"]}'/>">Purge</a>
+    <a href="deleteDestination.action?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=queue&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a>
 </td>
 </tr>
 </c:forEach>

diff --git a/activemq-web-console/src/main/webapp/send.jsp b/activemq-web-console/src/main/webapp/send.jsp
@@ -23,6 +23,7 @@
 <h2>Send a JMS Message</h2>
 
 <form action="sendMessage.action" method="post">
+<input type="hidden" name="secret" value="<c:out value='${sessionScope["secret"]}'/>"/>
 
 <table id="headers" class="autostripe">
 <thead>
@@ -37,7 +38,7 @@
 	    <label for="JMSDestination">Destination</label>
 	</td>
 	<td>
-	    <form:text name="JMSDestination"/>
+	    <form:text name="JMSDestination" defaultValue="foo.bar" />
 	</td>
 	<td class="label">
 	    <label for="queue">Queue or Topic</label>

diff --git a/activemq-web-console/src/main/webapp/subscribers.jsp b/activemq-web-console/src/main/webapp/subscribers.jsp
@@ -20,8 +20,9 @@
 </head>
 <body>
 
-<form action="createSubscriber.action" method="get">
+<form action="createSubscriber.action" method="post">
     <input type="hidden" name="JMSDestinationType" value="topic"/>
+    <input type="hidden" name="secret" value="<c:out value='${sessionScope["secret"]}'/>"/>
 
 <table id="createSubscribers" class="sortable autostripe">
 <thead>
@@ -102,7 +103,7 @@
 <td>${row.enqueueCounter}</td>
 <td>${row.dequeueCounter}</td>
 <td>
-    <a href="deleteSubscriber.action?clientId=${row.clientId}&subscriberName=${row.subscriptionName}">Delete</a>
+    <a href="deleteSubscriber.action?clientId=${row.clientId}&subscriberName=${row.subscriptionName}&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a>
 </td>
 </tr>
 </c:forEach>

diff --git a/activemq-web-console/src/main/webapp/topics.jsp b/activemq-web-console/src/main/webapp/topics.jsp
@@ -23,6 +23,7 @@
 <div>
 <form action="createDestination.action" method="get">
     <input type="hidden" name="JMSDestinationType" value="topic"/>
+    <input type="hidden" name="secret" value="<c:out value='${sessionScope["secret"]}'/>"/>
 
     <label name="destination">Topic Name</label>
     <input type="text" name="JMSDestination" value=""/>
@@ -46,13 +47,13 @@
 <tbody>
 <c:forEach items="${requestContext.brokerQuery.topics}" var="row">
 <tr>
-<td><a href="send.jsp?JMSDestination=${row.name}&JMSDestinationType=topic"><form:tooltip text="${row.name}" length="50"/></a></td>
+<td><a href="send.jsp?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=topic"><form:tooltip text="${row.name}" length="50"/></a></td>
 <td>${row.consumerCount}</td>
 <td>${row.enqueueCount}</td>
 <td>${row.dequeueCount}</td>
 <td>
-    <a href="send.jsp?JMSDestination=${row.name}&JMSDestinationType=topic">Send To</a>
-    <a href="deleteDestination.action?JMSDestination=${row.name}&JMSDestinationType=topic">Delete</a>
+    <a href="send.jsp?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=topic">Send To</a>
+    <a href="deleteDestination.action?JMSDestination=<c:out value="${row.name}" />&JMSDestinationType=topic&secret=<c:out value='${sessionScope["secret"]}'/>">Delete</a>
 </td>
 </tr>
 </c:forEach>

diff --git a/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java b/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java
@@ -172,6 +172,7 @@ public Collection<NetworkConnectorViewMBean> getNetworkConnectors() throws Excep
     @SuppressWarnings("unchecked")
     public Collection<SubscriptionViewMBean> getQueueConsumers(String queueName) throws Exception {
         String brokerName = getBrokerName();
+        queueName = StringUtils.replace(queueName, "\"", "_");
         ObjectName query = new ObjectName("org.apache.activemq:BrokerName=" + brokerName
                 + ",Type=Subscription,destinationType=Queue,destinationName=" + queueName + ",*");
         Set<ObjectName> queryResult = getManagementContext().queryNames(query, null);

diff --git a/activemq-web/src/main/java/org/apache/activemq/web/DestinationFacade.java b/activemq-web/src/main/java/org/apache/activemq/web/DestinationFacade.java
@@ -128,4 +128,8 @@ protected ModelAndView redirectToBrowseView() {
     protected String getPhysicalDestinationName() {
         return createDestination().getPhysicalName();
     }
+
+    public String[] getSupportedHttpMethods() {
+    	return new String[]{"GET", "POST"};
+    }
 }
diff --git a/activemq-web/src/main/java/org/apache/activemq/web/SessionFilter.java b/activemq-web/src/main/java/org/apache/activemq/web/SessionFilter.java
@@ -18,6 +18,7 @@
 package org.apache.activemq.web;
 
 import java.io.IOException;
+import java.util.UUID;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -39,7 +40,8 @@ public void init(FilterConfig filterConfig) throws ServletException {
     }
 
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-        ((HttpServletRequest)request).getSession(true);
+    	// set secret to prevent CSRF attacks
+        ((HttpServletRequest)request).getSession(true).setAttribute("secret", UUID.randomUUID().toString());;
         chain.doFilter(request, response);
     }