This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
     new caab2b9250 Improve CVE-2024-56337 protection
caab2b9250 is described below

commit caab2b9250346d140c920653dc395c99dc044a1a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Feb 21 11:54:55 2025 +0000

    Improve CVE-2024-56337 protection
    
    Don't use reflection unless necessary. This means less impact for those
    using Tomcat in an embedded environment.
---
 bin/catalina.bat                                   |   1 +
 bin/catalina.sh                                    |   1 +
 .../org/apache/tomcat/util/compat/Jre12Compat.java | 151 +++++++++++++++++++++
 .../org/apache/tomcat/util/compat/Jre16Compat.java |   2 +-
 java/org/apache/tomcat/util/compat/JreCompat.java  | 132 ++++++++++++++----
 .../tomcat/util/compat/LocalStrings.properties     |   2 +
 webapps/docs/changelog.xml                         |   6 +-
 7 files changed, 263 insertions(+), 32 deletions(-)

diff --git a/bin/catalina.bat b/bin/catalina.bat
index 5e1a3e8006..af97bedc4c 100755
--- a/bin/catalina.bat
+++ b/bin/catalina.bat
@@ -226,6 +226,7 @@ set 
LOGGING_MANAGER=-Djava.util.logging.manager=org.apache.juli.ClassLoaderLogMa
 
 rem Configure module start-up parameters
 set "JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.lang=ALL-UNNAMED"
+set "JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.lang.reflect=ALL-UNNAMED"
 set "JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.io=ALL-UNNAMED"
 set "JAVA_OPTS=%JAVA_OPTS% --add-opens=java.base/java.util=ALL-UNNAMED"
 set "JAVA_OPTS=%JAVA_OPTS% 
--add-opens=java.base/java.util.concurrent=ALL-UNNAMED"
diff --git a/bin/catalina.sh b/bin/catalina.sh
index 959a637389..abc43a6117 100755
--- a/bin/catalina.sh
+++ b/bin/catalina.sh
@@ -299,6 +299,7 @@ fi
 
 # Add the module start-up parameters required by Tomcat
 JAVA_OPTS="$JAVA_OPTS --add-opens=java.base/java.lang=ALL-UNNAMED"
+JAVA_OPTS="$JAVA_OPTS --add-opens=java.base/java.lang.reflect=ALL-UNNAMED"
 JAVA_OPTS="$JAVA_OPTS --add-opens=java.base/java.io=ALL-UNNAMED"
 JAVA_OPTS="$JAVA_OPTS --add-opens=java.base/java.util=ALL-UNNAMED"
 JAVA_OPTS="$JAVA_OPTS --add-opens=java.base/java.util.concurrent=ALL-UNNAMED"
diff --git a/java/org/apache/tomcat/util/compat/Jre12Compat.java 
b/java/org/apache/tomcat/util/compat/Jre12Compat.java
new file mode 100644
index 0000000000..ea27f455f4
--- /dev/null
+++ b/java/org/apache/tomcat/util/compat/Jre12Compat.java
@@ -0,0 +1,151 @@
+/*
+ *  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.compat;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.invoke.VarHandle;
+import java.lang.management.ManagementFactory;
+import java.lang.reflect.Field;
+import java.lang.reflect.InaccessibleObjectException;
+import java.lang.reflect.Modifier;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
+public class Jre12Compat extends JreCompat {
+
+    private static final Log log = LogFactory.getLog(Jre12Compat.class);
+    private static final StringManager sm = 
StringManager.getManager(Jre12Compat.class);
+
+    private static final boolean supported;
+
+    static {
+        // Don't need any Java 12 specific classes (yet) so just test for one 
of
+        // the new ones for now.
+        Class<?> c1 = null;
+        try {
+            c1 = Class.forName("java.text.CompactNumberFormat");
+        } catch (ClassNotFoundException cnfe) {
+            // Must be pre-Java 12
+            log.debug(sm.getString("jre12Compat.javaPre12"), cnfe);
+        }
+
+        supported = (c1 != null);
+    }
+
+    static boolean isSupported() {
+        return supported;
+    }
+
+
+    /*
+     * The behaviour of the canonical file name cache varies by Java version.
+     *
+     * The cache was removed in Java 21 so these methods and the associated 
code can be removed once the minimum Java
+     * version is 21.
+     *
+     * For 12 <= Java <= 20, the cache was present but disabled by default.
+     *
+     * For Java < 12, the cache was enabled by default. Tomcat assumes the 
cache is enabled unless proven otherwise.
+     *
+     * Tomcat 10.1 has a minimum Java version of 11.
+     *
+     * The static field in java.io.FileSystem will be set before any 
application code gets a chance to run. Therefore,
+     * the value of that field can be determined by looking at the command 
line arguments. This enables us to determine
+     * the status without having using reflection.
+     *
+     * This is Java 12 and later.
+     */
+    @Override
+    public boolean isCanonCachesDisabled() {
+        if (canonCachesDisabled != null) {
+            return canonCachesDisabled.booleanValue();
+        }
+        synchronized (canonCachesDisabledLock) {
+            if (canonCachesDisabled != null) {
+                return canonCachesDisabled.booleanValue();
+            }
+
+            List<String> args = 
ManagementFactory.getRuntimeMXBean().getInputArguments();
+            for (String arg : args) {
+                // If any command line argument attempts to enable the cache, 
assume it is enabled.
+                if (arg.startsWith(USE_CANON_CACHES_CMD_ARG)) {
+                    String value = 
arg.substring(USE_CANON_CACHES_CMD_ARG.length());
+                    boolean cacheEnabled = 
Boolean.valueOf(value).booleanValue();
+                    if (cacheEnabled) {
+                        canonCachesDisabled = Boolean.FALSE;
+                        return false;
+                    }
+                }
+            }
+            canonCachesDisabled = Boolean.TRUE;
+            return true;
+        }
+    }
+
+
+    /*
+     * Java 12 increased security around reflection so additional code is 
required to disable the cache since a final
+     * field needs to be changed.
+     */
+    @Override
+    protected void ensureUseCanonCachesFieldIsPopulated() {
+        if (useCanonCachesField != null) {
+            return;
+        }
+        synchronized (useCanonCachesFieldLock) {
+            if (useCanonCachesField != null) {
+                return;
+            }
+
+            Field f = null;
+            try {
+                Class<?> clazz = Class.forName("java.io.FileSystem");
+                f = clazz.getDeclaredField("useCanonCaches");
+                // Need this because the 'useCanonCaches' field is private
+                f.setAccessible(true);
+
+                /*
+                 * Need this in Java 12 to 17 (and it only works up to Java 
17) because the 'useCanonCaches' field is
+                 * final.
+                 *
+                 * This will fail in Java 18 to 20 but since those versions 
are no longer supported it is acceptable for
+                 * the attempt to set the 'useCanonCaches' field to fail. 
Users that really want to use Java 18 to 20
+                 * will have to ensure that they do not explicitly enable the 
canonical file name cache.
+                 */
+                VarHandle modifiers;
+                Lookup lookup = MethodHandles.privateLookupIn(Field.class, 
MethodHandles.lookup());
+                modifiers = lookup.findVarHandle(Field.class, "modifiers", 
int.class);
+                modifiers.set(f, f.getModifiers() & ~Modifier.FINAL);
+            } catch (InaccessibleObjectException | 
ReflectiveOperationException | IllegalArgumentException e) {
+                // Make sure field is not set.
+                f = null;
+                log.warn(sm.getString("jreCompat.useCanonCaches.init"), e);
+            }
+
+            if (f == null) {
+                useCanonCachesField = Optional.empty();
+            } else {
+                useCanonCachesField = Optional.of(f);
+            }
+        }
+    }
+}
diff --git a/java/org/apache/tomcat/util/compat/Jre16Compat.java 
b/java/org/apache/tomcat/util/compat/Jre16Compat.java
index c4d6f3f1d1..70e5fb1094 100644
--- a/java/org/apache/tomcat/util/compat/Jre16Compat.java
+++ b/java/org/apache/tomcat/util/compat/Jre16Compat.java
@@ -28,7 +28,7 @@ import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.res.StringManager;
 
-class Jre16Compat extends JreCompat {
+class Jre16Compat extends Jre12Compat {
 
     private static final Log log = LogFactory.getLog(Jre16Compat.class);
     private static final StringManager sm = 
StringManager.getManager(Jre16Compat.class);
diff --git a/java/org/apache/tomcat/util/compat/JreCompat.java 
b/java/org/apache/tomcat/util/compat/JreCompat.java
index e188d57ed1..a0feab9f3f 100644
--- a/java/org/apache/tomcat/util/compat/JreCompat.java
+++ b/java/org/apache/tomcat/util/compat/JreCompat.java
@@ -16,12 +16,15 @@
  */
 package org.apache.tomcat.util.compat;
 
+import java.lang.management.ManagementFactory;
 import java.lang.reflect.Field;
 import java.lang.reflect.InaccessibleObjectException;
 import java.net.SocketAddress;
 import java.nio.channels.ServerSocketChannel;
 import java.nio.channels.SocketChannel;
 import java.security.PrivilegedExceptionAction;
+import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CompletionException;
 
@@ -42,12 +45,17 @@ public class JreCompat {
 
     private static final JreCompat instance;
     private static final boolean graalAvailable;
+    private static final boolean jre12Available;
     private static final boolean jre16Available;
     private static final boolean jre19Available;
     private static final boolean jre21Available;
     private static final boolean jre22Available;
 
-    private static final Field useCanonCachesField;
+    protected static final String USE_CANON_CACHES_CMD_ARG = 
"-Dsun.io.useCanonCaches=";
+    protected static volatile Boolean canonCachesDisabled;
+    protected static final Object canonCachesDisabledLock = new Object();
+    protected static volatile Optional<Field> useCanonCachesField;
+    protected static final Object useCanonCachesFieldLock = new Object();
 
     static {
         boolean result = false;
@@ -69,46 +77,43 @@ public class JreCompat {
             jre21Available = true;
             jre19Available = true;
             jre16Available = true;
+            jre12Available = true;
         } else if (Jre21Compat.isSupported()) {
             instance = new Jre21Compat();
             jre22Available = false;
             jre21Available = true;
             jre19Available = true;
             jre16Available = true;
+            jre12Available = true;
         } else if (Jre19Compat.isSupported()) {
             instance = new Jre19Compat();
             jre22Available = false;
             jre21Available = false;
             jre19Available = true;
             jre16Available = true;
+            jre12Available = true;
         } else if (Jre16Compat.isSupported()) {
             instance = new Jre16Compat();
             jre22Available = false;
             jre21Available = false;
             jre19Available = false;
             jre16Available = true;
+            jre12Available = true;
+        } else if (Jre12Compat.isSupported()) {
+            instance = new Jre12Compat();
+            jre22Available = false;
+            jre21Available = false;
+            jre19Available = false;
+            jre16Available = false;
+            jre12Available = true;
         } else {
             instance = new JreCompat();
             jre22Available = false;
             jre21Available = false;
             jre19Available = false;
             jre16Available = false;
+            jre12Available = false;
         }
-
-        Field f1 = null;
-        try {
-            Class<?> clazz = Class.forName("java.io.FileSystem");
-            f1 = clazz.getDeclaredField("useCanonCaches");
-            f1.setAccessible(true);
-        } catch (InaccessibleObjectException | ReflectiveOperationException | 
IllegalArgumentException e) {
-            /*
-             * Log at debug level as this will only be an issue if the field 
needs to be accessed and most
-             * configurations will not need to do so. Appropriate warnings 
will be logged if an attempt is made to use
-             * the field when it could not be found/accessed.
-             */
-            log.debug(sm.getString("jreCompat.useCanonCaches.init"), e);
-        }
-        useCanonCachesField = f1;
     }
 
 
@@ -122,6 +127,11 @@ public class JreCompat {
     }
 
 
+    public static boolean isJre12Available() {
+        return jre12Available;
+    }
+
+
     public static boolean isJre16Available() {
         return jre16Available;
     }
@@ -286,28 +296,56 @@ public class JreCompat {
 
 
     /*
-     * The behaviour of the canonical file cache varies by Java version.
+     * The behaviour of the canonical file name cache varies by Java version.
      *
      * The cache was removed in Java 21 so these methods and the associated 
code can be removed once the minimum Java
      * version is 21.
      *
-     * For 12 <= Java <= 20, the cache was present but disabled by default. 
Since the user may have changed the default
-     * Tomcat has to assume the cache is enabled unless proven otherwise.
+     * For 12 <= Java <= 20, the cache was present but disabled by default.
      *
      * For Java < 12, the cache was enabled by default. Tomcat assumes the 
cache is enabled unless proven otherwise.
+     *
+     * Tomcat 10.1 has a minimum Java version of 11.
+     *
+     * The static field in java.io.FileSystem will be set before any 
application code gets a chance to run. Therefore,
+     * the value of that field can be determined by looking at the command 
line arguments. This enables us to determine
+     * the status without having using reflection.
+     *
+     * This is Java 11.
      */
     public boolean isCanonCachesDisabled() {
-        if (useCanonCachesField == null) {
-            // No need to log a warning. The warning will be logged when 
trying to disable the cache.
-            return false;
+        if (canonCachesDisabled != null) {
+            return canonCachesDisabled.booleanValue();
         }
-        boolean result = false;
-        try {
-            result = !((Boolean) useCanonCachesField.get(null)).booleanValue();
-        } catch (ReflectiveOperationException e) {
-            // No need to log a warning. The warning will be logged when 
trying to disable the cache.
+        synchronized (canonCachesDisabledLock) {
+            if (canonCachesDisabled != null) {
+                return canonCachesDisabled.booleanValue();
+            }
+
+            boolean cacheEnabled = true;
+            List<String> args = 
ManagementFactory.getRuntimeMXBean().getInputArguments();
+            for (String arg : args) {
+                // To consider the cache disabled
+                // - there must be at least one command line argument that 
disables it
+                // - there must be no command line arguments that enable it
+                if (arg.startsWith(USE_CANON_CACHES_CMD_ARG)) {
+                    String valueAsString = 
arg.substring(USE_CANON_CACHES_CMD_ARG.length());
+                    boolean valueAsBoolean = 
Boolean.valueOf(valueAsString).booleanValue();
+                    if (valueAsBoolean) {
+                        canonCachesDisabled = Boolean.FALSE;
+                        return false;
+                    } else {
+                        cacheEnabled = false;
+                    }
+                }
+            }
+            if (cacheEnabled) {
+                canonCachesDisabled = Boolean.FALSE;
+            } else {
+                canonCachesDisabled = Boolean.TRUE;
+            }
+            return canonCachesDisabled.booleanValue();
         }
-        return result;
     }
 
 
@@ -318,16 +356,50 @@ public class JreCompat {
      *             as a result of this call, otherwise {@code false}
      */
     public boolean disableCanonCaches() {
-        if (useCanonCachesField == null) {
+        ensureUseCanonCachesFieldIsPopulated();
+        if (useCanonCachesField.isEmpty()) {
             log.warn(sm.getString("jreCompat.useCanonCaches.none"));
             return false;
         }
         try {
-            useCanonCachesField.set(null, Boolean.FALSE);
+            useCanonCachesField.get().set(null, Boolean.FALSE);
         } catch (ReflectiveOperationException | IllegalArgumentException e) {
             log.warn(sm.getString("jreCompat.useCanonCaches.failed"), e);
             return false;
         }
+        synchronized (canonCachesDisabledLock) {
+            canonCachesDisabled = Boolean.TRUE;
+        }
         return true;
     }
+
+
+    protected void ensureUseCanonCachesFieldIsPopulated() {
+        if (useCanonCachesField != null) {
+            return;
+        }
+        synchronized (useCanonCachesFieldLock) {
+            if (useCanonCachesField != null) {
+                return;
+            }
+
+            Field f = null;
+            try {
+                Class<?> clazz = Class.forName("java.io.FileSystem");
+                f = clazz.getDeclaredField("useCanonCaches");
+                // Need this because the 'useCanonCaches' field is private 
final
+                f.setAccessible(true);
+            } catch (InaccessibleObjectException | 
ReflectiveOperationException | IllegalArgumentException e) {
+                // Make sure field is not set.
+                f = null;
+                log.warn(sm.getString("jreCompat.useCanonCaches.init"), e);
+            }
+
+            if (f == null) {
+                useCanonCachesField = Optional.empty();
+            } else {
+                useCanonCachesField = Optional.of(f);
+            }
+        }
+    }
 }
diff --git a/java/org/apache/tomcat/util/compat/LocalStrings.properties 
b/java/org/apache/tomcat/util/compat/LocalStrings.properties
index 738e2ae4ac..5f8bdaefb2 100644
--- a/java/org/apache/tomcat/util/compat/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/compat/LocalStrings.properties
@@ -16,6 +16,8 @@
 # Do not edit this file directly.
 # To edit translations see: 
https://tomcat.apache.org/getinvolved.html#Translations
 
+jre12Compat.javaPre12=Method not found so assuming code is running on a 
pre-Java 12 JVM
+
 jre16Compat.javaPre16=Class not found so assuming code is running on a 
pre-Java 16 JVM
 jre16Compat.unexpected=Failed to create references to Java 16 classes and 
methods
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index cfd1fe14cc..8ad82411bd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,10 @@
         Fix a bug in the JRE compatibility detection that incorrectly 
identified
         Java 19 and Java 20 as supporting Java 21 features. (markt)
       </fix>
+      <fix>
+        Improve the checks for exposure to and protection against 
CVE-2024-56337
+        so that reflection is not used unless required. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">
@@ -160,7 +164,7 @@
       <fix>
         <bug>69576</bug>: Avoid possible failure intializing
         <code>JreCompat</code> due to uncaught exception introduced for the
-        check for CVE-2004-56337. (remm)
+        check for CVE-2024-56337. (remm)
       </fix>
     </changelog>
   </subsection>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to