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
commit 0d86e866ff0bffa701ff1dae54f0fdd014abcc2c Author: Gary Gregory <[email protected]> AuthorDate: Fri Sep 4 18:30:35 2026 -0400 Verifier cache grows unboundedly with attacker-chosen class names (f024). --- src/changes/changes.xml | 3 +- .../org/apache/bcel/verifier/VerifierFactory.java | 50 +++++++++++++++---- .../apache/bcel/verifier/VerifierFactoryTest.java | 56 ++++++++++++++++++++++ 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index e6d89ec0..d740d622 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -108,7 +108,8 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Signature.matchGJIdent recurses unboundedly on nested generic signatures (Signature.translate) (f020).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Utility.decode fixed 3x buffer breaks the encode/decode round trip and throws unchecked AIOOBE (f021).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">BCELifier interpolates attacker class/package names into generated Java source unescaped (f022).</action> - <action type="fix" dev="ggregory" due-to="Gary Gregory">Verifier pass 3a delayed checks are quadratic in attribute and code size (f023).</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Verifier pass 3a delayed checks are quadratic in attribute and code size (f023).</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Verifier cache grows unboundedly with attacker-chosen class names (f024).</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add support for permitted subclasses #493.</action> <action type="add" dev="ggregory" due-to="nbauma109, Gary Gregory">Add RecordComponentInfo.getAttribute(byte tag)#494.</action> diff --git a/src/main/java/org/apache/bcel/verifier/VerifierFactory.java b/src/main/java/org/apache/bcel/verifier/VerifierFactory.java index dff3c80a..8274feec 100644 --- a/src/main/java/org/apache/bcel/verifier/VerifierFactory.java +++ b/src/main/java/org/apache/bcel/verifier/VerifierFactory.java @@ -18,24 +18,55 @@ */ package org.apache.bcel.verifier; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Vector; /** - * This class produces instances of the Verifier class. Its purpose is to make sure that they are singleton instances - * with respect to the class name they operate on. That means, for every class (represented by a unique fully qualified - * class name) there is exactly one Verifier. + * This class produces instances of the Verifier class. Its purpose is to make sure that they are singleton instances with respect to the class name they + * operate on. That means, for every class (represented by a unique fully qualified class name) there is at most one cached Verifier. The cache is bounded (see + * {@link #MAX_CACHE_SIZE_PROPERTY}); after eviction, a new Verifier is transparently created on the next request for that class name. + * <p> + * The system property {@code org.apache.bcel.verifier.VerifierFactory.maxCacheSize} controls how many Verifier instances this factory caches; + * least-recently-used entries are evicted first. Verifier names are taken from the constant pools of the (possibly untrusted) classes being verified, so an + * unbounded cache would let a single hostile class file referencing many distinct bogus type names grow the heap without limit in a long-running process. Set + * the property to {@code 0} or a negative value to opt out and restore the historical unbounded behavior. + * </p> * * @see Verifier */ public class VerifierFactory { /** - * The HashMap that holds the data about the already-constructed Verifier instances. + * Name of the system property controlling how many Verifier instances this factory caches; least-recently-used entries are evicted first. Verifier names + * are taken from the constant pools of the (possibly untrusted) classes being verified, so an unbounded cache would let a single hostile class file + * referencing many distinct bogus type names grow the heap without limit in a long-running process. Set the property to {@code 0} or a negative value to + * opt out and restore the historical unbounded behavior. */ - private static final Map<String, Verifier> MAP = new HashMap<>(); + static final String MAX_CACHE_SIZE_PROPERTY = "org.apache.bcel.verifier.VerifierFactory.maxCacheSize"; + + /** + * Default value used when {@link #MAX_CACHE_SIZE_PROPERTY} is not set. + * + * @since 6.13.0 + */ + public static final int DEFAULT_MAX_CACHE_SIZE = 10_000; + + /** + * The map that holds the data about the already-constructed Verifier instances, in least-recently-used order, + * bounded by {@link #MAX_CACHE_SIZE_PROPERTY}. + */ + private static final Map<String, Verifier> MAP = new LinkedHashMap<String, Verifier>(16, 0.75f, true) { + + private static final long serialVersionUID = 1L; + + @Override + protected boolean removeEldestEntry(final Map.Entry<String, Verifier> eldest) { + final int maxCacheSize = Integer.getInteger(MAX_CACHE_SIZE_PROPERTY, DEFAULT_MAX_CACHE_SIZE).intValue(); + return maxCacheSize > 0 && size() > maxCacheSize; + } + }; /** * The VerifierFactoryObserver instances that observe the VerifierFactory. @@ -71,11 +102,12 @@ public class VerifierFactory { } /** - * Returns the (only) verifier responsible for the class with the given name. Possibly a new Verifier object is - * transparently created. + * Returns the verifier responsible for the class with the given name. Possibly a new Verifier object is + * transparently created; if the cache bound ({@link #MAX_CACHE_SIZE_PROPERTY}) has been reached, the + * least-recently-used cached Verifier is evicted first. * * @param fullyQualifiedClassName The fully qualified class name. - * @return The (only) verifier responsible for the class with the given name. + * @return The verifier responsible for the class with the given name. */ public static Verifier getVerifier(final String fullyQualifiedClassName) { return MAP.computeIfAbsent(fullyQualifiedClassName, k -> { diff --git a/src/test/java/org/apache/bcel/verifier/VerifierFactoryTest.java b/src/test/java/org/apache/bcel/verifier/VerifierFactoryTest.java new file mode 100644 index 00000000..6f037ad6 --- /dev/null +++ b/src/test/java/org/apache/bcel/verifier/VerifierFactoryTest.java @@ -0,0 +1,56 @@ +/* + * 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.verifier; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +import org.junit.jupiter.api.Test; + +class VerifierFactoryTest { + + @Test + void testCacheIsBounded() { + final String previous = System.setProperty(VerifierFactory.MAX_CACHE_SIZE_PROPERTY, "3"); + VerifierFactory.clear(); + try { + for (int i = 0; i < 10; i++) { + VerifierFactory.getVerifier("com.example.bcel.Bogus" + i); + } + assertEquals(3, VerifierFactory.getVerifiers().length); + } finally { + if (previous != null) { + System.setProperty(VerifierFactory.MAX_CACHE_SIZE_PROPERTY, previous); + } else { + System.clearProperty(VerifierFactory.MAX_CACHE_SIZE_PROPERTY); + } + VerifierFactory.clear(); + } + } + + @Test + void testSameVerifierWhileCached() { + VerifierFactory.clear(); + try { + assertSame(VerifierFactory.getVerifier("com.example.bcel.Same"), VerifierFactory.getVerifier("com.example.bcel.Same")); + } finally { + VerifierFactory.clear(); + } + } +}
