Skip to content

Commit

Permalink
Fix use of AccessController in LoaderUtil
Browse files Browse the repository at this point in the history
This fixes issue #2129 where `AccessController::doPrivileged` is
needlessly invoked when no `SecurityManager` is installed.
  • Loading branch information
jvz committed Dec 29, 2023
1 parent b747810 commit 8255ffa
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,10 @@ public final class LoaderUtil {
} catch (final SecurityException ignored) {
try {
// let's see if we can obtain that permission
AccessController.doPrivileged(
(PrivilegedAction<Void>) () -> {
AccessController.checkPermission(GET_CLASS_LOADER);
return null;
},
null,
GET_CLASS_LOADER);
runPrivilegedActionWithGetClassLoaderPermission((PrivilegedAction<Void>) () -> {
AccessController.checkPermission(GET_CLASS_LOADER);
return null;
});
getClassLoaderDisabled = false;
} catch (final SecurityException ignore) {
// no chance
Expand Down Expand Up @@ -108,7 +105,7 @@ public static ClassLoader getClassLoader(final Class<?> class1, final Class<?> c
}
return isChild(loader1, loader2) ? loader1 : loader2;
};
return AccessController.doPrivileged(action, null, GET_CLASS_LOADER);
return runActionInvolvingGetClassLoaderPermission(action);
}

/**
Expand Down Expand Up @@ -143,22 +140,29 @@ private static boolean isChild(final ClassLoader loader1, final ClassLoader load
* @return the current thread's ClassLoader, a fallback loader, or null if no fallback can be determined
*/
public static ClassLoader getThreadContextClassLoader() {
if (GET_CLASS_LOADER_DISABLED) {
// we can at least get this class's ClassLoader regardless of security context
// however, if this is null, there's really no option left at this point
try {
return getThisClassLoader();
} catch (final SecurityException ignored) {
return null;
}
try {
return GET_CLASS_LOADER_DISABLED
? getThisClassLoader()
: runActionInvolvingGetClassLoaderPermission(TCCL_GETTER);
} catch (final SecurityException ignored) {
return null;
}
return AccessController.doPrivileged(TCCL_GETTER, null, GET_CLASS_LOADER);
}

private static ClassLoader getThisClassLoader() {
return LoaderUtil.class.getClassLoader();
}

private static <T> T runActionInvolvingGetClassLoaderPermission(final PrivilegedAction<T> action) {
return System.getSecurityManager() != null
? runPrivilegedActionWithGetClassLoaderPermission(action)
: action.run();
}

private static <T> T runPrivilegedActionWithGetClassLoaderPermission(final PrivilegedAction<T> action) {
return AccessController.doPrivileged(action, null, GET_CLASS_LOADER);
}

private static class ThreadContextClassLoaderGetter implements PrivilegedAction<ClassLoader> {
@Override
public ClassLoader run() {
Expand Down
27 changes: 27 additions & 0 deletions src/changelog/.2.x.x/fix_security_manager_use_in_LoaderUtil.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ 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.
-->
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://logging.apache.org/log4j/changelog"
xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
type="fixed">
<issue id="2129" link="https:/apache/logging-log4j2/issues/2129"/>
<description format="asciidoc">
Fixed use of `SecurityManager` in `LoaderUtil` where `AccessController::doPrivileged` should only be invoked when
a `SecurityManager` is installed. Some runtimes do not seem to have this method available.
</description>
</entry>

0 comments on commit 8255ffa

Please sign in to comment.