This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push:
new 34cdda2 Security hardening. Avoid loading user specified classes.
34cdda2 is described below
commit 34cdda2272a4d6cce6a9c1378cfaf596ef067f07
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Apr 18 19:32:57 2019 +0100
Security hardening. Avoid loading user specified classes.
The user is trusted so loading a user specified class should be safe but
given it isn't necessary to load the class, avoid doing so. Reported by
Coverity scan.
---
.../apache/catalina/manager/ManagerServlet.java | 150 +++++++++++----------
.../org/apache/tomcat/util/IntrospectionUtils.java | 22 +++
.../apache/tomcat/util/TestIntrospectionUtils.java | 112 +++++++++++++++
webapps/docs/changelog.xml | 4 +
4 files changed, 220 insertions(+), 68 deletions(-)
diff --git a/java/org/apache/catalina/manager/ManagerServlet.java
b/java/org/apache/catalina/manager/ManagerServlet.java
index 0e43f5f..12911ba 100644
--- a/java/org/apache/catalina/manager/ManagerServlet.java
+++ b/java/org/apache/catalina/manager/ManagerServlet.java
@@ -5,9 +5,9 @@
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -58,6 +58,7 @@ import org.apache.catalina.util.RequestUtil;
import org.apache.catalina.util.ServerInfo;
import org.apache.tomcat.util.Diagnostics;
import org.apache.tomcat.util.ExceptionUtils;
+import org.apache.tomcat.util.IntrospectionUtils;
import org.apache.tomcat.util.modeler.Registry;
import org.apache.tomcat.util.res.StringManager;
@@ -207,7 +208,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
*/
protected transient Host host = null;
-
+
/**
* The host appBase.
* @deprecated Unused
@@ -226,7 +227,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
* The associated deployer ObjectName.
*/
protected ObjectName oname = null;
-
+
/**
* The global JNDI <code>NamingContext</code> for this server,
@@ -290,7 +291,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
// Retrieve the MBean server
mBeanServer = Registry.getRegistry(null, null).getMBeanServer();
-
+
}
@@ -339,11 +340,11 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
String war = request.getParameter("war");
String tag = request.getParameter("tag");
boolean update = false;
- if ((request.getParameter("update") != null)
+ if ((request.getParameter("update") != null)
&& (request.getParameter("update").equals("true"))) {
update = true;
}
-
+
boolean statusLine = false;
if ("true".equals(request.getParameter("statusLine"))) {
statusLine = true;
@@ -435,7 +436,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
}
String tag = request.getParameter("tag");
boolean update = false;
- if ((request.getParameter("update") != null)
+ if ((request.getParameter("update") != null)
&& (request.getParameter("update").equals("true"))) {
update = true;
}
@@ -543,15 +544,15 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
*/
protected void findleaks(boolean statusLine, PrintWriter writer,
StringManager smClient) {
-
+
if (!(host instanceof StandardHost)) {
writer.println(smClient.getString("managerServlet.findleaksFail"));
return;
}
-
+
String[] results =
((StandardHost) host).findReloadedContextMemoryLeaks();
-
+
if (results.length > 0) {
if (statusLine) {
writer.println(
@@ -594,7 +595,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
/**
* Store server configuration.
- *
+ *
* @param path Optional context path to save
*/
protected synchronized void save(PrintWriter writer, String path,
@@ -745,7 +746,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
e.toString()));
return;
}
-
+
writeDeployResult(writer, smClient, name, displayPath);
}
@@ -772,7 +773,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
String baseName = cn.getBaseName();
String name = cn.getName();
String displayPath = cn.getDisplayName();
-
+
// Find the local WAR file
File localWar = new File(new File(versioned, tag), baseName + ".war");
@@ -803,7 +804,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
e.toString()));
return;
}
-
+
writeDeployResult(writer, smClient, name, displayPath);
}
@@ -821,14 +822,14 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
*/
protected void deploy(PrintWriter writer, String config, ContextName cn,
String war, boolean update, StringManager smClient) {
-
+
if (config != null && config.length() == 0) {
config = null;
}
if (war != null && war.length() == 0) {
war = null;
}
-
+
if (debug >= 1) {
if (config != null && config.length() > 0) {
if (war != null) {
@@ -847,7 +848,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
}
}
}
-
+
if (!validateContextName(cn, writer, smClient)) {
return;
}
@@ -855,7 +856,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
String name = cn.getName();
String baseName = cn.getBaseName();
String displayPath = cn.getDisplayName();
-
+
// If app exists deployment can only proceed if update is true
// Note existing files will be deleted and then replaced
Context context = (Context) host.findChild(name);
@@ -864,14 +865,14 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
displayPath));
return;
}
-
+
if (config != null && (config.startsWith("file:"))) {
config = config.substring("file:".length());
}
if (war != null && (war.startsWith("file:"))) {
war = war.substring("file:".length());
}
-
+
try {
if (isServiced(name)) {
writer.println(smClient.getString("managerServlet.inService",
displayPath));
@@ -919,7 +920,7 @@ public class ManagerServlet extends HttpServlet implements
ContainerServlet {
writer.println(smClient.getString("managerServlet.exception",
t.toString()));
}
-
+
}
@@ -1051,44 +1052,58 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
writer.println(smClient.getString("managerServlet.resourcesAll"));
}
- Class<?> clazz = null;
- try {
- if (type != null) {
- clazz = Class.forName(type);
- }
- } catch (Throwable t) {
- ExceptionUtils.handleThrowable(t);
- log("ManagerServlet.resources[" + type + "]", t);
- writer.println(smClient.getString("managerServlet.exception",
- t.toString()));
- return;
- }
+ printResources(writer, "", global, type, smClient);
+
+ }
- printResources(writer, "", global, type, clazz, smClient);
+ /**
+ * List the resources of the given context.
+ *
+ * @param writer Writer to render to
+ * @param prefix Path for recursion
+ * @param namingContext The naming context for lookups
+ * @param type Fully qualified class name of the resource type of interest,
+ * or <code>null</code> to list resources of all types
+ * @param clazz Unused
+ * @param smClient i18n support for current client's locale
+ *
+ * @deprecated Use {@link #printResources(PrintWriter, String,
+ * javax.naming.Context, String, StringManager)}
+ * This method will be removed in Tomcat 10.x onwards
+ */
+ @Deprecated
+ protected void printResources(PrintWriter writer, String prefix,
+ javax.naming.Context namingContext,
+ String type, Class<?> clazz, StringManager smClient) {
+ printResources(writer, prefix, namingContext, type, smClient);
}
/**
* List the resources of the given context.
+ * @param writer Writer to render to
+ * @param prefix Path for recursion
+ * @param namingContext The naming context for lookups
+ * @param type Fully qualified class name of the resource type of interest,
+ * or <code>null</code> to list resources of all types
+ * @param smClient i18n support for current client's locale
*/
protected void printResources(PrintWriter writer, String prefix,
javax.naming.Context namingContext,
- String type, Class<?> clazz,
+ String type,
StringManager smClient) {
-
try {
NamingEnumeration<Binding> items = namingContext.listBindings("");
while (items.hasMore()) {
Binding item = items.next();
- if (item.getObject() instanceof javax.naming.Context) {
- printResources
- (writer, prefix + item.getName() + "/",
- (javax.naming.Context) item.getObject(), type, clazz,
- smClient);
+ Object obj = item.getObject();
+ if (obj instanceof javax.naming.Context) {
+ printResources(writer, prefix + item.getName() + "/",
+ (javax.naming.Context) obj, type, smClient);
} else {
- if ((clazz != null) &&
- (!(clazz.isInstance(item.getObject())))) {
+ if (type != null && (obj == null ||
+ !IntrospectionUtils.isInstance(obj.getClass(),
type))) {
continue;
}
writer.print(prefix + item.getName());
@@ -1104,7 +1119,6 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
writer.println(smClient.getString("managerServlet.exception",
t.toString()));
}
-
}
@@ -1162,7 +1176,7 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
if(manager == null) {
writer.println(smClient.getString("managerServlet.noManager",
RequestUtil.filter(displayPath)));
- return;
+ return;
}
int maxCount = 60;
int histoInterval = 1;
@@ -1301,7 +1315,7 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
try {
Context context = (Context) host.findChild(cn.getName());
if (context == null) {
- writer.println(smClient.getString("managerServlet.noContext",
+ writer.println(smClient.getString("managerServlet.noContext",
RequestUtil.filter(displayPath)));
return;
}
@@ -1455,7 +1469,7 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
/**
* Return a File object representing the "application root" directory
* for our associated Host.
- *
+ *
* @deprecated Unused
*/
@Deprecated
@@ -1482,61 +1496,61 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
/**
* Invoke the isDeployed method on the deployer.
*/
- protected boolean isDeployed(String name)
+ protected boolean isDeployed(String name)
throws Exception {
String[] params = { name };
String[] signature = { "java.lang.String" };
- Boolean result =
+ Boolean result =
(Boolean) mBeanServer.invoke(oname, "isDeployed", params,
signature);
return result.booleanValue();
}
-
+
/**
* Invoke the check method on the deployer.
*/
- protected void check(String name)
+ protected void check(String name)
throws Exception {
String[] params = { name };
String[] signature = { "java.lang.String" };
mBeanServer.invoke(oname, "check", params, signature);
}
-
+
/**
* Invoke the isServiced method on the deployer.
*/
- protected boolean isServiced(String name)
+ protected boolean isServiced(String name)
throws Exception {
String[] params = { name };
String[] signature = { "java.lang.String" };
- Boolean result =
+ Boolean result =
(Boolean) mBeanServer.invoke(oname, "isServiced", params,
signature);
return result.booleanValue();
}
-
+
/**
* Invoke the addServiced method on the deployer.
*/
- protected void addServiced(String name)
+ protected void addServiced(String name)
throws Exception {
String[] params = { name };
String[] signature = { "java.lang.String" };
mBeanServer.invoke(oname, "addServiced", params, signature);
}
-
+
/**
* Invoke the removeServiced method on the deployer.
*/
- protected void removeServiced(String name)
+ protected void removeServiced(String name)
throws Exception {
String[] params = { name };
String[] signature = { "java.lang.String" };
mBeanServer.invoke(oname, "removeServiced", params, signature);
}
-
+
/**
* Delete the specified directory, including all of its contents and
@@ -1654,14 +1668,14 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
protected static boolean validateContextName(ContextName cn,
PrintWriter writer, StringManager smClient) {
-
+
// ContextName should be non-null with a path that is empty or starts
// with /
if (cn != null &&
(cn.getPath().startsWith("/") || cn.getPath().equals(""))) {
return true;
}
-
+
String path = null;
if (cn != null) {
path = RequestUtil.filter(cn.getPath());
@@ -1689,7 +1703,7 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
return result;
}
-
+
/**
* Copy the specified file or directory to the destination.
*
@@ -1697,9 +1711,9 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
* @param dest File object representing the destination
*/
public static boolean copyInternal(File src, File dest, byte[] buf) {
-
+
boolean result = true;
-
+
String files[] = null;
if (src.isDirectory()) {
files = src.list();
@@ -1751,8 +1765,8 @@ public class ManagerServlet extends HttpServlet
implements ContainerServlet {
}
}
return result;
-
+
}
-
-
+
+
}
diff --git a/java/org/apache/tomcat/util/IntrospectionUtils.java
b/java/org/apache/tomcat/util/IntrospectionUtils.java
index 5c1dd2f..fee02f6 100644
--- a/java/org/apache/tomcat/util/IntrospectionUtils.java
+++ b/java/org/apache/tomcat/util/IntrospectionUtils.java
@@ -966,6 +966,28 @@ public final class IntrospectionUtils {
return result;
}
+
+ public static boolean isInstance(Class<?> clazz, String type) {
+ if (type.equals(clazz.getName())) {
+ return true;
+ }
+
+ Class<?>[] ifaces = clazz.getInterfaces();
+ for (Class<?> iface : ifaces) {
+ if (isInstance(iface, type)) {
+ return true;
+ }
+ }
+
+ Class<?> superClazz = clazz.getSuperclass();
+ if (superClazz == null) {
+ return false;
+ } else {
+ return isInstance(superClazz, type);
+ }
+ }
+
+
// -------------------- Get property --------------------
// This provides a layer of abstraction
diff --git a/test/org/apache/tomcat/util/TestIntrospectionUtils.java
b/test/org/apache/tomcat/util/TestIntrospectionUtils.java
new file mode 100644
index 0000000..64864f6
--- /dev/null
+++ b/test/org/apache/tomcat/util/TestIntrospectionUtils.java
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.core.StandardContext;
+
+public class TestIntrospectionUtils {
+
+ // Test for all the classes and interfaces in StandardContext's type
hierarchy
+
+ @Test
+ public void testIsInstanceStandardContext01() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class,
"org.apache.catalina.core.StandardContext"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext02() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class,
"org.apache.catalina.util.LifecycleMBeanBase"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext03() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class,
"org.apache.catalina.util.LifecycleBase"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext04() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "java.lang.Object"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext05() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "org.apache.catalina.Lifecycle"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext06() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "org.apache.catalina.JmxEnabled"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext07() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "javax.management.MBeanRegistration"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext08() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "org.apache.catalina.Container"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext09() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class, "org.apache.tomcat.ContextBind"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext10() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class,
"javax.management.NotificationEmitter"));
+ }
+
+
+ @Test
+ public void testIsInstanceStandardContext11() {
+ Assert.assertTrue(IntrospectionUtils.isInstance(
+ StandardContext.class,
"javax.management.NotificationBroadcaster"));
+ }
+
+ // And one to check that non-matches return false
+
+
+ @Test
+ public void testIsInstanceStandardContext12() {
+ Assert.assertFalse(IntrospectionUtils.isInstance(
+ StandardContext.class, "com.example.Other"));
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 367dcde..ae21089 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -85,6 +85,10 @@
Fix a potential resource leak when a JNDI lookup returns an object of
an
in compatible class. Identified by Coverity scan. (markt)
</fix>
+ <scode>
+ Refactor <code>ManagerServlet</code> to avoid loading classes when
+ filtering JNDI resources for resources of a specified type. (markt)
+ </scode>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
