From 41a0d57f1cb9ae0dc2b1c3d0bca274f416f4b749 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Wed, 6 May 2026 06:37:15 -0400 Subject: [PATCH] StringUtils.joins() for primitive types can throw OOME before index validation. --- .../org/apache/commons/lang3/StringUtils.java | 17 +++ .../lang3/StringUtilsJoinExceptionTest.java | 126 ++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 src/test/java/org/apache/commons/lang3/StringUtilsJoinExceptionTest.java diff --git a/src/main/java/org/apache/commons/lang3/StringUtils.java b/src/main/java/org/apache/commons/lang3/StringUtils.java index 02d32f8a091..7bc5d99c021 100644 --- a/src/main/java/org/apache/commons/lang3/StringUtils.java +++ b/src/main/java/org/apache/commons/lang3/StringUtils.java @@ -654,6 +654,15 @@ public static String center(String str, final int size, String padStr) { return rightPad(str, size, padStr); } + private static void checkFromToIndex(final int startIndex, final int endIndex, final int length) { + if (startIndex < 0) { + throw new ArrayIndexOutOfBoundsException(startIndex); + } + if (endIndex > length) { + throw new ArrayIndexOutOfBoundsException(endIndex); + } + } + /** * Removes one newline from end of a String if it's there, otherwise leave it alone. A newline is "{@code \n}", "{@code \r}", or * "{@code \r\n}". @@ -3866,6 +3875,7 @@ public static String join(final boolean[] array, final char delimiter, final int if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -3943,6 +3953,7 @@ public static String join(final byte[] array, final char delimiter, final int st if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4020,6 +4031,7 @@ public static String join(final char[] array, final char delimiter, final int st if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4097,6 +4109,7 @@ public static String join(final double[] array, final char delimiter, final int if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4174,6 +4187,7 @@ public static String join(final float[] array, final char delimiter, final int s if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4251,6 +4265,7 @@ public static String join(final int[] array, final char delimiter, final int sta if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4491,6 +4506,7 @@ public static String join(final long[] array, final char delimiter, final int st if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; @@ -4687,6 +4703,7 @@ public static String join(final short[] array, final char delimiter, final int s if (array == null) { return null; } + checkFromToIndex(startIndex, endIndex, array.length); final int count = endIndex - startIndex; if (count <= 0) { return EMPTY; diff --git a/src/test/java/org/apache/commons/lang3/StringUtilsJoinExceptionTest.java b/src/test/java/org/apache/commons/lang3/StringUtilsJoinExceptionTest.java new file mode 100644 index 00000000000..74ca21fdb2c --- /dev/null +++ b/src/test/java/org/apache/commons/lang3/StringUtilsJoinExceptionTest.java @@ -0,0 +1,126 @@ +/* + * 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.commons.lang3; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeout; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; + +/** + * Primitive joins can OOM before index validation. + *

+ * capacity() is called before bounds validation in join(byte[], char, int, int). An endIndex of Integer.MAX_VALUE causes an attempt to allocate ~8 GB before + * the array bounds are checked. Negative-startIndex bypass case. + *

+ *

+ * For {@code startIndex = -2_000_000_000, endIndex = 1, array.length = 2}: {@code count = endIndex - startIndex = 2_000_000_001}, which is positive (no + * overflow into negative territory), so the {@code count <= 0} early-return does not fire. The pre-existing {@code endIndex > array.length} check also passes + * (1 < 2). The capacity allocation then attempts to size a StringBuilder for ~2 billion items, triggering OOM before the loop body would have caught the bad + * index. + *

+ * + * Pre-patch: throws OutOfMemoryError or hangs for > 2 seconds. Post-patch: throws ArrayIndexOutOfBoundsException quickly (within 2 seconds). + */ +public class StringUtilsJoinExceptionTest { + + private static final Class EXPECTED_EX = ArrayIndexOutOfBoundsException.class; + private static final Duration TIMEOUT = Duration.ofSeconds(2); + + @Test + public void testBooleanJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new boolean[] { true }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testBooleanJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new boolean[] { true, false }, ',', -2_000_000_000, 1))); + } + + @Test + public void testByteJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new byte[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testByteJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new byte[] { 1, 2 }, ',', -2_000_000_000, 1))); + } + + @Test + public void testCharJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new char[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testCharJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new char[] { 1, 2 }, ',', -2_000_000_000, 1))); + } + + @Test + public void testDoubleJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new double[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testDoubleJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new double[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1))); + } + + @Test + public void testFloatJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new float[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testFloatJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new float[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1))); + } + + @Test + public void testIntJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new int[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testIntJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new int[] { 1, 2 }, ',', Integer.MIN_VALUE + 100, 1))); + } + + @Test + public void testLongJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new long[] { 1L }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testLongJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new long[] { 1L, 2L }, ',', -2_000_000_000, 1))); + } + + @Test + public void testShortJoinValidatesEndIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new short[] { 1 }, ',', 0, Integer.MAX_VALUE))); + } + + @Test + public void testShortJoinValidatesNegativeStartIndexBeforeCapacity() { + assertTimeout(TIMEOUT, () -> assertThrows(EXPECTED_EX, () -> StringUtils.join(new short[] { 1, 2 }, ',', -2_000_000_000, 1))); + } +}