Cap allocation size in BinaryReader.ReadString

Do not allow the untrusted payload to dictate the initial capacity of temporary StringBuilder instances.

Author: GrabYourPitchforks

src/System.Private.CoreLib/shared/System/IO/BinaryReader.cs

@@ -323,9 +323,12 @@ public virtual string ReadString()
                     return new string(_charBuffer, 0, charsRead);
                 }
 
+                // Since we could be reading from an untrusted data source, limit the initial size of the
+                // StringBuilder instance we're about to get or create. It'll expand automatically as needed.
+
                 if (sb == null)
                 {
-                    sb = StringBuilderCache.Acquire(stringLength); // Actual string length in chars may be smaller.
+                    sb = StringBuilderCache.Acquire(Math.Min(stringLength, StringBuilderCache.MaxBuilderSize)); // Actual string length in chars may be smaller.
                 }
 
                 sb.Append(_charBuffer, 0, charsRead);

src/System.Private.CoreLib/shared/System/Text/StringBuilderCache.cs

@@ -11,7 +11,7 @@ internal static class StringBuilderCache
         // The value 360 was chosen in discussion with performance experts as a compromise between using
         // as litle memory per thread as possible and still covering a large part of short-lived
         // StringBuilder creations on the startup path of VS designers.
-        private const int MaxBuilderSize = 360;
+        internal const int MaxBuilderSize = 360;
         private const int DefaultCapacity = 16; // == StringBuilder.DefaultCapacity
 
         // WARNING: We allow diagnostic tools to directly inspect this member (t_cachedInstance).