Runtime: StringBuilder can AV when working with very large strings

[GrabYourPitchforks]

Repro code follows. The example is a bit contrived but serves to demo the problem in as few lines of code as possible.

static void Main(string[] args)
{
    StringBuilder sb = new StringBuilder();
    sb.Append(new char[2_000_000_000]); // m_ChunkLength = 2_000_000_000
    sb.Length--; // m_ChunkLength = 1_999_999_999
    string s = new string('x', 500_000_000);
    sb.Append(s); // newCurrentIndex = ‭-1_794_967_296‬ (integer overflow!) -> AV in memcpy
}

Root cause is an unchecked addition in the below line in StringBuilder.Append(string). There may be other places within StringBuilder where this pattern occurs.

https://github.com/dotnet/runtime/blob/9d85b82e3ad856068459c8f8bba8509e038afb40/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs#L813

[danmosemsft]

Besides adding the missing check here, should we also put checked{.. } around all similar chunk and offset math in StringBuilder (so if we miss a check, we might get an OverflowException rather than an AV)

[danmosemsft]

Maybe interesting to @MarcoRossignoli

[MarcoRossignoli]

Bit busy weeks I'll take a look when queue will be empty

[iSazonov]

Wouldn't a hard limit be the best solution? There are many places where int overflow could be and it’s not always possible to return the correct value, ex.: https://github.com/dotnet/runtime/blob/9d85b82e3ad856068459c8f8bba8509e038afb40/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs#L296-L298

[danmosemsft]

@iSazonov do you believe Capacity can overflow? I don't disbelieve you, but that would mean there's a missing check in a different codepath.

[iSazonov]

@danmosemsft My notice is that there are many problematic places and Capacity is only one example. Also with explicit hard limit I guess users could get more informative message in exception.

[danmosemsft]

@iSazonov yes but I'm unsure of the codepath that could lead to that particular line overflowing. Anyway, let us know if you do see other similar issues.