When using the new sessionAttributeValueClassNameFilter, apply the fi…

…lter earlier rather than loading the class and then deciding to filter it out.

When a SecurityManager is used, enable filtering by default.
This is part 2 of 2 of the fix for CVE-2016-0714

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1725914 13f79535-47bb-0310-9956-ffa450edef68

java/org/apache/catalina/session/ManagerBase.java

@@ -37,6 +37,7 @@
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.Engine;
+import org.apache.catalina.Globals;
 import org.apache.catalina.Lifecycle;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleState;
@@ -193,7 +194,20 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager
     private boolean warnOnSessionAttributeFilterFailure;
 
 
-    // ------------------------------------------------------------- Properties
+    // ------------------------------------------------------------ Constructors
+
+    public ManagerBase() {
+        if (Globals.IS_SECURITY_ENABLED) {
+            // Minimum set required for default distribution/persistence to work
+            // plus String
+            setSessionAttributeValueClassNameFilter(
+                    "java\\.lang\\.(?:Boolean|Integer|Long|Number|String)");
+            setWarnOnSessionAttributeFilterFailure(true);
+        }
+    }
+
+
+    // -------------------------------------------------------------- Properties
 
     /**
      * Obtain the regular expression used to filter session attribute based on

java/org/apache/catalina/session/StandardManager.java

@@ -191,10 +191,12 @@ protected void doLoad() throws ClassNotFoundException, IOException {
         }
         Loader loader = null;
         ClassLoader classLoader = null;
+        Log logger = null;
         try (FileInputStream fis = new FileInputStream(file.getAbsolutePath());
-                BufferedInputStream bis = new BufferedInputStream(fis);) {
+                BufferedInputStream bis = new BufferedInputStream(fis)) {
             Context c = getContext();
             loader = c.getLoader();
+            logger = c.getLogger();
             if (loader != null) {
                 classLoader = loader.getClassLoader();
             }
@@ -204,7 +206,9 @@ protected void doLoad() throws ClassNotFoundException, IOException {
 
             // Load the previously unloaded active sessions
             synchronized (sessions) {
-                try (ObjectInputStream ois = new CustomObjectInputStream(bis, classLoader)) {
+                try (ObjectInputStream ois = new CustomObjectInputStream(bis, classLoader, logger,
+                        getSessionAttributeValueClassNamePattern(),
+                        getWarnOnSessionAttributeFilterFailure())) {
                     Integer count = (Integer) ois.readObject();
                     int n = count.intValue();
                     if (log.isDebugEnabled())

java/org/apache/catalina/session/StoreBase.java

@@ -215,7 +215,20 @@ public void processExpires() {
      */
     protected ObjectInputStream getObjectInputStream(InputStream is) throws IOException {
         BufferedInputStream bis = new BufferedInputStream(is);
-        return new CustomObjectInputStream(bis, Thread.currentThread().getContextClassLoader());
+
+        CustomObjectInputStream ois;
+        ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
+
+        if (manager instanceof ManagerBase) {
+            ManagerBase managerBase = (ManagerBase) manager;
+            ois = new CustomObjectInputStream(bis, classLoader, manager.getContext().getLogger(),
+                    managerBase.getSessionAttributeValueClassNamePattern(),
+                    managerBase.getWarnOnSessionAttributeFilterFailure());
+        } else {
+            ois = new CustomObjectInputStream(bis, classLoader);
+        }
+
+        return ois;
     }
 
 

java/org/apache/catalina/util/CustomObjectInputStream.java

@@ -18,9 +18,18 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InvalidClassException;
 import java.io.ObjectInputStream;
 import java.io.ObjectStreamClass;
 import java.lang.reflect.Proxy;
+import java.util.Collections;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Pattern;
+
+import org.apache.juli.logging.Log;
+import org.apache.tomcat.util.res.StringManager;
 
 /**
  * Custom subclass of <code>ObjectInputStream</code> that loads from the
@@ -32,23 +41,83 @@
  */
 public final class CustomObjectInputStream extends ObjectInputStream {
 
+    private static final StringManager sm = StringManager.getManager(CustomObjectInputStream.class);
+
+    private static final WeakHashMap<ClassLoader, Set<String>> reportedClassCache =
+            new WeakHashMap<>();
+
     /**
      * The class loader we will use to resolve classes.
      */
     private final ClassLoader classLoader;
+    private final Set<String> reportedClasses;
+    private final Log log;
+
+    private final Pattern allowedClassNamePattern;
+    private final String allowedClassNameFilter;
+    private final boolean warnOnFailure;
 
 
     /**
-     * Construct a new instance of CustomObjectInputStream
+     * Construct a new instance of CustomObjectInputStream without any filtering
+     * of deserialized classes.
      *
      * @param stream The input stream we will read from
      * @param classLoader The class loader used to instantiate objects
      *
      * @exception IOException if an input/output error occurs
      */
     public CustomObjectInputStream(InputStream stream, ClassLoader classLoader) throws IOException {
+        this(stream, classLoader, null, null, false);
+    }
+
+
+    /**
+     * Construct a new instance of CustomObjectInputStream with filtering of
+     * deserialized classes.
+     *
+     * @param stream The input stream we will read from
+     * @param classLoader The class loader used to instantiate objects
+     * @param log The logger to use to report any issues. It may only be null if
+     *            the filterMode does not require logging
+     * @param allowedClassNamePattern The regular expression to use to filter
+     *                                deserialized classes. The fully qualified
+     *                                class name must match this pattern for
+     *                                deserialization to be allowed if filtering
+     *                                is enabled.
+     * @param warnOnFailure Should any failures be logged?
+     *
+     * @exception IOException if an input/output error occurs
+     */
+    public CustomObjectInputStream(InputStream stream, ClassLoader classLoader,
+            Log log, Pattern allowedClassNamePattern, boolean warnOnFailure)
+            throws IOException {
         super(stream);
+        if (log == null && allowedClassNamePattern != null && warnOnFailure) {
+            throw new IllegalArgumentException(
+                    sm.getString("customObjectInputStream.logRequired"));
+        }
         this.classLoader = classLoader;
+        this.log = log;
+        this.allowedClassNamePattern = allowedClassNamePattern;
+        if (allowedClassNamePattern == null) {
+            this.allowedClassNameFilter = null;
+        } else {
+            this.allowedClassNameFilter = allowedClassNamePattern.toString();
+        }
+        this.warnOnFailure = warnOnFailure;
+
+        Set<String> reportedClasses = reportedClassCache.get(classLoader);
+        if (reportedClasses == null) {
+            reportedClasses = Collections.newSetFromMap(new ConcurrentHashMap<>());
+            Set<String> original = reportedClassCache.putIfAbsent(classLoader, reportedClasses);
+            if (original != null) {
+                // Concurrent attempts to create the new Set. Make sure all
+                // threads use the first successfully added Set.
+                reportedClasses = original;
+            }
+        }
+        this.reportedClasses = reportedClasses;
     }
 
 
@@ -64,8 +133,24 @@ public CustomObjectInputStream(InputStream stream, ClassLoader classLoader) thro
     @Override
     public Class<?> resolveClass(ObjectStreamClass classDesc)
         throws ClassNotFoundException, IOException {
+
+        String name = classDesc.getName();
+        if (allowedClassNamePattern != null) {
+            boolean allowed = allowedClassNamePattern.matcher(name).matches();
+            if (!allowed) {
+                boolean doLog = warnOnFailure && reportedClasses.add(name);
+                String msg = sm.getString("customObjectInputStream.nomatch", name, allowedClassNameFilter);
+                if (doLog) {
+                    log.warn(msg);
+                } else if (log.isDebugEnabled()) {
+                    log.debug(msg);
+                }
+                throw new InvalidClassException(msg);
+            }
+        }
+
         try {
-            return Class.forName(classDesc.getName(), false, classLoader);
+            return Class.forName(name, false, classLoader);
         } catch (ClassNotFoundException e) {
             try {
                 // Try also the superclass because of primitive types

java/org/apache/catalina/util/LocalStrings.properties

@@ -17,6 +17,8 @@ parameterMap.locked=No modifications are allowed to a locked ParameterMap
 resourceSet.locked=No modifications are allowed to a locked ResourceSet
 hexUtil.bad=Bad hexadecimal digit
 hexUtil.odd=Odd number of hexadecimal digits
+customObjectInputStream.logRequired=A valid logger is required for class name filtering with logging
+customObjectInputStream.nomatch=The class [{0}] did not match the regular expression [{1}] for classes allowed to be deserialized
 #Default Messages Utilized by the ExtensionValidator
 extensionValidator.web-application-manifest=Web Application Manifest
 extensionValidator.extension-not-found-error=ExtensionValidator[{0}][{1}]: Required extension [{2}] not found.

webapps/docs/changelog.xml

@@ -212,7 +212,8 @@
         based on the implementation class of the value and optional
         <code>WARN</code> level logging if an attribute is filtered. These
         options are avaialble for all of the Manager implementations that ship
-        with Tomcat. (markt)
+        with Tomcat. When a <code>SecurityManager</code> is used filtering will
+        be enabled by default. (markt)
       </add>
       <scode>
         Remove <code>distributable</code> and <code>maxInactiveInterval</code>

webapps/docs/config/cluster-manager.xml

@@ -180,7 +180,9 @@
         length or <code>null</code>, all attributes are eligible for
         replication. The pattern is anchored so the fully qualified class name
         must fully match the pattern. If not specified, the default value of
-        <code>null</code> will be used.</p>
+        <code>null</code> will be used unless a <code>SecurityManager</code> is
+        enabled in which case the default will be
+        <code>java\\.lang\\.(?:Boolean|Integer|Long|Number|String)</code>.</p>
       </attribute>
       <attribute name="stateTimestampDrop" required="false">
         When this node sends a <code>GET_ALL_SESSIONS</code> message to other
@@ -199,7 +201,8 @@
         attribute, should this be logged at <code>WARN</code> level? If
         <code>WARN</code> level logging is disabled then it will be logged at
         <code>DEBUG</code>. The default value of this attribute is
-        <code>false</code>.</p>
+        <code>false</code> unless a <code>SecurityManager</code> is enabled in
+        which case the default will be <code>true</code>.</p>
       </attribute>
     </attributes>
   </subsection>
@@ -242,7 +245,9 @@
         length or <code>null</code>, all attributes are eligible for
         replication. The pattern is anchored so the fully qualified class name
         must fully match the pattern. If not specified, the default value of
-        <code>null</code> will be used.</p>
+        <code>null</code> will be used unless a <code>SecurityManager</code> is
+        enabled in which case the default will be
+        <code>java\\.lang\\.(?:Boolean|Integer|Long|Number|String)</code>.</p>
       </attribute>
       <attribute name="terminateOnStartFailure" required="false">
         Set to true if you wish to terminate replication map when replication
@@ -257,7 +262,8 @@
         attribute, should this be logged at <code>WARN</code> level? If
         <code>WARN</code> level logging is disabled then it will be logged at
         <code>DEBUG</code>. The default value of this attribute is
-        <code>false</code>.</p>
+        <code>false</code> unless a <code>SecurityManager</code> is enabled in
+        which case the default will be <code>true</code>.</p>
       </attribute>
     </attributes>
   </subsection>

webapps/docs/config/manager.xml

@@ -159,7 +159,9 @@
         length or <code>null</code>, all attributes are eligible for
         distribution. The pattern is anchored so the fully qualified class name
         must fully match the pattern. If not specified, the default value of
-        <code>null</code> will be used.</p>
+        <code>null</code> will be used unless a <code>SecurityManager</code> is
+        enabled in which case the default will be
+        <code>java\\.lang\\.(?:Boolean|Integer|Long|Number|String)</code>.</p>
       </attribute>
 
       <attribute name="warnOnSessionAttributeFilterFailure" required="false">
@@ -168,7 +170,8 @@
         attribute, should this be logged at <code>WARN</code> level? If
         <code>WARN</code> level logging is disabled then it will be logged at
         <code>DEBUG</code>. The default value of this attribute is
-        <code>false</code>.</p>
+        <code>false</code> unless a <code>SecurityManager</code> is enabled in
+        which case the default will be <code>true</code>.</p>
       </attribute>
     </attributes>
 
@@ -279,7 +282,9 @@
         length or <code>null</code>, all attributes are eligible for
         distribution. The pattern is anchored so the fully qualified class name
         must fully match the pattern. If not specified, the default value of
-        <code>null</code> will be used.</p>
+        <code>null</code> will be used unless a <code>SecurityManager</code> is
+        enabled in which case the default will be
+        <code>java\\.lang\\.(?:Boolean|Integer|Long|Number|String)</code>.</p>
       </attribute>
 
       <attribute name="warnOnSessionAttributeFilterFailure" required="false">
@@ -288,7 +293,8 @@
         attribute, should this be logged at <code>WARN</code> level? If
         <code>WARN</code> level logging is disabled then it will be logged at
         <code>DEBUG</code>. The default value of this attribute is
-        <code>false</code>.</p>
+        <code>false</code> unless a <code>SecurityManager</code> is enabled in
+        which case the default will be <code>true</code>.</p>
       </attribute>
     </attributes>