Skip to content

Commit 8b72076

Browse files
jvzjvz
authored
Merge pull request #2138 from jvz/issue-2129
Fix use of AccessController in LoaderUtil
2 parents 23f6e63 + 6dafba3 commit 8b72076

File tree

4 files changed

+113
-43
lines changed

4 files changed

+113
-43
lines changed

‎log4j-api-test/src/test/java/org/apache/logging/log4j/util/LoaderUtilSecurityManagerTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.util;
18+
19+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
20+
import static org.junit.jupiter.api.Assertions.assertFalse;
21+
22+
import java.security.Permission;
23+
import org.apache.logging.log4j.test.junit.SecurityManagerTestRule;
24+
import org.junit.Rule;
25+
import org.junit.Test;
26+
import org.junit.jupiter.api.parallel.ResourceLock;
27+
28+
@ResourceLock("java.lang.SecurityManager")
29+
public class LoaderUtilSecurityManagerTest {
30+
@Rule
31+
public final SecurityManagerTestRule rule = new SecurityManagerTestRule(new TestSecurityManager());
32+
33+
private static class TestSecurityManager extends SecurityManager {
34+
@Override
35+
public void checkPermission(final Permission perm) {
36+
if (perm.equals(LoaderUtil.GET_CLASS_LOADER)) {
37+
throw new SecurityException("disabled");
38+
}
39+
}
40+
}
41+
42+
@Test
43+
public void canGetClassLoaderThroughPrivileges() {
44+
assertFalse(LoaderUtil.GET_CLASS_LOADER_DISABLED);
45+
assertDoesNotThrow(() -> LoaderUtil.getClassLoader(LoaderUtilSecurityManagerTest.class, String.class));
46+
}
47+
}

‎log4j-api/src/main/java/org/apache/logging/log4j/util/LoaderUtil.java‎

Lines changed: 34 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -50,39 +50,31 @@ public final class LoaderUtil {
5050
// wants to use PropertiesUtil, but then PropertiesUtil wants to use LoaderUtil.
5151
private static Boolean ignoreTCCL;
5252

53-
private static final RuntimePermission GET_CLASS_LOADER = new RuntimePermission("getClassLoader");
54-
private static final boolean GET_CLASS_LOADER_DISABLED;
55-
56-
private static final PrivilegedAction<ClassLoader> TCCL_GETTER = new ThreadContextClassLoaderGetter();
57-
58-
static {
59-
if (System.getSecurityManager() != null) {
60-
boolean getClassLoaderDisabled;
53+
static final RuntimePermission GET_CLASS_LOADER = new RuntimePermission("getClassLoader");
54+
static final LazyBoolean GET_CLASS_LOADER_DISABLED = new LazyBoolean(() -> {
55+
if (System.getSecurityManager() == null) {
56+
return false;
57+
}
58+
try {
59+
AccessController.checkPermission(GET_CLASS_LOADER);
60+
// seems like we'll be ok
61+
return false;
62+
} catch (final SecurityException ignored) {
6163
try {
62-
AccessController.checkPermission(GET_CLASS_LOADER);
63-
// seems like we'll be ok
64-
getClassLoaderDisabled = false;
65-
} catch (final SecurityException ignored) {
66-
try {
67-
// let's see if we can obtain that permission
68-
AccessController.doPrivileged(
69-
(PrivilegedAction<Void>) () -> {
70-
AccessController.checkPermission(GET_CLASS_LOADER);
71-
return null;
72-
},
73-
null,
74-
GET_CLASS_LOADER);
75-
getClassLoaderDisabled = false;
76-
} catch (final SecurityException ignore) {
77-
// no chance
78-
getClassLoaderDisabled = true;
79-
}
64+
// let's see if we can obtain that permission
65+
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
66+
AccessController.checkPermission(GET_CLASS_LOADER);
67+
return null;
68+
});
69+
return false;
70+
} catch (final SecurityException ignore) {
71+
// no chance
72+
return true;
8073
}
81-
GET_CLASS_LOADER_DISABLED = getClassLoaderDisabled;
82-
} else {
83-
GET_CLASS_LOADER_DISABLED = false;
8474
}
85-
}
75+
});
76+
77+
private static final PrivilegedAction<ClassLoader> TCCL_GETTER = new ThreadContextClassLoaderGetter();
8678

8779
private LoaderUtil() {}
8880

@@ -100,15 +92,15 @@ public static ClassLoader getClassLoader(final Class<?> class1, final Class<?> c
10092
PrivilegedAction<ClassLoader> action = () -> {
10193
final ClassLoader loader1 = class1 == null ? null : class1.getClassLoader();
10294
final ClassLoader loader2 = class2 == null ? null : class2.getClassLoader();
103-
final ClassLoader referenceLoader = GET_CLASS_LOADER_DISABLED
95+
final ClassLoader referenceLoader = GET_CLASS_LOADER_DISABLED.getAsBoolean()
10496
? getThisClassLoader()
10597
: Thread.currentThread().getContextClassLoader();
10698
if (isChild(referenceLoader, loader1)) {
10799
return isChild(referenceLoader, loader2) ? referenceLoader : loader2;
108100
}
109101
return isChild(loader1, loader2) ? loader1 : loader2;
110102
};
111-
return AccessController.doPrivileged(action, null, GET_CLASS_LOADER);
103+
return runPrivileged(action);
112104
}
113105

114106
/**
@@ -143,22 +135,21 @@ private static boolean isChild(final ClassLoader loader1, final ClassLoader load
143135
* @return the current thread's ClassLoader, a fallback loader, or null if no fallback can be determined
144136
*/
145137
public static ClassLoader getThreadContextClassLoader() {
146-
if (GET_CLASS_LOADER_DISABLED) {
147-
// we can at least get this class's ClassLoader regardless of security context
148-
// however, if this is null, there's really no option left at this point
149-
try {
150-
return getThisClassLoader();
151-
} catch (final SecurityException ignored) {
152-
return null;
153-
}
138+
try {
139+
return GET_CLASS_LOADER_DISABLED.getAsBoolean() ? getThisClassLoader() : runPrivileged(TCCL_GETTER);
140+
} catch (final SecurityException ignored) {
141+
return null;
154142
}
155-
return AccessController.doPrivileged(TCCL_GETTER, null, GET_CLASS_LOADER);
156143
}
157144

158145
private static ClassLoader getThisClassLoader() {
159146
return LoaderUtil.class.getClassLoader();
160147
}
161148

149+
private static <T> T runPrivileged(final PrivilegedAction<T> action) {
150+
return System.getSecurityManager() != null ? AccessController.doPrivileged(action) : action.run();
151+
}
152+
162153
private static class ThreadContextClassLoaderGetter implements PrivilegedAction<ClassLoader> {
163154
@Override
164155
public ClassLoader run() {
@@ -167,7 +158,7 @@ public ClassLoader run() {
167158
return contextClassLoader;
168159
}
169160
final ClassLoader thisClassLoader = getThisClassLoader();
170-
if (thisClassLoader != null || GET_CLASS_LOADER_DISABLED) {
161+
if (thisClassLoader != null || GET_CLASS_LOADER_DISABLED.getAsBoolean()) {
171162
return thisClassLoader;
172163
}
173164
return ClassLoader.getSystemClassLoader();
@@ -472,7 +463,7 @@ static Collection<UrlResource> findUrlResources(final String resource, final boo
472463
final ClassLoader[] candidates = {
473464
useTccl ? getThreadContextClassLoader() : null,
474465
LoaderUtil.class.getClassLoader(),
475-
GET_CLASS_LOADER_DISABLED ? null : ClassLoader.getSystemClassLoader()
466+
GET_CLASS_LOADER_DISABLED.getAsBoolean() ? null : ClassLoader.getSystemClassLoader()
476467
};
477468
// @formatter:on
478469
final Collection<UrlResource> resources = new LinkedHashSet<>();

‎src/changelog/.2.x.x/fix_security_manager_use_in_LoaderUtil.xml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to you under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
-->
18+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
19+
xmlns="http://logging.apache.org/log4j/changelog"
20+
xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
21+
type="fixed">
22+
<issue id="2129" link="https://github.com/apache/logging-log4j2/issues/2129"/>
23+
<description format="asciidoc">
24+
Fixed use of `SecurityManager` in `LoaderUtil` where `AccessController::doPrivileged` should only be invoked when
25+
a `SecurityManager` is installed. Some runtimes do not seem to have this method available.
26+
</description>
27+
</entry>

‎src/site/_release-notes/_2.x.x.adoc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ This releases contains ...
3838
3939
* Deprecated the `RingBufferLogEventHandler` class for removal from the public API in 3.x.
4040
41+
[#release-notes-2-x-x-fixed]
42+
=== Fixed
43+
44+
* 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. (https://github.com/apache/logging-log4j2/issues/2129[2129])
45+
4146
[#release-notes-2-x-x-updated]
4247
=== Updated
4348

0 commit comments

Comments
 (0)