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