From 1f464b9412e1b1c08d40c8ffac40edd52731da48 Mon Sep 17 00:00:00 2001 From: Bosanac Dejan Date: Tue, 23 Feb 2010 15:38:26 +0000 Subject: [PATCH] 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 --- .../web/controller/CreateDestination.java | 6 ++++++ .../web/controller/CreateSubscriber.java | 4 ++++ .../activemq/web/controller/SendMessage.java | 4 ++++ .../BindingBeanNameUrlHandlerMapping.java | 20 ++++++++++++++++++- .../main/webapp/WEB-INF/tags/form/short.tag | 2 ++ .../main/webapp/WEB-INF/tags/form/text.tag | 12 ++++++----- .../src/main/webapp/WEB-INF/web.xml | 4 ++-- .../src/main/webapp/browse.jsp | 6 +++--- .../src/main/webapp/graph.jsp | 2 +- .../src/main/webapp/message.jsp | 8 ++++---- .../src/main/webapp/queueConsumers.jsp | 4 ++-- .../src/main/webapp/queues.jsp | 20 ++++++++++--------- activemq-web-console/src/main/webapp/send.jsp | 3 ++- .../src/main/webapp/subscribers.jsp | 5 +++-- .../src/main/webapp/topics.jsp | 7 ++++--- .../activemq/web/BrokerFacadeSupport.java | 1 + .../activemq/web/DestinationFacade.java | 4 ++++ .../apache/activemq/web/SessionFilter.java | 4 +++- 18 files changed, 82 insertions(+), 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 index 7e1c7eebd75..c2520422564 100644 --- 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 index b9055e11880..fb87ae3595d 100644 --- 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 index 79003d7a1fc..037927b7ca4 100644 --- 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/activemq-web-console/src/main/java/org/apache/activemq/web/handler/BindingBeanNameUrlHandlerMapping.java b/activemq-web-console/src/main/java/org/apache/activemq/web/handler/BindingBeanNameUrlHandlerMapping.java index 399ca509bb7..6f899286a06 100644 --- a/activemq-web-console/src/main/java/org/apache/activemq/web/handler/BindingBeanNameUrlHandlerMapping.java +++ b/activemq-web-console/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 index 3302e442938..ed82a97e172 100644 --- 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 index a557a42bb26..521bc6da9aa 100644 --- 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); + %> diff --git a/activemq-web-console/src/main/webapp/WEB-INF/web.xml b/activemq-web-console/src/main/webapp/WEB-INF/web.xml index dc96d1af9ca..5bc4eefcdbf 100755 --- 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 @@ spring - /* + *.jsp @@ -101,7 +101,7 @@ session - /* + *.jsp spring-rq diff --git a/activemq-web-console/src/main/webapp/browse.jsp b/activemq-web-console/src/main/webapp/browse.jsp index e2b76558a10..6e7c4ec05c6 100644 --- a/activemq-web-console/src/main/webapp/browse.jsp +++ b/activemq-web-console/src/main/webapp/browse.jsp @@ -39,7 +39,7 @@ -" title="${row.properties}">${row.JMSMessageID} ${row.JMSCorrelationID} @@ -49,7 +49,7 @@ ${row.JMSType} - Delete + &messageId=${row.JMSMessageID}&secret=">Delete @@ -57,7 +57,7 @@
-View Consumers +">View Consumers
diff --git a/activemq-web-console/src/main/webapp/graph.jsp b/activemq-web-console/src/main/webapp/graph.jsp index 21805f865b8..8736b9c12fa 100644 --- a/activemq-web-console/src/main/webapp/graph.jsp +++ b/activemq-web-console/src/main/webapp/graph.jsp @@ -51,7 +51,7 @@ ${row.JMSTimestamp} ${row.JMSType} - Delete + ">Delete diff --git a/activemq-web-console/src/main/webapp/message.jsp b/activemq-web-console/src/main/webapp/message.jsp index 75d18cef3d8..b5db6aec1a5 100644 --- a/activemq-web-console/src/main/webapp/message.jsp +++ b/activemq-web-console/src/main/webapp/message.jsp @@ -130,16 +130,16 @@ No message could be found for ID ${requestContext.messageQuery.id} - Delete + &messageId=${row.JMSMessageID}&secret=">Delete - Copy + &messageId=${row.JMSMessageID}&JMSDestinationType=queue&secret=')">Copy @@ -147,7 +147,7 @@ No message could be found for ID ${requestContext.messageQuery.id} - Move + &messageId=${row.JMSMessageID}&JMSDestinationType=queue&secret=')">Move diff --git a/activemq-web-console/src/main/webapp/queueConsumers.jsp b/activemq-web-console/src/main/webapp/queueConsumers.jsp index d38fbfd5f46..862c6075bf4 100644 --- a/activemq-web-console/src/main/webapp/queueConsumers.jsp +++ b/activemq-web-console/src/main/webapp/queueConsumers.jsp @@ -16,11 +16,11 @@ --%> -Consumers for ${requestContext.queueConsumerQuery.JMSDestination} +Consumers for <c:out value="${requestContext.queueConsumerQuery.JMSDestination}" /> -

Active Consumers for ${requestContext.queueConsumerQuery.JMSDestination}

+

Active Consumers for

diff --git a/activemq-web-console/src/main/webapp/queues.jsp b/activemq-web-console/src/main/webapp/queues.jsp index bb68e0ac063..ab535e471dd 100644 --- a/activemq-web-console/src/main/webapp/queues.jsp +++ b/activemq-web-console/src/main/webapp/queues.jsp @@ -21,8 +21,9 @@
-
+ + "/> @@ -48,22 +49,23 @@
+ - + diff --git a/activemq-web-console/src/main/webapp/send.jsp b/activemq-web-console/src/main/webapp/send.jsp index ddcfcbe34ca..056c0a876a3 100644 --- a/activemq-web-console/src/main/webapp/send.jsp +++ b/activemq-web-console/src/main/webapp/send.jsp @@ -23,6 +23,7 @@

Send a JMS Message

+"/>
"> ${row.queueSize} ${row.consumerCount} ${row.enqueueCount} ${row.dequeueCount} - Browse - Active Consumers
- - + ">Browse + ">Active Consumers
+ ?view=rss&feedType=atom_1.0" title="Atom 1.0"> + ?view=rss&feedType=rss_2.0" title="RSS 2.0"> 		- Send To - Purge - Delete + &JMSDestinationType=queue">Send To + &JMSDestinationType=queue&secret=">Purge + &JMSDestinationType=queue&secret=">Delete
@@ -37,7 +38,7 @@
- + diff --git a/activemq-web-console/src/main/webapp/subscribers.jsp b/activemq-web-console/src/main/webapp/subscribers.jsp index 0dd16097c9b..843a035d80a 100644 --- a/activemq-web-console/src/main/webapp/subscribers.jsp +++ b/activemq-web-console/src/main/webapp/subscribers.jsp @@ -20,8 +20,9 @@ - + + "/> @@ -102,7 +103,7 @@ diff --git a/activemq-web-console/src/main/webapp/topics.jsp b/activemq-web-console/src/main/webapp/topics.jsp index 40f6a8514e0..310333dd874 100644 --- a/activemq-web-console/src/main/webapp/topics.jsp +++ b/activemq-web-console/src/main/webapp/topics.jsp @@ -23,6 +23,7 @@
+ "/> @@ -46,13 +47,13 @@
- + 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 index 4b6bd88d2f3..f0273797937 100644 --- 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 getNetworkConnectors() throws Excep @SuppressWarnings("unchecked") public Collection 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 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 index 8cfb0ee3994..b7dfbf5948c 100644 --- 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 index 120f4325795..9a698836885 100644 --- 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); }
${row.enqueueCounter} ${row.dequeueCounter} - Delete + ">Delete
&JMSDestinationType=topic"> ${row.consumerCount} ${row.enqueueCount} ${row.dequeueCount} - Send To - Delete + &JMSDestinationType=topic">Send To + &JMSDestinationType=topic&secret=">Delete