From 5990bf10f50d439249c0541da05f0f8427a6bf46 Mon Sep 17 00:00:00 2001 From: prozolic <42107886+prozolic@users.noreply.github.com> Date: Thu, 14 May 2026 21:00:52 +0900 Subject: [PATCH] Fix integer overflow in string.IndexOf range validation The validation `startIndex + count > Length` can overflow when comparisonType is OrdinalIgnoreCase and startIndex + count exceeds int.MaxValue. Change it to `count > Length - startIndex` to avoid the overflow. This ensures an ArgumentOutOfRangeException is thrown when startIndex + count exceeds int.MaxValue. Also update IndexOf(Rune, int, int, StringComparison) to use ThrowIfNegative and ThrowIfGreaterThan consistently with the char overload. --- .../src/System/String.Searching.cs | 9 +++--- .../System/StringTests.cs | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs b/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs index 5d69071526579b..0364871c253956 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs @@ -127,7 +127,7 @@ private int IndexOfCharOrdinalIgnoreCase(char value, int startIndex, int count) ArgumentOutOfRangeException.ThrowIfNegative(startIndex); ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex, Length); ArgumentOutOfRangeException.ThrowIfNegative(count); - ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex + count, Length); + ArgumentOutOfRangeException.ThrowIfGreaterThan(count, Length - startIndex); int subIndex; @@ -427,9 +427,10 @@ public int IndexOf(Rune value, int startIndex, StringComparison comparisonType) /// public unsafe int IndexOf(Rune value, int startIndex, int count, StringComparison comparisonType) { - ArgumentOutOfRangeException.ThrowIfLessThan(startIndex, 0); - ArgumentOutOfRangeException.ThrowIfLessThan(count, 0); - ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex + count, Length); + ArgumentOutOfRangeException.ThrowIfNegative(startIndex); + ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex, Length); + ArgumentOutOfRangeException.ThrowIfNegative(count); + ArgumentOutOfRangeException.ThrowIfGreaterThan(count, Length - startIndex); // Convert value to span ReadOnlySpan valueChars = value.AsSpan(stackalloc char[Rune.MaxUtf16CharsPerRune]); diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs index 4ec41acad8c125..d6f3f92244f687 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs @@ -1346,6 +1346,35 @@ public static void ImplicitCast_NullString_ReturnsDefaultSpan() Assert.True(span == default); } + [Fact] + public static void IndexOf_Char_OrdinalIgnoreCase_ThrowsArgumentOutOfRangeException() + { + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf('o', -1, 0, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf('o', 6, 0, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf('o', 0, -1, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf('o', 0, 6, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf('o', 3, 3, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf('o', 1, int.MaxValue, StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public static void IndexOf_Rune_StartIndexCount_ThrowsArgumentOutOfRangeException() + { + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf(new Rune('o'), -1, 0)); + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf(new Rune('o'), 6, 0)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 0, -1)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 0, 6)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 3, 3)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 1, int.MaxValue)); + + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf(new Rune('o'), -1, 0, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("startIndex", () => "Hello".IndexOf(new Rune('o'), 6, 0, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 0, -1, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 0, 6, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 3, 3, StringComparison.OrdinalIgnoreCase)); + AssertExtensions.Throws("count", () => "Hello".IndexOf(new Rune('o'), 1, int.MaxValue, StringComparison.OrdinalIgnoreCase)); + } + public static IEnumerable IndexOf_SingleLetter_StringComparison_TestData() { yield return new object[] { "Hello", 'l', 0, int.MaxValue, StringComparison.Ordinal, null, 2 };