This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
The following commit(s) were added to refs/heads/master by this push:
new 14890bf2 Repositories cache classes under the input-defined this_class
name; global static Repository/VerifierFactory make the poisoning
ClassLoader-wide (f012).
14890bf2 is described below
commit 14890bf2b9014df25f9b4de86f29b5e917e5656b
Author: Gary Gregory <[email protected]>
AuthorDate: Fri Sep 4 17:06:23 2026 -0400
Repositories cache classes under the input-defined this_class name;
global static Repository/VerifierFactory make the poisoning
ClassLoader-wide (f012).
---
.../bcel/util/AbstractClassPathRepository.java | 6 +++
.../apache/bcel/util/ClassLoaderRepository.java | 6 +++
.../apache/bcel/EnclosingMethodAttributeTest.java | 5 +-
.../apache/bcel/generic/FieldAnnotationsTest.java | 5 +-
.../generic/GeneratingAnnotatedClassesTest.java | 5 +-
.../bcel/util/RepositoryNameMismatchTest.java | 58 ++++++++++++++++++++++
6 files changed, 79 insertions(+), 6 deletions(-)
diff --git
a/src/main/java/org/apache/bcel/util/AbstractClassPathRepository.java
b/src/main/java/org/apache/bcel/util/AbstractClassPathRepository.java
index 60f56590..429c156a 100644
--- a/src/main/java/org/apache/bcel/util/AbstractClassPathRepository.java
+++ b/src/main/java/org/apache/bcel/util/AbstractClassPathRepository.java
@@ -89,6 +89,12 @@ abstract class AbstractClassPathRepository implements
Repository {
if (inputStream != null) {
final ClassParser parser = new ClassParser(inputStream,
className);
final JavaClass clazz = parser.parse();
+ // The this_class name inside the parsed bytes is
attacker-controlled: caching it under its own name
+ // would let a class file found under one name poison the
repository entry for another (the JDK's
+ // defineClass() rejects the same mismatch). Refuse to cache
or return it.
+ if (!clazz.getClassName().equals(className)) {
+ throw new ClassNotFoundException("Class name mismatch:
requested " + className + " but the class file declares " +
clazz.getClassName());
+ }
storeClass(clazz);
return clazz;
}
diff --git a/src/main/java/org/apache/bcel/util/ClassLoaderRepository.java
b/src/main/java/org/apache/bcel/util/ClassLoaderRepository.java
index 52e207fd..c1675e1e 100644
--- a/src/main/java/org/apache/bcel/util/ClassLoaderRepository.java
+++ b/src/main/java/org/apache/bcel/util/ClassLoaderRepository.java
@@ -93,6 +93,12 @@ public class ClassLoaderRepository implements Repository {
}
final ClassParser parser = new ClassParser(is, className);
RC = parser.parse();
+ // The this_class name inside the parsed bytes is
attacker-controlled: caching it under its own name
+ // would let a class file found under one name poison the
repository entry for another (the JDK's
+ // defineClass() rejects the same mismatch). Refuse to cache or
return it.
+ if (!RC.getClassName().equals(className)) {
+ throw new ClassNotFoundException("Class name mismatch:
requested " + className + " but the class file declares " + RC.getClassName());
+ }
storeClass(RC);
return RC;
} catch (final IOException e) {
diff --git a/src/test/java/org/apache/bcel/EnclosingMethodAttributeTest.java
b/src/test/java/org/apache/bcel/EnclosingMethodAttributeTest.java
index 9fdfdf87..3e6a3f95 100644
--- a/src/test/java/org/apache/bcel/EnclosingMethodAttributeTest.java
+++ b/src/test/java/org/apache/bcel/EnclosingMethodAttributeTest.java
@@ -39,7 +39,8 @@ class EnclosingMethodAttributeTest extends AbstractTest {
*/
@Test
void testAttributeSerializtion() throws ClassNotFoundException,
IOException {
- final JavaClass clazz = getTestJavaClass(PACKAGE_BASE_NAME +
".data.AttributeTestClassEM02$1");
+ final String name = PACKAGE_BASE_NAME +
".data.AttributeTestClassEM02$1";
+ final JavaClass clazz = getTestJavaClass(name);
final ConstantPool pool = clazz.getConstantPool();
final Attribute[] encMethodAttrs = findAttribute("EnclosingMethod",
clazz);
assertEquals(1, encMethodAttrs.length, "Wrong number of
EnclosingMethod attributes");
@@ -48,7 +49,7 @@ class EnclosingMethodAttributeTest extends AbstractTest {
clazz.dump(tfile);
// Read in the new version and check it is OK
final SyntheticRepository repos2 = createRepos(".");
- final JavaClass clazz2 = repos2.loadClass("AttributeTestClassEM02$1");
+ final JavaClass clazz2 = repos2.loadClass(name);
assertNotNull(clazz2); // Use the variable to avoid a warning
final EnclosingMethod em = (EnclosingMethod) encMethodAttrs[0];
final String enclosingClassName =
em.getEnclosingClass().getBytes(pool);
diff --git a/src/test/java/org/apache/bcel/generic/FieldAnnotationsTest.java
b/src/test/java/org/apache/bcel/generic/FieldAnnotationsTest.java
index 5a05c552..37302716 100644
--- a/src/test/java/org/apache/bcel/generic/FieldAnnotationsTest.java
+++ b/src/test/java/org/apache/bcel/generic/FieldAnnotationsTest.java
@@ -75,14 +75,15 @@ class FieldAnnotationsTest extends AbstractTest {
*/
@Test
void testFieldAnnotationEntrysReadWrite() throws ClassNotFoundException,
IOException {
- final JavaClass clazz = getTestJavaClass(PACKAGE_BASE_NAME +
".data.AnnotatedFields");
+ final String name = PACKAGE_BASE_NAME + ".data.AnnotatedFields";
+ final JavaClass clazz = getTestJavaClass(name);
checkAnnotatedField(clazz, "i", "L" + PACKAGE_BASE_SIG +
"/data/SimpleAnnotation;", "id", "1");
checkAnnotatedField(clazz, "s", "L" + PACKAGE_BASE_SIG +
"/data/SimpleAnnotation;", "id", "2");
// Write it out
final File tfile = createTestdataFile("AnnotatedFields.class");
clazz.dump(tfile);
final SyntheticRepository repos2 = createRepos(".");
- repos2.loadClass("AnnotatedFields");
+ repos2.loadClass(name);
checkAnnotatedField(clazz, "i", "L" + PACKAGE_BASE_SIG +
"/data/SimpleAnnotation;", "id", "1");
checkAnnotatedField(clazz, "s", "L" + PACKAGE_BASE_SIG +
"/data/SimpleAnnotation;", "id", "2");
assertTrue(tfile.delete());
diff --git
a/src/test/java/org/apache/bcel/generic/GeneratingAnnotatedClassesTest.java
b/src/test/java/org/apache/bcel/generic/GeneratingAnnotatedClassesTest.java
index f3e007b3..ddcd7ca5 100644
--- a/src/test/java/org/apache/bcel/generic/GeneratingAnnotatedClassesTest.java
+++ b/src/test/java/org/apache/bcel/generic/GeneratingAnnotatedClassesTest.java
@@ -455,13 +455,14 @@ class GeneratingAnnotatedClassesTest extends AbstractTest
{
*/
@Test
void testModifyingClasses2() throws ClassNotFoundException {
- final JavaClass jc = getTestJavaClass(PACKAGE_BASE_NAME +
".data.SimpleAnnotatedClass");
+ final String name = PACKAGE_BASE_NAME + ".data.SimpleAnnotatedClass";
+ final JavaClass jc = getTestJavaClass(name);
final ClassGen cgen = new ClassGen(jc);
final ConstantPoolGen cp = cgen.getConstantPool();
cgen.addAnnotationEntry(createCombinedAnnotation(cp));
assertEquals(2, cgen.getAnnotationEntries().length, "Wrong number of
annotations");
dumpClass(cgen, "SimpleAnnotatedClass.class");
- final JavaClass jc2 = getClassFrom(".", "SimpleAnnotatedClass");
+ final JavaClass jc2 = getClassFrom(".", name);
jc2.getAnnotationEntries();
assertTrue(delete("SimpleAnnotatedClass.class"));
// System.err.println(jc2.toString());
diff --git a/src/test/java/org/apache/bcel/util/RepositoryNameMismatchTest.java
b/src/test/java/org/apache/bcel/util/RepositoryNameMismatchTest.java
new file mode 100644
index 00000000..13f36721
--- /dev/null
+++ b/src/test/java/org/apache/bcel/util/RepositoryNameMismatchTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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
+ *
+ * https://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.bcel.util;
+
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+
+import org.apache.bcel.Const;
+import org.apache.bcel.generic.ClassGen;
+import org.junit.jupiter.api.Test;
+
+/**
+ * A class file served under one name whose this_class declares another name
must not be returned or cached:
+ * repositories key their caches on the parsed name, so accepting the mismatch
lets attacker bytes squat on
+ * an arbitrary class name JVM-wide (including the static
org.apache.bcel.Repository facade and the
+ * VerifierFactory verdict cache).
+ */
+class RepositoryNameMismatchTest {
+
+ private static byte[] classBytes(final String className) {
+ return new ClassGen(className, "java.lang.Object", className +
".java", Const.ACC_PUBLIC, null).getJavaClass().getBytes();
+ }
+
+ @Test
+ void testClassLoaderRepositoryRejectsMismatchedName() {
+ final byte[] bytes = classBytes("evil.Squatter");
+ final ClassLoader loader = new
ClassLoader(getClass().getClassLoader()) {
+ @Override
+ public InputStream getResourceAsStream(final String name) {
+ return "victim/Innocent.class".equals(name) ? new
ByteArrayInputStream(bytes) : super.getResourceAsStream(name);
+ }
+ };
+ final ClassLoaderRepository repository = new
ClassLoaderRepository(loader);
+ assertThrows(ClassNotFoundException.class, () ->
repository.loadClass("victim.Innocent"));
+ assertNull(repository.findClass("evil.Squatter"), "mismatched class
must not be cached under its declared name");
+ assertNull(repository.findClass("victim.Innocent"), "mismatched class
must not be cached under the requested name");
+ }
+}
