dotnet / dotnet/runtime

`CompositeFormat.Parse` overflows `int` on a large format-item index, giving `IndexOutOfRangeException` or a silently wrong argument

Open
#133,792 1 comment 0 reactions 0 assignees View on GitHub
area-System.Runtime untriaged
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Description

`CompositeFormat.Parse` accumulates a format item's index and alignment with `index = index * 10 + ch - '0'` in unchecked arithmetic, and nothing bounds how many digits it consumes (`CompositeFormat.cs:201-203` for the index and `:258-260` for the alignment, at `d84e42c`). Past `int.MaxValue` the value wraps, and the wrapped result is stored in the parsed segment. Two distinct failure modes follow.

**A wrapped-negative index escapes the documented contract.** `"{2147483648}"` stores `ArgIndex = int.MinValue`. That segment is then neither a literal nor a hole, so the constructor counts it as neither, `_argsRequired` stays `0`, `ValidateNumberOfArgs` passes, and every formatting entry point reaches `handler.AppendFormatted(args[index], ...)` with a negative index. The result is `IndexOutOfRangeException`, which none of the fifteen public overloads taking a `CompositeFormat` document — they document `ArgumentNullException` and a `FormatException` for *"The index of a format item is greater than or equal to the number of supplied arguments."*, and a negative index is not that condition.

**A wrapped index that lands back in range silently formats the wrong argument.** `"{4294967296}"` wraps to `0`, so the hole that asked for argument 4,294,967,296 is filled with argument 0 and nothing is reported at all.

It also breaks an invariant the type documents about itself. `CompositeFormat.cs:17` says of `_segments`: *"Every segment represents either a literal or a format hole, based on whether Literal is non-null or ArgIndex is non-negative."* The wrapped segment is `Literal=null ArgIndex=-2147483648`, so it is neither.

The same hand-written parser in `StringBuilder.AppendFormat` and `ValueStringBuilder.AppendFormatHelper` carries an `IndexLimit`/`WidthLimit` guard that stops digit consumption and makes both failure modes unreachable. `CompositeFormat`'s copy dropped it. That *divergence* is #119756; this issue is the overflow itself, which is a separate defect and stays reachable however the ceiling question there is settled.

### Reproduction Steps

```csharp
using System.Text;

CompositeFormat cf = CompositeFormat.Parse("{2147483648}"); // parses without complaint
Console.WriteLine(cf.MinimumArgumentCount); // 0

Show(() => string.Format(null, cf, "x"));
Show(() => new StringBuilder().AppendFormat(null, cf, "x").ToString());
Show(() =>
{
Span buffer = stackalloc char[64];
buffer.TryWrite(null, cf, out int written, "x");
return new string(buffer[..written]);
});

// the silently-wrong-argument form
Show(() => string.Format(null, CompositeFormat.Parse("{4294967296}"), "x"));

// the same two strings through the guarded parser, for contrast
Show(() => string.Format("{2147483648}", "x"));
Show(() => string.Format("{4294967296}", "x"));

static void Show(Func f)
{
try { Console.WriteLine($" -> [{f()}]"); }
catch (Exception e) { Console.WriteLine($" -> {e.GetType()}: {e.Message}"); }
}
```

Each call is caught so the program runs to the end — the first one to throw would otherwise stop it. The first result is the one that differs by runtime: on a `main` build all three entry points throw, while on 10.0.12 `string.Format` returns the raw format string and only the other two throw, for the reason in **Regression?** below.

### Expected behavior

`Parse` rejects a format item whose index or alignment cannot be represented, with `FormatException` — which is exactly what the same two strings do through `string.Format(string, ...)` and `StringBuilder.AppendFormat(string, ...)`:

```
System.FormatException: Input string was not in a correct format. Failure to parse near offset 8. Format item ends prematurely.
```

Failing that, the formatting entry points should at least throw the documented `FormatException`, and should never format an argument the format string did not ask for.

### Actual behavior

On a local `main` build at `d84e42c` (`clr+libs -rc release`, reporting 12.0.0-dev). One line per entry point, taken from a longer run that also covers the `object[]` and `ReadOnlySpan` overloads of the first two — those behave identically and are left out here:

```
cf = Parse("{2147483648}"), MinimumArgumentCount = 0
string.Format(null, cf, "x")
-> System.IndexOutOfRangeException: Index was outside the bounds of the array.
new StringBuilder().AppendFormat(null, cf, "x")
-> System.IndexOutOfRangeException: Index was outside the bounds of the array.
Span[64].TryWrite(null, cf, out _, "x")
-> System.IndexOutOfRangeException: Index was outside the bounds of the array.

silently wrong argument (2^32 wraps to index 0):
string.Format(null, cf("{4294967296}"), "x")
-> [x]
what the guarded parser does with the same string:
string.Format("{4294967296}", "x")
-> System.FormatException: Input string was not in a correct format. Failure to parse near offset 8. Format item ends prematurely.
```

Reflecting over the private `_segments` gives the same values on 10.0.12 and on that `main` build. Only segment `[1]`, the hole, is shown; `[0]` and `[2]` are empty literals:

```
"{2147483648}" -> literalLength=0 formattedCount=0 argsRequired=0
[1] Literal=null ArgIndex=-2147483648 Alignment=0 Format=null
"{4294967296}" -> literalLength=0 formattedCount=1 argsRequired=1
[1] Literal=null ArgIndex=0 Alignment=0 Format=null
"{0,2147483648}" -> literalLength=0 formattedCount=1 argsRequired=1
[1] Literal=null ArgIndex=0 Alignment=-2147483648 Format=null
```

### Regression?

The overflow is not a regression — the parsing logic is unchanged between 10.0.12 and `main` (`git diff release/10.0 main -- src/libraries/System.Private.CoreLib/src/System/Text/CompositeFormat.cs` is a BOM plus `unsafe` added to the `TryParseLiterals` signature), and the transcripts above are identical on both.

Its *reachability through `string.Format`* is new, and correctly so. On 10.0.12, `string.Format(null, cf, "x")` returns the raw `"{2147483648}"` rather than throwing, because its fast path tested `format._formattedCount == 0` alone and the negative `ArgIndex` had left `_formattedCount` at 0 — so the fast path fired on a format string that does have a hole. `StringBuilder.AppendFormat` and `MemoryExtensions.TryWrite` have no such fast path and throw on 10.0.12 today. #127819 (`2e33a3e335d`) added `format._literalLength == format.Format.Length` to both `string.Format` fast paths while fixing an unrelated brace-escaping bug (#127794); for `"{2147483648}"` those are 0 and 12, so `main` now falls through to segment iteration and throws like the other two. That commit removed an accident that had been concealing a third of this.

### Known Workarounds

None within the API. A caller that may see generated or untrusted format strings has to validate the index and alignment digits itself before calling `Parse`.

### Configuration

.NET 10.0.12, x64, Windows 11; and a local `main` build at `d84e42c` (`clr+libs -rc release`, reporting 12.0.0-dev). Not specific to either — it is arithmetic in the parser.

### Other information

Adding the same guard the other two parsers already have removes both failure modes at the source. I built that (the two constants and the two loop conditions added to `CompositeFormat.TryParseLiterals`, in that method's own `TryMoveNext`/`goto` style) and ran it: `Parse("{2147483648}")` and `Parse("{4294967296}")` then both throw `FormatException`, so no wrapped index reaches a segment and all three entry points fail at `Parse` instead. It does change behaviour — an index or alignment above 9,999,999 parses today and would start throwing — which is the compatibility question being discussed on #119756, so the two are worth settling together even though the defects are separate. Bounding the accumulator at `int.MaxValue` without touching that ceiling would fix this issue alone.

I could not find a prior report: `CompositeFormat overflow` returns nothing across issues and PRs, and the only hit for `CompositeFormat IndexOutOfRangeException` is #90357, a 2023 build-time crash in `ProvideCorrectArgumentsToFormattingMethodsAnalyzer`, which is unrelated.

I can send a PR for whichever shape an area owner prefers.

> [!NOTE]
> AI-generated, written at my direction and reviewed by me before posting. The behaviour reported here was executed, not inferred: on .NET 10.0.12 and on a local `main` build at `d84e42c` (`clr+libs -rc release`, reporting 12.0.0-dev), and the guarded-`CompositeFormat` result on a further build of that tree with the guard added. The source and git-history claims were checked against the repo at `d84e42c`.

Contributor guide

Open the contributing guide

Research direction

Start in src/libraries/System.Private.CoreLib/src/System/Text/CompositeFormat.cs, especially TryParseLiterals and the index and alignment parsing at the reported lines. Compare its digit-consumption behavior with the guarded parsers in StringBuilder.AppendFormat and ValueStringBuilder.AppendFormatHelper, then run the supplied large-index and alignment reproductions. Done means oversized values no longer wrap into parsed segments and the inputs consistently produce the documented FormatException.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.