Skip to content

Commit af4f28a

Browse files
Merge pull request from GHSA-883x-6fch-6wjx
* Blacklist ReflectionUtils.getUnderlyingCause as it is in the invocation This means that trusted classes are no longer have a fully whitelisted stack when the cause is fetched and exception methods are invoked. * Allow security manager to work with method names when checking the stack * Add security test for a malicious InvocationTargetException A variant of the exploit found by Daniel, related to GHSA-883x-6fch-6wjx Co-authored-by: Daniel Kirschten <melodicahaspa@gmail.com> Co-authored-by: Christian Femers <c.femers@tum.de> Co-authored-by: Daniel Kirschten <melodicahaspa@gmail.com>
1 parent c93275e commit af4f28a

File tree

6 files changed

+47
-6
lines changed

6 files changed

+47
-6
lines changed

Diff for: src/main/java/de/tum/in/test/api/security/ArtemisSecurityManager.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -511,19 +511,20 @@ private boolean isNotPrivileged(StackFrame stackFrame) {
511511
return !AccessController.class.getName().equals(stackFrame.getClassName());
512512
}
513513

514-
private boolean isCallNotWhitelisted(String call) {
514+
private boolean isCallNotWhitelisted(String className, String methodName) {
515+
String call = className + "." + methodName; //$NON-NLS-1$
515516
return SecurityConstants.STACK_BLACKLIST.stream().anyMatch(call::startsWith)
516517
|| (SecurityConstants.STACK_WHITELIST.stream().noneMatch(call::startsWith)
517-
&& (configuration == null || !(configuration.whitelistedClassNames().contains(call)
518-
|| configuration.trustedPackages().stream().anyMatch(pm -> pm.matches(call)))));
518+
&& (configuration == null || !(configuration.whitelistedClassNames().contains(className)
519+
|| configuration.trustedPackages().stream().anyMatch(pm -> pm.matches(className)))));
519520
}
520521

521522
private boolean isStackFrameNotWhitelisted(StackFrame sf) {
522-
return isCallNotWhitelisted(sf.getClassName());
523+
return isCallNotWhitelisted(sf.getClassName(), sf.getMethodName());
523524
}
524525

525526
private boolean isStackFrameNotWhitelisted(StackTraceElement ste) {
526-
return isCallNotWhitelisted(ste.getClassName());
527+
return isCallNotWhitelisted(ste.getClassName(), ste.getMethodName());
527528
}
528529

529530
public static Optional<StackTraceElement> firstNonWhitelisted(StackTraceElement... elements) {

Diff for: src/main/java/de/tum/in/test/api/security/SecurityConstants.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ public final class SecurityConstants {
1515
static final Set<String> STACK_WHITELIST = Set.of("java.", "org.junit.", "jdk.", "org.eclipse.", "com.intellij", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
1616
"org.assertj", "org.opentest4j.", "com.sun.", "sun.", "org.apache.", "de.tum.in.test.", "net.jqwik", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$
1717
"ch.qos.logback", "org.jacoco", "javax.", "org.json", SECURITY_PACKAGE_NAME); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
18-
static final Set<String> STACK_BLACKLIST = Set.of(BlacklistedInvoker.class.getName());
18+
static final Set<String> STACK_BLACKLIST = Set.of(BlacklistedInvoker.class.getName(),
19+
"org.junit.platform.commons.util.ReflectionUtils.getUnderlyingCause"); //$NON-NLS-1$
1920

2021
static final Set<String> PACKAGE_USE_BLACKLIST = Set.of(SECURITY_PACKAGE_NAME, "de.tum.in.test.api.internal", //$NON-NLS-1$
2122
"jdk.internal", "sun."); //$NON-NLS-1$ //$NON-NLS-2$

Diff for: src/test/java/de/tum/in/test/api/SecurityTest.java

+7
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class SecurityTest {
2626
private final String testExecuteGit = "testExecuteGit";
2727
private final String testMaliciousExceptionA = "testMaliciousExceptionA";
2828
private final String testMaliciousExceptionB = "testMaliciousExceptionB";
29+
private final String testMaliciousInvocationTargetException = "testMaliciousInvocationTargetException";
2930
private final String testNewClassLoader = "testNewClassLoader";
3031
private final String testNewSecurityManager = "testNewSecurityManager";
3132
private final String tryManageProcess = "tryManageProcess";
@@ -80,6 +81,12 @@ void test_testMaliciousExceptionB() {
8081
tests.assertThatEvents().haveExactly(1, testFailedWith(testMaliciousExceptionB, SecurityException.class));
8182
}
8283

84+
@TestTest
85+
void test_testMaliciousInvocationTargetException() {
86+
tests.assertThatEvents().haveExactly(1,
87+
testFailedWith(testMaliciousInvocationTargetException, SecurityException.class));
88+
}
89+
8390
@TestTest
8491
void test_testNewClassLoader() {
8592
tests.assertThatEvents().haveExactly(1, testFailedWith(testNewClassLoader, SecurityException.class));

Diff for: src/test/java/de/tum/in/testuser/SecurityUser.java

+5
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ void testMaliciousExceptionB() {
7777
assertFalse(SecurityPenguin.maliciousExceptionB());
7878
}
7979

80+
@Test
81+
void testMaliciousInvocationTargetException() throws Exception {
82+
SecurityPenguin.maliciousInvocationTargetException();
83+
}
84+
8085
@Test
8186
void testNewClassLoader() throws IOException {
8287
SecurityPenguin.newClassLoader();

Diff for: src/test/java/de/tum/in/testuser/subject/SecurityPenguin.java

+5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.apache.xyz.Circumvention;
2020
import org.apache.xyz.FakeTrustedClass;
2121
import org.apache.xyz.MaliciousExceptionB;
22+
import org.apache.xyz.MaliciousInvocationTargetException;
2223

2324
import de.tum.in.test.api.io.IOTester;
2425

@@ -49,6 +50,10 @@ public static boolean maliciousExceptionB() {
4950
return ab.get();
5051
}
5152

53+
public static void maliciousInvocationTargetException() throws Exception {
54+
throw new MaliciousInvocationTargetException();
55+
}
56+
5257
@SuppressWarnings("resource")
5358
public static void newClassLoader() throws IOException {
5459
new URLClassLoader(new URL[0]).close();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package org.apache.xyz;
2+
3+
import java.io.IOException;
4+
import java.io.UncheckedIOException;
5+
import java.lang.reflect.InvocationTargetException;
6+
import java.nio.file.Files;
7+
import java.nio.file.Path;
8+
9+
public class MaliciousInvocationTargetException extends InvocationTargetException {
10+
11+
private static final long serialVersionUID = 1L;
12+
13+
@Override
14+
public Throwable getTargetException() {
15+
try {
16+
Files.readString(Path.of("pom.xml"));
17+
} catch (IOException e) {
18+
throw new UncheckedIOException(e);
19+
}
20+
return new Error("succeeded");
21+
}
22+
}

0 commit comments

Comments
 (0)